Bug #60872 | cmake-based build system declares DBUG_OFF/DBUG_ON in CFLAGS instead of headers | ||
---|---|---|---|
Submitted: | 14 Apr 2011 15:22 | Modified: | 20 Apr 2011 9:11 |
Reporter: | Antony Dovgal | Email Updates: | |
Status: | Verified | Impact on me: | |
Category: | MySQL Server: Compiling | Severity: | S1 (Critical) |
Version: | 5.1, 5.5 | OS: | Any |
Assigned to: | CPU Architecture: | Any |
[14 Apr 2011 15:22]
Antony Dovgal
[14 Apr 2011 15:42]
Davi Arnaut
The same bug exist in 5.1 to some extent. Also, watch your tongue and read the license of the product you are using.
[14 Apr 2011 16:04]
Antony Dovgal
I don't know what you mean by 'some extent', but: mysql-5.1.32/include/my_config.h .. /* Don't use libdbug */ #define DBUG_OFF 1 .. with autotools this definitely worked fine, so the problem is more or less related to cmake. And yes, I realize a sound a bit annoyed, but I keep fixing API/ABI breaks after each MySQL release and this is really getting on my nerves.
[14 Apr 2011 16:07]
Davi Arnaut
There is also a cmake build system on 5.1
[14 Apr 2011 16:10]
Antony Dovgal
Yes, there was. Do you want me to change Version to 5.1?
[14 Apr 2011 16:15]
Davi Arnaut
Done.
[14 Apr 2011 16:49]
Valeriy Kravchuk
In 5.5 (and in 5.1 when cmake is used) we have: macbook-pro:mysql-5.5-work openxs$ grep -rn DBUG_OFF * | grep my_config.h macbook-pro:mysql-5.5-work openxs$ while in 5.1 build by default (with autotools): macbook-pro:mysql-5.1-work openxs$ grep -rn DBUG_OFF * | grep my_config.h include/my_config.h:22:/* #undef DBUG_OFF */ macbook-pro:mysql-5.5-work openxs$ In both cases debug builds were used, like this: BUILD/compile-pentium-debug-max --prefix=/Users/openxs/dbs/5.1
[14 Apr 2011 17:53]
Peter Laursen
A private comment to Davi is that he should rather learn 'watching his tounge'. I can't count the times he has offended me. Of course we all post bug reports just in order to bother him personally - and he knows, it seems!
[14 Apr 2011 18:06]
Davi Arnaut
Peter, I usually comment on bugs that are affecting areas that I have some knowledge about. For example, you don't see me commenting on installer bugs, neither you see me adding out of topic comments on random bug reports. The "watch your tongue" (which is a "pay attention") is just a reminder that offending us won't get us to fix certain bugs more quickly, it might have the opposite effect in some cases.
[18 Apr 2011 19:21]
Vladislav Vaintroub
This can't possibly be critical severity or triaged as serious bug. If you use MYSQL_ADD_PLUGIN as described in Wiki ( http://forge.mysql.com/wiki/CMake#MYSQL_ADD_PLUGIN_-_build_mysql_plugin ) then any plugin will build fine, with the very same flags as the server itself. As of why DBUG_OFF is not defined in my_config.h, for that there is another section in the Wiki : http://forge.mysql.com/wiki/CMake#Debug-only_options In short, having DBUG_OFF in my_config.h does not work because it does not work with multi-config CMake generators of which there are currently two: Visual Studio and Xcode. Thus, the flags have to be passed on the command line. And yes, this would be nice to be able to build plugins without having to compile MySQL, however this is rather a feature request than a bug (this never worked so far)
[19 Apr 2011 10:34]
Antony Dovgal
>This can't possibly be critical severity or triaged as serious bug. This is critical for third-party plugins. >If you use MYSQL_ADD_PLUGIN as described in Wiki Yes, and if you use autoconf/automake, it works either. I don't use Cmake and I see no reason to switch to it. >In short, having DBUG_OFF in my_config.h does not work because it does not >work with multi-config CMake generators of which there are currently two: >Visual Studio and Xcode. So.. You in fact admit that the problem IS in CMake, right? Doesn't this mean that CMake is.. well.. doesn't work? >Thus, the flags have to be passed on the command line. I don't really care if you pass DBUG_OFF or DBUG_ON or BUILD_MODE=Debug or anything else as long as your headers are still usable. As it is, they contain only part of the information, which means they're only partly usable. >And yes, this would be nice to be able to build plugins without having to >compile MySQL, however this is rather a feature request than a bug (this never >worked so far) Just apply this patch: http://dev.daylessday.org/diff/mysql_install_headers.diff I (and MySQL developers) have checked it and it DOES work fine. But you guys weren't happy with the 'radical' way of installing all the headers. No idea what you're trying to hide, though.
[19 Apr 2011 11:30]
Vladislav Vaintroub
@Antony, I cannot not really decipher what you're trying to tell. You aim to build a storage engine plugin having a binary distribution only. This was never supported so far, as I said. Documented way was to put your engine source dir under storage/ and use either MYSQL_ADD_PLUGIN (or plug.in in the glorious autotools past). You may develop your own way to build storage engines using binary distribution, sure, but you'll have to understand that bugs you file are feature requests. So if you do not care about how DBUG_OFF is passed, as long as it does work, do MYSQL_ADD_PLUGIN as other engines do, and you're fine. You will have to use CMake, sorry. The patch http://dev.daylessday.org/diff/mysql_install_headers.diff cannot be applied, there are no Makefile.am anymore. >>In short, having DBUG_OFF in my_config.h does not work because it does not >>work with multi-config CMake generators of which there are currently two: >>Visual Studio and Xcode. >So.. You in fact admit that the problem IS in CMake, right? >Doesn't this mean that CMake is.. well.. doesn't work? Wrong. CMake supports both debug and release builds with the single config step, once underlying tool (in this case VS and Xcode) support it. In that it is superior to autotools. As we're using the same header file in both debug and release configurations, debug only flags have to be passed with CFLAGS.
[19 Apr 2011 11:47]
Davi Arnaut
Can't we generate the header for each configuration? I tend to think that this is a CMake/Tool problem that is being pushed down to us.
[19 Apr 2011 11:53]
Davi Arnaut
Also, this is not a autotools vs cmake issue, let's not make it one. The point is what is most convenient for us and third-party developers.
[19 Apr 2011 12:22]
Vladislav Vaintroub
@Davi, this could be can of worms. 2 versions of server are distributed with binary packages (normal and -debug one), which for me means 2 versions of my_config.h have to be included into distribution and placed into different include directories. Maybe cleaner way would to export DBUG functionality (dbug_* functions) would be server service (in libmysqlservices), since this is designed to export server functions to plugins. Those dbug_* service functions might as well be dummy if server itself is not compiled with DBUG.
[19 Apr 2011 18:17]
Antony Dovgal
>Antony, I cannot not really decipher what you're trying to tell. Ok, I'l use shorter words for you. >You aim to build a storage engine plugin having a binary distribution only. >This was never supported so far, as I said. Thank you. I know that. And I also offered a patch that fixes it. >Documented way was to put your engine source dir under storage/ >and use either MYSQL_ADD_PLUGIN (or plug.in in the glorious autotools past). It wasn't documented this way when I started writing my plugin and it still makes no sense, since every decent Linux/Unix distribution has it's own MySQL package built with its very own flags and options. (everything clear so far? I can switch to Russian if it's not) >You may develop your own way to build storage engines using binary >distribution, sure, but you'll have to understand that bugs you file are >feature requests. There is that thing called 'plugin API' in MySQL. It's a part of MySQL. This part was broken when you switched from autotools to cmake. I'm not asking you to add a feature, I'm reporting a bug. >So if you do not care about how DBUG_OFF is passed, as long as it does work, >do MYSQL_ADD_PLUGIN as other engines do, and you're fine. >You will have to use CMake, sorry. It's been working fine for years and now it doesn't work anymore because of some silly CMake limitations. A bug is a bug, face it. >The patch http://dev.daylessday.org/diff/mysql_install_headers.diff >cannot be applied, there are no Makefile.am anymore. Whatever. Missing headers is a different problem and it should be discussed in separate report. >Wrong. CMake supports both debug and release builds with the single >config step, once underlying tool (in this case VS and Xcode) support it. >In that it is superior to autotools. As we're using the same header file in >both debug and release configurations, debug only flags have to be passed with CFLAGS. Use a different file in different configurations. my_config_debug.h, my_config_release.h. Or add another header file for that case. Or you can drop that DBUG_OFF nonsense completely and go on with just DBUG_ON - just use #ifdef DBUG_ON in my_dbug.h and remove the #ifdef in dbug.c. I.e. consider that the build is in Release mode by default. This way the functions would be still present, but the macros would be empty in my plugin only (since you don't know of any way to add DBUG_ON to my_config.h). There is a million solutions available, if you think about it.
[19 Apr 2011 18:23]
Davi Arnaut
Antony, Are you sure the plugin API is affected? There shouldn't be any DBUG specific code in it. Perhaps you mean the storage engine API? Also, just to see if I understand your problem, it all boils down to the fact that you use DBUG in your code and you want to be able to switch it on/off depending on whether it was on/off for the server?
[19 Apr 2011 18:50]
Antony Dovgal
>Are you sure the plugin API is affected? >There shouldn't be any DBUG specific code in it. >Perhaps you mean the storage engine API? My code simply uses DBUG_* macros. Now see for yourself: include/my_dbug.h: #if !defined(DBUG_OFF) && !defined(_lint) ... #define DBUG_ENTER(a) struct _db_stack_frame_ _db_stack_frame_; \ _db_enter_ (a,__FILE__,__LINE__,&_db_stack_frame_) .. #endif dbug.c: #ifndef DBUG_OFF .. void _db_enter_(const char *_func_, const char *_file_, uint _line_, struct _db_stack_frame_ *_stack_frame_) .. #endif Apparently I don't have DBUG_OFF defined since it's missing in my_config.h. But it was defined when MySQL was compiled. So when I include the headers in my plugin, the macros act as the debug mode is ON and the debug functions are undefined, which means my plugin won't load - 'undefined symbol _db_enter()' etc. >Also, just to see if I understand your problem, it all boils down to the >fact that you use DBUG in your code and you want to be able to switch it >on/off depending on whether it was on/off for the server? It all comes down to this: I want to build an extension/plugin/storage engine using the headers of MySQL server. There are two parts of this problem: 1) I'm forced to use MySQL sources because MySQL server headers aren't installed when you do 'make install' and therefore not included in binary distributions (see bug #44722). 2) The DBUG_OFF/DBUG_ON defines are not defined in my_config.h anymore and there is not way to know their values, which is the reason of 'undefined symbols' errors.
[19 Apr 2011 19:08]
Davi Arnaut
> So when I include the headers in my plugin, the macros act as the debug mode > is ON and the debug functions are undefined, which means my plugin won't load > - 'undefined symbol _db_enter()' etc. OK, but keep in mind that DBUG is a server internal debugging facility, it isn't a public header nor a plugin interface in any shape or form. > It all comes down to this: I want to build an extension/plugin/storage engine using > the headers of MySQL server. But that's not the problem you are encountering. You are using DBUG in your code and you want to determine at build time whether a hypothetical server binary was also built with DBUG code. > 1) I'm forced to use MySQL sources because MySQL server headers aren't installed > when you do 'make install' and therefore not included in binary distributions (see > bug #44722). Which headers are these exactly? > 2) The DBUG_OFF/DBUG_ON defines are not defined in my_config.h anymore and there > is not way to know their values, which is the reason of 'undefined symbols' errors. This is a consequence of DBUG being server internal, yet it is being used by a plugin/engine. Like we discussed some time ago on the internals mailing list, not everything in our headers is for external/plugin consumption. See include/mysql/service* for an example of things that are exported. Also, DBUG_ON/OFF in any kind of header is not going to definitely solve this issue either because it does not really establishes whether the binary your plugin is being loaded into has the DBUG symbols. If you think DBUG is something worthwhile for plugin developers, we should go the path of properly exporting the DBUG functionality. In which case, the final decision on whether to ultimately enable or disabled debug support will fall down to the plugin itself.
[19 Apr 2011 19:13]
Davi Arnaut
Also, I would like to stress the point that storage engines are still tightly integrated with the server. This is crucial to the point that they should be compiled in the same exact way the server was built, down to the compiler flags. For this, MYSQL_ADD_PLUGIN must be used. We know this is all a bit crappy, but we are slowly working towards making the interfaces better. See http://dev.mysql.com/doc/refman/5.5/en/plugin-api.html for progress we have made on this area.
[19 Apr 2011 19:23]
Antony Dovgal
>OK, but keep in mind that DBUG is a server internal debugging facility, >it isn't a public header nor a plugin interface in any shape or form. In this case you might consider removing my_dbug.h from my_global.h. Also I really doubt I can create a storage engine plugin without using any internal APIs. >you want to determine at build time whether a hypothetical server binary was >also built with DBUG code. *sigh* No. I'm forced to find a way to do it since you left me no option. It's a workaround to this bug, not the problem itself. >Which headers are these exactly? #include <include/mysql_version.h> #include <sql/field.h> #include <sql/structs.h> #include <sql/handler.h> #include <my_dir.h> #include <mysql/plugin.h> #include <mysql.h> >Also, DBUG_ON/OFF in any kind of header is not going to definitely solve this >issue either because it does not really establishes whether the binary your >plugin is being loaded into has the DBUG symbols. You really really miss my point. At the moment I can't build my plugin with the headers of MySQL server. Please re-read what I said in my previous comment - the macros have wrong values when DBUG_OFF is not declared and the actual functions are not even compiled. >If you think DBUG is something worthwhile for plugin developers Whoa.. wait one here. What IS the difference between say .. storage/csv and any other storage engine plugin? Why do you think storage/csv is 'internal enough' to use DBUG_* macros and some other plugin is not?
[19 Apr 2011 19:32]
Davi Arnaut
> In this case you might consider removing my_dbug.h from my_global.h. In the future we might. But, again, just because something is in my_global.h it does not automatically mean it can be used by a plugin. > Also I really doubt I can create a storage engine plugin without using any internal APIs. Yes, that's why I said that storage engines are a special case. > I'm forced to find a way to do it since you left me no option. > It's a workaround to this bug, not the problem itself. So, you are not actually using DBUG code in your plugin? > Please re-read what I said in my previous comment - the macros have wrong values when > DBUG_OFF is not declared and the actual functions are not even compiled. Yes, because the _server_ was compiled without debug, NOT your plugin. So, what is it? Your plugin is using DBUG code or it's a header that you are including that forces your plugin to have DBUG code? > What IS the difference between say .. storage/csv and any other storage engine plugin? See my comment above about storage engines being different.
[19 Apr 2011 19:33]
Antony Dovgal
>Also, I would like to stress the point that storage engines are still tightly >integrated with the server. This is crucial to the point that they should be >compiled in the same exact way the server was built, down to the compiler flag. Yes, this is the reason why bug #44722 exists. But hey, at least we have some understanding here. >For this, MYSQL_ADD_PLUGIN must be used. No, just add all defines to my_config.h and install all the other headers and you're done (or make the defines accessible my some other means, I don't care). Really, guys. If I want to use a function from some library, I only need to include its headers and link against the library. If I write an extension for, say, PHP, I just need to include its headers. I don't need to switch to CMake, scons or create a handcrafted Makefile AND build PHP from scratch just to compile an extension. That's what I call 'plugin API'.
[19 Apr 2011 19:34]
Davi Arnaut
Also, are we talking about plugins (as in the documented link I posted above) or storage engines? The rules are different for plugins and storage engines.
[19 Apr 2011 19:37]
Davi Arnaut
> No, just add all defines to my_config.h and install all the other headers and you're done > (or make the defines accessible my some other means, I don't care). No. As I tried to explain, MySQL headers > If I want to use a function from some library, I only need to include its headers and > link against the library. Yes, but our headers are not ready for this yet. > If I write an extension for, say, PHP, I just need to include its headers. Yes, but PHP is not MySQL. It simply does NOT work for us in the current form of the headers. > That's what I call 'plugin API'. Yes, that's why we have the 'plugin API' and the 'storage engine API'.
[19 Apr 2011 19:45]
Davi Arnaut
Antony, I just want to make it clear that I'm not denying that the headers required for storage engine development are a mess -- they are and because of this they also pull server internal stuff, making it even worse. For things to work, the storage engine must be compiled exactly as the server, but the information required for this is not solely based on headers -- even if it worked to some extent in the past. With cmake we made this a little less worse by adding MYSQL_ADD_PLUGIN which makes the storage engine be compiled like the server was. I urge you to go down this road, it will save you trouble in the future.
[20 Apr 2011 4:44]
Antony Dovgal
> So, you are not actually using DBUG code in your plugin? How many times do I have to repeat myself? I do use DBUG_* macros. They stopped having proper values because of missing DBUG_OFF define. >Yes, because the _server_ was compiled without debug, NOT your plugin. YES. Because DBUG_OFF was declared when the server was compiled and it wasn't when my plugin was compiled. >The rules are different for plugins and storage engines. Believe it or not, we're talking about plugin storage engines. >No. As I tried to explain, MySQL headers Yes? MySQL headers.. what? >Yes, but PHP is not MySQL. >It simply does NOT work for us in the current form of the headers. Okay, let's get back to the thread in devel list: http://lists.mysql.com/internals/36428 Obviously all the developers agreed that installing the headers is good idea, they just wanted to take some time and separate really 'private headers' from public ones. And now you're saying "it simply doesn't work for us". That's a tough argument. I don't think I can beat this one. >With cmake we made this a little less worse by adding MYSQL_ADD_PLUGIN which >makes the storage engine be compiled like the server was. It's all very nice, but how is that supposed to help me in general case? Everybody will still have to: 1) download the sources 2) figure out what were the options used for their binary distribution 3) configure the sources in the same way 4) put my plugin in there 5) then compile it. Compare this to the way I propose: 1) compile the plugin using the headers available. It did work for me and it did work for other people. If it doesn't work for you, you're not trying hard enough.
[20 Apr 2011 9:11]
Antony Dovgal
Ok, back to the real problem. Is there anything obviously wrong with the solution I mentioned earlier? Here's the patch: http://dev.daylessday.org/diff/dbug_off_default.diff Or, you can check if both DBUG_OFF and DBUG_ON are undefined and define one of them (probably DBUG_OFF, since I can't imagine why non-developers would prefer having a debug build by default): http://dev.daylessday.org/diff/dbug_off_define.diff