Re: [PATCH] tests: import variables for MSVC.
Den 2010-09-27 10:45 skrev Peter Rosin: > Den 2010-09-25 07:28 skrev Charles Wilson: >> With regards to Ralf's question re: _MSC_VER. Well, technically you'd >> probably be "more" correct to do: >> >> #if (defined(_WIN32) || defined(_WIN32_WCE)) && !defined(__GNUC__) >> ... >> >> rather than _MSC_VER; that formula would indicate "any win32 or wince >> platform, using any compiler EXCEPT gcc" -- because only gcc has >> "auto-import" on win32. > > I agree, it should be more stable, so I'm going with your #if. Pushing > with that squashed in (but I'm still waiting on the testsuite, so if > anybody objects there's still some time). Testsuite was happy with the new #if (Cygwin/gcc and MSVC), so I have now pushed. Thanks for reviewing! Cheers, Peter
Re: [PATCH] tests: import variables for MSVC.
Den 2010-09-25 07:28 skrev Charles Wilson: > On 9/24/2010 2:46 PM, Peter Rosin wrote: >> Now I'm also confused. > > That's not good. > >> /me double checks (see below) >> >> WHAT? It doesn't work as I stated!?! >> >> *ponders that for a bit* >> *scratches head* >> >> Ahh, you said "libtool does this by default IIRC". If that's actually the >> case than that is what has have me fooled for years. > > Pay close attention to how libtool compiles (the single, only) main.o. > Does it get the picflag (-DDLL_EXPORT) or not? > > As I've always understood it, *without* auto-import magic, you MUST have > the following: > > 1) shared: > a) a dll, compiled with declspec(dllexport) [or, create the DLL with > an explicit .def file listing the exported symbols] > b) the client *must* be compiled with declspec(dllimport) decorators > on all symbols the client wants to use from the DLL > > 2) static > a) a lib. Nothing should be compiled with declspec(dllexport), and > obviously there is no .def file involved > b) the client must NOT be compiled with declspec(dllimport) -- because > then you get the missing __imp__foo error. > > So, your test case below doesn't surprise me. What DOES surprise me, is > that it ever worked at all with MSVC (or gcc + -disable-auto-import), > since it appears that Ralf is correct and the *_PROGRAM objects are > built in only one "mode". Whether that is "picflag" (-DDLL_EXPORT) or > not, one or the other linking modality should fail (static linking if > main.o is compiled with -DDLL_EXPORT; dynamic linking if main.o is NOT > compiled with -DLL_EXPORT). > > So, yeah, I'm a little confused as well. I think you need to do some > archeology on the *_PROGRAM objects (nm -B or whatever the equivalent is > in msvc land), and figure out what symbols are undefined. I'd take a > hard look at demo/. I have done some more digging and am no longer confused. As it happens, you could say that we are both right. The little sample I quoted was a little bit too small, so it behaved according to your mental image. If you "flesh out" the example so that the program also uses a function from the library, it magically starts to find the variable too, making it behave according to my mental image. Now, libraries that only export data items are /rare/, and clients that only use data items are also /rare/. So, in theory you're correct and in practice I'm correct. So, this works: $ cat << EOF > dllfoo.c extern __declspec(dllexport) int variable; int variable = 2; int foo(void) { return 0; } EOF $ cl -nologo -Fefoo.dll -MD dllfoo.c -link -dll -export:variable,DATA -export:foo -implib:foo.dll.lib dllfoo.c Creating library foo.dll.lib and object foo.dll.exp $ cat << EOF > bar.c extern __declspec(dllimport) int variable; int foo(void); int main(void) { foo(); return variable; } EOF $ cl -nologo -Febardll.exe -MD bar.c foo.dll.lib bar.c $ ./bardll $ echo $? 2 $ cat << EOF > libfoo.c extern int variable; int variable = 2; int foo(void) { return 0; } EOF $ cl -nologo -c -MD libfoo.c libfoo.c $ lib -nologo -out:foo.lib libfoo.obj $ cl -nologo -Febarlib.exe -MD bar.c foo.lib bar.c bar.obj : warning LNK4217: locally defined symbol _variable imported in function _main $ ./barlib $ echo $? 2 Note, I only added the -export options when building the dll as libtool adds those automatically, and I wanted to eliminate as many differences as possible from what libtool is doing. GCC does not have the ability to find "locally defined symbols" when it is looking for imports. > NOTE: Bruno Haible's method worked around this by ALWAYS defining > symbols in a library as declspec(dllimport), when building the library > shared, building the library static, OR building a client. > > BUT...to make linking the DLL itself work (with internal references to > these "dllimported" symbols!), he uses some nm and asm magic to (a) > manually define the __imp_* redirection symbol values and set them to > point to the address of the actual symbol, and (b) explicitly export > each "exported" symbol using an asm .drective, even tho it was marked > dllimport. > > It's really rather clever, but I've never really figured out how to > merge it into the typical auto*/libtool process (Bruno's libiconv and > gettext do it, but with a lot of Makefile.am hackery). Second, I don't > know if it would work with MSVC easily; certainly the asm magic would > need modification. Third, it almost *requires* to build *everything* > with --disable-auto-import. Which would NOT go over well with the > larger community. So, I've never pursued it, and obviously Bruno hasn't > pushed the issue. Note that since libtool does add the -export options when linking the dll, it will work to unconditionally declare (for MSVC) any variables with __declspec(dllimport), and then rely on the -export option when linking to "win" and make it an export. But, I suspect that any references to the variable will go through an indirection, as I suspect will be th
Re: [PATCH] tests: import variables for MSVC.
Charles Wilson wrote: On 9/24/2010 8:06 PM, Roumen Petrov wrote: I would like to propose different macros for export/import of variables in format: #define XXX(type)decorator_before type decorator_after Why? Peter's formula is practically universal in most packages I have seen (ncurses is the only exception that springs to mind). What advantage does using an "unusual" decorator structure bring? It's not as if we're going to, in our demo/test code, start using __stdcall decorator_afters, are we? (Remember that we're only talking about how the test code is structured, not how libtool "requires" client code to be written. Clients would still be free if they chose to, to use XXX(type) macros). -- Chuck Because "#define XXX(type)decorator_before type decorator_after" is easy to be adapted to every compiler: - some will accept both : __export int and int __export - other only support : int __export and so on. For the define XXX(type) check apache source Roumen
Re: [PATCH] tests: import variables for MSVC.
On 9/24/2010 2:46 PM, Peter Rosin wrote: > Now I'm also confused. That's not good. > /me double checks (see below) > > WHAT? It doesn't work as I stated!?! > > *ponders that for a bit* > *scratches head* > > Ahh, you said "libtool does this by default IIRC". If that's actually the > case than that is what has have me fooled for years. Pay close attention to how libtool compiles (the single, only) main.o. Does it get the picflag (-DDLL_EXPORT) or not? As I've always understood it, *without* auto-import magic, you MUST have the following: 1) shared: a) a dll, compiled with declspec(dllexport) [or, create the DLL with an explicit .def file listing the exported symbols] b) the client *must* be compiled with declspec(dllimport) decorators on all symbols the client wants to use from the DLL 2) static a) a lib. Nothing should be compiled with declspec(dllexport), and obviously there is no .def file involved b) the client must NOT be compiled with declspec(dllimport) -- because then you get the missing __imp__foo error. So, your test case below doesn't surprise me. What DOES surprise me, is that it ever worked at all with MSVC (or gcc + -disable-auto-import), since it appears that Ralf is correct and the *_PROGRAM objects are built in only one "mode". Whether that is "picflag" (-DDLL_EXPORT) or not, one or the other linking modality should fail (static linking if main.o is compiled with -DDLL_EXPORT; dynamic linking if main.o is NOT compiled with -DLL_EXPORT). So, yeah, I'm a little confused as well. I think you need to do some archeology on the *_PROGRAM objects (nm -B or whatever the equivalent is in msvc land), and figure out what symbols are undefined. I'd take a hard look at demo/. NOTE: Bruno Haible's method worked around this by ALWAYS defining symbols in a library as declspec(dllimport), when building the library shared, building the library static, OR building a client. BUT...to make linking the DLL itself work (with internal references to these "dllimported" symbols!), he uses some nm and asm magic to (a) manually define the __imp_* redirection symbol values and set them to point to the address of the actual symbol, and (b) explicitly export each "exported" symbol using an asm .drective, even tho it was marked dllimport. It's really rather clever, but I've never really figured out how to merge it into the typical auto*/libtool process (Bruno's libiconv and gettext do it, but with a lot of Makefile.am hackery). Second, I don't know if it would work with MSVC easily; certainly the asm magic would need modification. Third, it almost *requires* to build *everything* with --disable-auto-import. Which would NOT go over well with the larger community. So, I've never pursued it, and obviously Bruno hasn't pushed the issue. > *deep sigh* > > Thanks for setting me straight. > > What now? Is the patch still good? (with a reworded changelog of course) Well, I think so. It's possible we might need a follow-on patch for "strict correctness" -- but this patch appears to be correct as far as it goes. > But now I'm really confused. What made the original patch work? It had > "#define EXTERN extern __declspec(dllimport)" unconditionally for MSVC. > And that patch also had two SKIPs and one FAIL (libfoo.a vs. foo.lib). > I.e. the exact same result, which means I can't be completely wrong. Or > is the testsuite not doing any static builds? But that seems highly > unlikely indeed. WTF? Look really closely at how hell_static.exe is built in demo/. But, to sum up, I see no problems with THIS patch, as far as it goes. With regards to Ralf's question re: _MSC_VER. Well, technically you'd probably be "more" correct to do: #if (defined(_WIN32) || defined(_WIN32_WCE)) && !defined(__GNUC__) ... rather than _MSC_VER; that formula would indicate "any win32 or wince platform, using any compiler EXCEPT gcc" -- because only gcc has "auto-import" on win32. But nobody does that; pretty much all source code with pretensions of both cross-platform use, AND windows support, uses _MSC_VER (*badly* ported code uses _WIN32 to mean "MSVC" which causes no end of cygwin and mingw headaches!). Because of that, IIUC most users of "other" compilers for win32 (Intel C/C++, Watcom, Borland, etc) either (a) routinely s/_MSC_VER/__WATCOMC__/g, or (b) add -D_MSC_VER anyway. -- Chuck
Re: [PATCH] tests: import variables for MSVC.
On 9/24/2010 2:53 PM, Ralf Wildenhues wrote: >> Den 2010-09-24 19:30 skrev Charles Wilson: >>> That is the typical approach. The drawback -- usually an acceptable one >>> -- is that if you are building a "stack" of dependent DLLs: >>> >>> EXE --> C.DLL -> B.DLL >>> --> A.DLL >>> >>> Then (a) you must link exe using .obj's compiled as pic (e.g. with >>> -DDLL_EXPORT, even tho the EXE *itself* is not a "shared library"). >>> libtool does this by default IIRC. > > Huh? But automake won't go this way usually. With > > bin_PROGRAMS = foo > foo_SOURCES = foo.c > foo_LDADD = libc.la > > foo will be linked with foo.o (*not* created by libtool), and neither > foo.lo nor .libs/foo.o will ever be created. Err...maybe you are right. I've been so used to auto-import (and now, even the warnings are suppressed with modern cygwin and mingw gcc/binutils), that I'm just used to it simply working. If I understand the process, the above would fail if libc.la had a shared library, but we linked foo using -disable-auto-import (e.g. or we were talking about MSVC.) More in my reply to Peter's message. -- Chuck
Re: [PATCH] tests: import variables for MSVC.
On 9/24/2010 8:06 PM, Roumen Petrov wrote: > > I would like to propose different macros for export/import of variables > in format: > > #define XXX(type)decorator_before type decorator_after Why? Peter's formula is practically universal in most packages I have seen (ncurses is the only exception that springs to mind). What advantage does using an "unusual" decorator structure bring? It's not as if we're going to, in our demo/test code, start using __stdcall decorator_afters, are we? (Remember that we're only talking about how the test code is structured, not how libtool "requires" client code to be written. Clients would still be free if they chose to, to use XXX(type) macros). -- Chuck
Re: [PATCH] tests: import variables for MSVC.
On 9/24/2010 8:13 PM, Roumen Petrov wrote: > About pre-processor flags - better is C code to start with #define > BUIILD_FOO instead -DBUIILD_FOO in makefile. No, actually, it is not better. The reason is, any given C file *might* be used in a library, or it *might* be used in an application -- or both, depending on compile flags. For instance, suppose you have a utility library, where each function has a built-in self test: int some_util_function() { } #if defined(PACKAGE_FOO_TESTING) int main(int argc, char *argv[]) { } #endif You wouldn't want to unconditionally define BUILDING_LIBUTIL in this case. Now, certainly, you could do some magic like #if !defined(PACKAGE_FOO_TESTING) # define BUILDING_LIBUTIL #endif but...(a) this is a deliberately simple example, and (b) there's a better way. There is *one place* in the package where you KNOW which files are being compiled for inclusion in a library, and which are not: and that's the Makefile (or Makefile.am, or cmakefiles.list, or whatever) -- NOT the C code itself. Why should you duplicate that knowledge in the source code itself? What happens when you refactor a "big" library into multiple, smaller libraries? With the Makefile approach, you simply reassign when .c's go with which libfoo_SOURCES, and each libfoo_la_CFLAGS has a different -DBUILDING_* -- and you don't have to modify any of the .c's at all (you'd have to modify some .h's, but you'd need to do THAT regardless). Your way, this refactoring requires coupled changes in each and every .c file -- because you put "knowledge" (about which library each .c file belongs to -- inside each .c file itself, and that's the wrong place for that knowledge. It *belongs* in the buildsystem (e.g. the Makefile). -- Chuck
Re: [PATCH] tests: import variables for MSVC.
I sent my email before to finish, sorry. Roumen Petrov wrote: Ralf Wildenhues wrote: * Peter Rosin wrote on Fri, Sep 24, 2010 at 02:44:25PM CEST: [SNIP] I'll let Chuck and you hash out and decide all the details on this. One question though: will all of these '#ifdef _MSC_VER' cases need changing as soon as we add support for yet another w32 compiler? In that case, I think the strategy should be reconsidered. The idea is that the sources should ideally not need any, or at least not many, changes in the future. Relying on compiler defines seems like a suboptimal strategy, and should only be done if there is no other way. I would like to propose different macros for export/import of variables in format: #define XXX(type) decorator_before type decorator_after I sent my email before to finish, sorry. Next in code to replace "int foo" with "XXX(int) foo". About pre-processor flags - better is C code to start with #define BUIILD_FOO instead -DBUIILD_FOO in makefile. Roumen
Re: [PATCH] tests: import variables for MSVC.
Ralf Wildenhues wrote: * Peter Rosin wrote on Fri, Sep 24, 2010 at 02:44:25PM CEST: [SNIP] I'll let Chuck and you hash out and decide all the details on this. One question though: will all of these '#ifdef _MSC_VER' cases need changing as soon as we add support for yet another w32 compiler? In that case, I think the strategy should be reconsidered. The idea is that the sources should ideally not need any, or at least not many, changes in the future. Relying on compiler defines seems like a suboptimal strategy, and should only be done if there is no other way. I would like to propose different macros for export/import of variables in format: #define XXX(type)decorator_before type decorator_after [SNIP] Roumen
Re: [PATCH] tests: import variables for MSVC.
* Peter Rosin wrote on Fri, Sep 24, 2010 at 02:44:25PM CEST: > Den 2010-09-24 07:21 skrev Ralf Wildenhues: > > Patch is ok with me if it keeps GCC working, and Chuck is ok with it. > > You meant to use __declspec everywhere not declspec, even in your text > > part of the mail, right? > > Yes indeed, I intended __declspec. I have revised the patch so that it > handles "building" correctly (dllexport for dlls, not for static) and > "using" the best way possible (still dllimports from from both dlls and > static libs). For Cygwin I removed some dead code in tests/pdemo, > similar code was deleted from tests/demo back in 2002 (see commit > 45d16ee8bf4559d6b976bfd4d6482767f16eac95). I have verified that the > Cygwin related cleanup does not affect the Cygwin testsuite results. I'll let Chuck and you hash out and decide all the details on this. One question though: will all of these '#ifdef _MSC_VER' cases need changing as soon as we add support for yet another w32 compiler? In that case, I think the strategy should be reconsidered. The idea is that the sources should ideally not need any, or at least not many, changes in the future. Relying on compiler defines seems like a suboptimal strategy, and should only be done if there is no other way. > With this patch, the old testsuite SKIPs cdemo-undef and tagdemo-undef, > FAILs demo-deplibs(1) and all the rest PASS (on MSYS/MSVC). So it is > looking really nice. Cool. > > A documentation addition describing the most general case of annotations > > (multiple libraries, mixed static/shared, full MSVC + everything else > > support) plus simplifications for common cases, > > - no MSVC, > > - no shared/static mixing, > > - rely on auto-import, > > - ... > > > > would be good to have. Something from which I can deduce that your > > patch must be correct. > > That documentation would be nice, yes, and I plan to write something about > that eventually. Is it a prerequisite for pushing this? No. But alongside the documentation, it would be good to have at least some testsuite exposure for all the code that we recommend. IOW, most of the testsuite now works with MSVC and all, so it ought to follow more or less the most general case of annotations. Then, we should have a couple of tests that simplify based on the assumption that we can rely on auto-import; these tests should be skipped when auto-import is not available, and they are for users whose code needs to rely on it anyway. Then a couple of tests that assume static/shared mixing does not happen. Etc. So that we can rely on our documentation to remain accurate, because we test it in the testsuite. > > Also, may I remind you that you promised a number of testsuite additions > > before the release. > > I have been digging in the archives for quite a bit, but I'm only finding > http://lists.gnu.org/archive/html/libtool-patches/2010-09/msg00266.html > > What else have I promised? Oh, s/you promised/y'all promised/ ;-) Thanks, Ralf
Re: [PATCH] tests: import variables for MSVC.
> Den 2010-09-24 19:30 skrev Charles Wilson: > > That is the typical approach. The drawback -- usually an acceptable one > > -- is that if you are building a "stack" of dependent DLLs: > > > > EXE --> C.DLL -> B.DLL > > --> A.DLL > > > > Then (a) you must link exe using .obj's compiled as pic (e.g. with > > -DDLL_EXPORT, even tho the EXE *itself* is not a "shared library"). > > libtool does this by default IIRC. Huh? But automake won't go this way usually. With bin_PROGRAMS = foo foo_SOURCES = foo.c foo_LDADD = libc.la foo will be linked with foo.o (*not* created by libtool), and neither foo.lo nor .libs/foo.o will ever be created. Or, I am misunderstanding your statement, and going back to lurking mode. Cheers, Ralf
Re: [PATCH] tests: import variables for MSVC.
Den 2010-09-24 19:30 skrev Charles Wilson: > On 9/23/2010 6:25 PM, Peter Rosin wrote: >> I don't know how to set up the defines so that EXTERN becomes >> >> 1. "extern" when you use a static library >> 2. "extern" when you build a static library >> 3. "extern declspec(dllimport)" when you use a shared library >> 4. "extern declspec(dllexport)" when you build a shared library >> >> I could fix 2 and 4, but separating 1 and 3 is not possible. Since >> extern declspec(dllimport) works everywhere with MSVC I'm taking the >> easy option with this patch. >> >> Or should I add -DBUILDING_FOO to Makefile.am and variations of the below >> to the code? > > That is the typical approach. The drawback -- usually an acceptable one > -- is that if you are building a "stack" of dependent DLLs: > > EXE --> C.DLL -> B.DLL > --> A.DLL > > Then (a) you must link exe using .obj's compiled as pic (e.g. with > -DDLL_EXPORT, even tho the EXE *itself* is not a "shared library"). > libtool does this by default IIRC. (b) You MUST link EXE against shared > C.DLL and shared A.DLL; you can't link against static C.lib and B.lib, > but dynamic A.DLL, or vice versa (because DLL_EXPORT, for EXE's objs, > either IS, or IS NOT, defined). Now I'm also confused. /me double checks (see below) WHAT? It doesn't work as I stated!?! *ponders that for a bit* *scratches head* Ahh, you said "libtool does this by default IIRC". If that's actually the case than that is what has have me fooled for years. *deep sigh* Thanks for setting me straight. What now? Is the patch still good? (with a reworded changelog of course) *thinks again* But now I'm really confused. What made the original patch work? It had "#define EXTERN extern __declspec(dllimport)" unconditionally for MSVC. And that patch also had two SKIPs and one FAIL (libfoo.a vs. foo.lib). I.e. the exact same result, which means I can't be completely wrong. Or is the testsuite not doing any static builds? But that seems highly unlikely indeed. WTF? Cheers, Peter $ echo "extern __declspec(dllexport) int variable; int variable = 0;" > dllfoo.c $ cl -nologo -Fefoo.dll -MD dllfoo.c -link -dll -export:variable -implib:foo.dll.lib dllfoo.c Creating library foo.dll.lib and object foo.dll.exp $ echo "extern __declspec(dllimport) int variable; int main(void) { return variable; }" > bar.c $ cl -nologo -Febar.exe -MD bar.c foo.dll.lib bar.c $ ./bar $ echo $? 0 $ echo "extern int variable; int variable = 0;" > libfoo.c $ cl -nologo -c -MD libfoo.c libfoo.c $ lib -nologo -out:foo.lib libfoo.obj $ cl -nologo -Febar.exe -MD bar.c foo.lib bar.c bar.obj : error LNK2019: unresolved external symbol __imp__variable referenced in function _main bar.exe : fatal error LNK1120: 1 unresolved externals
Re: [PATCH] tests: import variables for MSVC.
On 9/24/2010 8:44 AM, Peter Rosin wrote: > Yes indeed, I intended __declspec. I have revised the patch so that it > handles "building" correctly (dllexport for dlls, not for static) and > "using" the best way possible (still dllimports from from both dlls and > static libs). Well, I'm confused. The linker really ought to fail in this case, since dllimport mangles the symbol name IIRC -- and the mangled name is not present in the static lib. > For Cygwin I removed some dead code in tests/pdemo, > similar code was deleted from tests/demo back in 2002 (see commit > 45d16ee8bf4559d6b976bfd4d6482767f16eac95). I have verified that the > Cygwin related cleanup does not affect the Cygwin testsuite results. Always good to know. > With this patch, the old testsuite SKIPs cdemo-undef and tagdemo-undef, > FAILs demo-deplibs(1) and all the rest PASS (on MSYS/MSVC). So it is > looking really nice. That's great. (Still confused, tho). > That documentation would be nice, yes, and I plan to write something about > that eventually. Is it a prerequisite for pushing this? IMO, we should probably document it before 2.4.2... >> Of course, if libtool can somehow help with this any more, so much the >> better. But I'm less optimistic on this than I was those five years >> ago. :-/ > > Yes, and with auto-import in place for gnu tools on w32, the itch is gone > for a whole bunch of people. Well, Bruno Haible still hates auto-import. He has wanted a certain patch in libtool for a long time, but I still don't understand whether doing so would break existing expectations and force everybody to use his method, or if it would basically have no effect for most of us yet enable his method... http://www.haible.de/bruno/woe32dll.html >> Also, may I remind you that you promised a number of testsuite additions >> before the release. > > I have been digging in the archives for quite a bit, but I'm only finding > http://lists.gnu.org/archive/html/libtool-patches/2010-09/msg00266.html > > What else have I promised? I think it was kinda given that the new functionality would need tests (for anything not also covered by existing ones). Maybe manifests ('course, IIRC the end user needs to explicitly set MT before running the testsuite, which is kindof odd). Some of the "promised tests" are on my plate, and relate to non-msvc-specific stuff, which msvc leverages. Patch (as revised) is fine with me. -- Chuck
Re: [PATCH] tests: import variables for MSVC.
On 9/23/2010 6:25 PM, Peter Rosin wrote: > I don't know how to set up the defines so that EXTERN becomes > > 1. "extern" when you use a static library > 2. "extern" when you build a static library > 3. "extern declspec(dllimport)" when you use a shared library > 4. "extern declspec(dllexport)" when you build a shared library > > I could fix 2 and 4, but separating 1 and 3 is not possible. Since > extern declspec(dllimport) works everywhere with MSVC I'm taking the > easy option with this patch. > > Or should I add -DBUILDING_FOO to Makefile.am and variations of the below > to the code? That is the typical approach. The drawback -- usually an acceptable one -- is that if you are building a "stack" of dependent DLLs: EXE --> C.DLL -> B.DLL --> A.DLL Then (a) you must link exe using .obj's compiled as pic (e.g. with -DDLL_EXPORT, even tho the EXE *itself* is not a "shared library"). libtool does this by default IIRC. (b) You MUST link EXE against shared C.DLL and shared A.DLL; you can't link against static C.lib and B.lib, but dynamic A.DLL, or vice versa (because DLL_EXPORT, for EXE's objs, either IS, or IS NOT, defined). We already enforce the restriction that C.DLL can't depend on B.lib, so that "complication" is a non-issue. > #ifdef _MSC_VER > # ifdef BUILDING_FOO > # ifdef DLL_EXPORT > # define EXTERN extern declspec(dllexport) > # endif > # else > # define EXTERN extern declspec(dllimport) > # endif > #endif > #ifndef EXTERN > # define EXTERN extern > #endif More indepth review against your revised version. -- Chuck
Re: [PATCH] tests: import variables for MSVC.
Hi Ralf, Den 2010-09-24 07:21 skrev Ralf Wildenhues: > * Peter Rosin wrote on Fri, Sep 24, 2010 at 12:25:14AM CEST: >> Subject: [PATCH] tests: import variables for MSVC. >> >> * tests/depdemo/sysdep.h (EXTERN): New define, saying how to >> declare variables that might need to be imported. >> * tests/depdemo/l1/l1.h, tests/depdemo/l2/l2.h, >> tests/depdemo/l3/l3.h, tests/depdemo/l4/l4.h: Use it. >> * tests/demo/foo.h (EXTERN): New define, saying how to declare >> variables that might need to be imported. Use it. >> * tests/pdemo/foo.h (EXTERN) [MSVC]: Make MSVC import variables >> if needed. > > Patch is ok with me if it keeps GCC working, and Chuck is ok with it. > You meant to use __declspec everywhere not declspec, even in your text > part of the mail, right? Yes indeed, I intended __declspec. I have revised the patch so that it handles "building" correctly (dllexport for dlls, not for static) and "using" the best way possible (still dllimports from from both dlls and static libs). For Cygwin I removed some dead code in tests/pdemo, similar code was deleted from tests/demo back in 2002 (see commit 45d16ee8bf4559d6b976bfd4d6482767f16eac95). I have verified that the Cygwin related cleanup does not affect the Cygwin testsuite results. With this patch, the old testsuite SKIPs cdemo-undef and tagdemo-undef, FAILs demo-deplibs(1) and all the rest PASS (on MSYS/MSVC). So it is looking really nice. 1 of 104 tests failed (2 tests were not run) See ./test-suite.log Please report to bug-libtool_gnu.org (1) http://lists.gnu.org/archive/html/automake/2010-09/msg00063.html > A documentation addition describing the most general case of annotations > (multiple libraries, mixed static/shared, full MSVC + everything else > support) plus simplifications for common cases, > - no MSVC, > - no shared/static mixing, > - rely on auto-import, > - ... > > would be good to have. Something from which I can deduce that your > patch must be correct. That documentation would be nice, yes, and I plan to write something about that eventually. Is it a prerequisite for pushing this? > Of course, if libtool can somehow help with this any more, so much the > better. But I'm less optimistic on this than I was those five years > ago. :-/ Yes, and with auto-import in place for gnu tools on w32, the itch is gone for a whole bunch of people. > Also, may I remind you that you promised a number of testsuite additions > before the release. I have been digging in the archives for quite a bit, but I'm only finding http://lists.gnu.org/archive/html/libtool-patches/2010-09/msg00266.html What else have I promised? Cheers, Peter >From d51a0c0fe3a77fb186aa331088414b4a9304e333 Mon Sep 17 00:00:00 2001 From: Peter Rosin Date: Fri, 24 Sep 2010 14:38:21 +0200 Subject: [PATCH] tests: clean up importing and exporting on w32. Makes the touched tests pass for MSVC when DLLs are built. * tests/demo/Makefile.am, tests/pdemo/Makefile.am: Define BUILDING_LIBHELLO when building libhello.la. * tests/demo/foo.h, tests/pdemo/foo.h (nothing) : Export variable when building the libhello dll and import when using libhello. For non-MSVC and when building a static libhello, leave as an ordinary extern. * tests/pdemo/foo.h [Cygwin]: Remove unneeded and "dead" export and import logic (LIBFOO_DLL is always undefined). * tests/pdemo/longer_file_name_foo.c, tests/pdemo/longer_file_name_foo2.c (_LIBFOO_COMPILATION_): Not useful before, even less so now. Removed. * tests/depdemo/l1/Makefile.am: Define BUILDING_LIBL1 when building libl1.la. * tests/depdemo/l2/Makefile.am: Define BUILDING_LIBL2 when building libl2.la. * tests/depdemo/l3/Makefile.am: Define BUILDING_LIBL3 when building libl3.la. * tests/depdemo/l4/Makefile.am: Define BUILDING_LIBL4 when building libl4.la. * tests/depdemo/l1/l1.h, tests/depdemo/l2/l2.h, tests/depdemo/l3/l3.h, tests/depdemo/l4/l4.h : Export variables when building the associated library dll and import when using the library. For non-MSVC and when building static libraries, leave as an ordinary extern. Signed-off-by: Peter Rosin --- ChangeLog | 29 + tests/demo/Makefile.am |3 ++- tests/demo/foo.h| 15 ++- tests/depdemo/l1/Makefile.am|4 ++-- tests/depdemo/l1/l1.h | 17 +++-- tests/depdemo/l2/Makefile.am|4 ++-- tests/depdemo/l2/l2.h | 17 +++-- tests/depdemo/l3/Makefile.am|4 ++-- tests/depdemo/l3/l3.h | 17 +++-- tests/depdemo/l4/Makefile.am|4 ++-- tests/depdemo/l4/l4.h | 17 +++-- tests/pdemo/Makefile.am |
Re: [PATCH] tests: import variables for MSVC.
Hi Peter, * Peter Rosin wrote on Fri, Sep 24, 2010 at 12:25:14AM CEST: > Subject: [PATCH] tests: import variables for MSVC. > > * tests/depdemo/sysdep.h (EXTERN): New define, saying how to > declare variables that might need to be imported. > * tests/depdemo/l1/l1.h, tests/depdemo/l2/l2.h, > tests/depdemo/l3/l3.h, tests/depdemo/l4/l4.h: Use it. > * tests/demo/foo.h (EXTERN): New define, saying how to declare > variables that might need to be imported. Use it. > * tests/pdemo/foo.h (EXTERN) [MSVC]: Make MSVC import variables > if needed. Patch is ok with me if it keeps GCC working, and Chuck is ok with it. You meant to use __declspec everywhere not declspec, even in your text part of the mail, right? A documentation addition describing the most general case of annotations (multiple libraries, mixed static/shared, full MSVC + everything else support) plus simplifications for common cases, - no MSVC, - no shared/static mixing, - rely on auto-import, - ... would be good to have. Something from which I can deduce that your patch must be correct. Of course, if libtool can somehow help with this any more, so much the better. But I'm less optimistic on this than I was those five years ago. :-/ Also, may I remind you that you promised a number of testsuite additions before the release. Thanks, Ralf
Re: [PATCH] tests: import variables for MSVC.
On Fri, 24 Sep 2010, Peter Rosin wrote: Hi! I don't know how to set up the defines so that EXTERN becomes 1. "extern" when you use a static library 2. "extern" when you build a static library 3. "extern declspec(dllimport)" when you use a shared library 4. "extern declspec(dllexport)" when you build a shared library I could fix 2 and 4, but separating 1 and 3 is not possible. Since extern declspec(dllimport) works everywhere with MSVC I'm taking the easy option with this patch. Or should I add -DBUILDING_FOO to Makefile.am and variations of the below to the code? #ifdef _MSC_VER # ifdef BUILDING_FOO # ifdef DLL_EXPORT # define EXTERN extern declspec(dllexport) # endif # else # define EXTERN extern declspec(dllimport) # endif #endif #ifndef EXTERN # define EXTERN extern #endif here is what I use: http://trac.enlightenment.org/e/browser/trunk/PROTO/evil/src/lib/Evil.h#L7 It's what you do, with just an additional else in DLL_EXPORT case, without your #ifndef. works fine with gcc (mingw, mingw-w64, mingw32ce) and vc++ hth Vincent Torri
[PATCH] tests: import variables for MSVC.
Hi! This is a repost of an old patch. Ralf said a long time ago that I should drop the fixes for the old testsuite, but since I'm short of problems in the new testsuite and the old one isn't going away, I'm revisiting this. This was first posted here as part of a much bigger patch: http://lists.gnu.org/archive/html/libtool-patches/2005-07/msg00033.html Ralf didn't like it here: http://lists.gnu.org/archive/html/libtool-patches/2005-08/msg00069.html Some words from me on the subject: http://lists.gnu.org/archive/html/libtool-patches/2005-08/msg00076.html I don't know how to set up the defines so that EXTERN becomes 1. "extern" when you use a static library 2. "extern" when you build a static library 3. "extern declspec(dllimport)" when you use a shared library 4. "extern declspec(dllexport)" when you build a shared library I could fix 2 and 4, but separating 1 and 3 is not possible. Since extern declspec(dllimport) works everywhere with MSVC I'm taking the easy option with this patch. Or should I add -DBUILDING_FOO to Makefile.am and variations of the below to the code? #ifdef _MSC_VER # ifdef BUILDING_FOO # ifdef DLL_EXPORT # define EXTERN extern declspec(dllexport) # endif # else # define EXTERN extern declspec(dllimport) # endif #endif #ifndef EXTERN # define EXTERN extern #endif Cheers, Peter >From 044c0e8de4c98da753e7b17d87f03c8eebe2bcbe Mon Sep 17 00:00:00 2001 From: Peter Rosin Date: Thu, 23 Sep 2010 22:55:04 +0200 Subject: [PATCH] tests: import variables for MSVC. * tests/depdemo/sysdep.h (EXTERN): New define, saying how to declare variables that might need to be imported. * tests/depdemo/l1/l1.h, tests/depdemo/l2/l2.h, tests/depdemo/l3/l3.h, tests/depdemo/l4/l4.h: Use it. * tests/demo/foo.h (EXTERN): New define, saying how to declare variables that might need to be imported. Use it. * tests/pdemo/foo.h (EXTERN) [MSVC]: Make MSVC import variables if needed. Signed-off-by: Peter Rosin --- ChangeLog | 12 tests/demo/foo.h |8 +++- tests/depdemo/l1/l1.h |2 +- tests/depdemo/l2/l2.h |2 +- tests/depdemo/l3/l3.h |2 +- tests/depdemo/l4/l4.h |2 +- tests/depdemo/sysdep.h |6 ++ tests/pdemo/foo.h |2 ++ 8 files changed, 31 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index 647c151..af99b0c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2010-09-23 Peter Rosin + + tests: import variables for MSVC. + * tests/depdemo/sysdep.h (EXTERN): New define, saying how to + declare variables that might need to be imported. + * tests/depdemo/l1/l1.h, tests/depdemo/l2/l2.h, + tests/depdemo/l3/l3.h, tests/depdemo/l4/l4.h: Use it. + * tests/demo/foo.h (EXTERN): New define, saying how to declare + variables that might need to be imported. Use it. + * tests/pdemo/foo.h (EXTERN) [MSVC]: Make MSVC import variables + if needed. + 2010-09-22 Ralf Wildenhues Fix regression in command-line length computation. diff --git a/tests/demo/foo.h b/tests/demo/foo.h index 167096a..b0a1a47 100644 --- a/tests/demo/foo.h +++ b/tests/demo/foo.h @@ -37,6 +37,12 @@ or obtained by writing to the Free Software Foundation, Inc., # endif #endif +#ifdef _MSC_VER +# define EXTERN extern __declspec(dllimport) +#else +# define EXTERN extern +#endif + /* __BEGIN_DECLS should be used at the beginning of your declarations, so that C++ compilers don't mangle their names. Use __END_DECLS at the end of C declarations. */ @@ -83,7 +89,7 @@ or obtained by writing to the Free Software Foundation, Inc., __BEGIN_DECLS int foo LT_PARAMS((void)); int hello LT_PARAMS((void)); -extern int nothing; +EXTERN int nothing; __END_DECLS #endif /* !_FOO_H_ */ diff --git a/tests/depdemo/l1/l1.h b/tests/depdemo/l1/l1.h index 8e773ca..47f7be0 100644 --- a/tests/depdemo/l1/l1.h +++ b/tests/depdemo/l1/l1.h @@ -30,7 +30,7 @@ or obtained by writing to the Free Software Foundation, Inc., #include "sysdep.h" __BEGIN_DECLS -extern int var_l1; +EXTERN int var_l1; intfunc_l1 __P((int)); __END_DECLS diff --git a/tests/depdemo/l2/l2.h b/tests/depdemo/l2/l2.h index 728b1df..500049d 100644 --- a/tests/depdemo/l2/l2.h +++ b/tests/depdemo/l2/l2.h @@ -30,7 +30,7 @@ or obtained by writing to the Free Software Foundation, Inc., #include "sysdep.h" __BEGIN_DECLS -extern int var_l2; +EXTERN int var_l2; intfunc_l2 __P((int)); __END_DECLS diff --git a/tests/depdemo/l3/l3.h b/tests/depdemo/l3/l3.h index 6f0b446..59eff6d 100644 --- a/tests/depdemo/l3/l3.h +++ b/tests/depdemo/l3/l3.h @@ -30,7 +30,7 @@ or obtained by writing to the Free Software Foundation, Inc., #include "sysdep.h" __BEGIN_DECLS -extern int var_l3; +EXTERN int var_l3; intfunc_l3 __P((int)); __END_DECLS diff --git a/tests/depdemo/l4/l4.h b/tests/depdemo/l4/l4.h index bdf2