bugFreeciv - Bugs: bug #20723, Enabling multiple luasql backends...

 
 
Show feedback again

bug #20723: Enabling multiple luasql backends broken on S2_5 & trunk

Submitted by:  Jacob Nevins <jtn>
Submitted on:  Sun Apr 7 10:59:35 2013  
 
Category: generalSeverity: 5 - Blocker
Priority: 5 - NormalStatus: Fixed
Assigned to: Marko Lindqvist <cazfi>Open/Closed: Closed
Release: trunk r22685Operating System: GNU/Linux
Planned Release: 2.5.0, 2.6.0Contains string changes: None

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sun Aug 3 18:53:14 2014, SVN revision 25799:

Made authentication to work when multiple sql backends has been enabled simultaneously.

Reported by Jacob Nevins

See bug #20723

(Browse SVN revision 25799)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sun Aug 3 18:53:09 2014, SVN revision 25798:

Made authentication to work when multiple sql backends has been enabled simultaneously.

Reported by Jacob Nevins

See bug #20723

(Browse SVN revision 25798)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat Aug 2 12:13:48 2014, comment #7:

> I don't currently have good setup to test this, so I'd be
> grateful if someone else could do some testing on this.

I confirm that a server binary built with this patch works against both MySQL and SQLite backends. Thanks!

Jacob Nevins <jtn>
Project Administrator
Fri Aug 1 00:36:32 2014, comment #6:

> The LuaSQL docs indicate that an equivalent set of 'require'
> statements inside Lua can end up with a 'luasql' table
> containing multiple drivers simultaneously.


(That site does not respond, but I found some documentation from luasql git)

No, the example shows how 'require' is used to open multiple tables, not one where everything works.

So first step is obviously opening all the tables. That's easy.

Ideally I would then like to combine the tables in to one, but that's not very easy, at least to be done right. So I think it's sufficient for now to use multiple tables in database.lua too.

Patch attached. I don't currently have good setup to test this, so I'd be grateful if someone else could do some testing on this.

(file #21612)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sat Jul 12 20:31:50 2014, comment #5:

I want to fix this properly for 2.5.0 rather than have a regression. Can anyone help?

However, failing that, we could make configure disallow multiple luasql backends.

Jacob Nevins <jtn>
Project Administrator
Sun Jan 12 15:56:09 2014, comment #4:

So I think the luaL_requiref() invocations each replace the 'luasql' table/object/thing in the global namespace with a new one, so only the last one invoked wins, giving us a 'luasql' table containing only e.g. 'sqlite3()'.

The LuaSQL docs indicate that an equivalent set of 'require' statements inside Lua can end up with a 'luasql' table containing multiple drivers simultaneously.

I haven't got enough of a handle on Lua semantics or implementation yet to figure out how to simulate this when loading modules from C, however.

Jacob Nevins <jtn>
Project Administrator
Wed Apr 10 22:25:25 2013, comment #3:

And the place where I would look first is server/scripting/script_fcdb.c, currently lines 203-214:

#ifdef HAVE_FCDB_MYSQL
luaL_requiref(fcl->state, "luasql", luaopen_luasql_mysql, 1);
lua_pop(fcl->state, 1);
#endif
#ifdef HAVE_FCDB_POSTGRES
luaL_requiref(fcl->state, "luasql", luaopen_luasql_postgres, 1);
lua_pop(fcl->state, 1);
#endif
#ifdef HAVE_FCDB_SQLITE3
luaL_requiref(fcl->state, "luasql", luaopen_luasql_sqlite3, 1);
lua_pop(fcl->state, 1);
#endif

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sun Apr 7 11:15:56 2013, comment #2:

Yes, --enable-fcdb=mysql works, confirming your diagnosis (and incidentally confirming that MySQL works).

Jacob Nevins <jtn>
Project Administrator
Sun Apr 7 11:02:14 2013, comment #1:

I actually think that enabling multiple backends at once is now broken, but never got around to testing it. Can you test if mysql alone works?

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Sun Apr 7 10:59:35 2013, original submission:

Finally got around to testing after bug #20612. Unfortunately, while sqlite works, mysql doesn't. Attempting to start a server with a MySQL-configured configuration file gives errors:

(My fc_config.h does HAVE_FCDB_MYSQL; I built with --enable-fcdb=sqlite3,mysql. But "/fcdb lua print(luasql.mysql)" does indeed return "nil".)

Jacob Nevins <jtn>
Project Administrator

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #21612:  MultibackendLuasql.patch added by cazfi (2kB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one

Digest:
   task dependencies.

 

Carbon-Copy List
  • -unavailable- added by cazfi (Posted a comment)
  • -unavailable- added by jtn (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Error: not logged in

     

     

    Follow 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun Aug 3 18:53:27 2014cazfiStatusReady For Test=>Fixed
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Fri Aug 1 00:36:32 2014cazfiAttached File-=>Added MultibackendLuasql.patch, #21612
      StatusNone=>Ready For Test
    Tue May 13 21:00:54 2014cazfiSummaryEnabling multiple luasql backends broken on trunk=>Enabling multiple luasql backends broken on S2_5 & trunk
    Sun Apr 13 06:53:23 2014cazfiSeverity3 - Normal=>5 - Blocker
      Planned Release2.5.0=>2.5.0, 2.6.0
    Sun Apr 7 11:15:56 2013jtnSummaryLuasql mysql backend seems broken on trunk=>Enabling multiple luasql backends broken on trunk
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup