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 07 Apr 2013 10:59:35 AM UTC  
 
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.0

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 03 Aug 2014 06:53:14 PM UTC, 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 03 Aug 2014 06:53:09 PM UTC, 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 02 Aug 2014 12:13:48 PM UTC, 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 01 Aug 2014 12:36:32 AM UTC, 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 12 Jul 2014 08:31:50 PM UTC, 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 12 Jan 2014 03:56:09 PM UTC, 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 10 Apr 2013 10:25:25 PM UTC, 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 07 Apr 2013 11:15:56 AM UTC, comment #2:

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

Jacob Nevins <jtn>
Project Administrator
Sun 07 Apr 2013 11:02:14 AM UTC, 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 07 Apr 2013 10:59:35 AM UTC, 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.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 03 Aug 2014 06:53:27 PM UTCcazfiStatusReady For Test=>Fixed
      Assigned toNone=>cazfi
      Open/ClosedOpen=>Closed
    Fri 01 Aug 2014 12:36:32 AM UTCcazfiAttached File-=>Added MultibackendLuasql.patch, #21612
      StatusNone=>Ready For Test
    Tue 13 May 2014 09:00:54 PM UTCcazfiSummaryEnabling multiple luasql backends broken on trunk=>Enabling multiple luasql backends broken on S2_5 & trunk
    Sun 13 Apr 2014 06:53:23 AM UTCcazfiSeverity3 - Normal=>5 - Blocker
      Planned Release2.5.0=>2.5.0, 2.6.0
    Sun 07 Apr 2013 11:15:56 AM UTCjtnSummaryLuasql 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