Re: [SCM] GNU Libtool branch, master, updated. v2.4-1-gf0ba93d
Hallo Ralf, On 23 Sep 2010, at 00:30, Ralf Wildenhues wrote: > * Gary V. Vaughan wrote on Wed, Sep 22, 2010 at 06:27:27PM CEST: >>* libltdl/Makefile.inc (LTDL_VERSION_INFO): We've added the >>static libprefix interface, so new version-info is C+1:0:R+1. > > libprefix is a *static* local undocumented variable, not public API. > There was no reason to bump the API version for this. :-( Ugh. Sorry. Luckily there are still quite a lot of numbers left before we run out. I propose that to avoid this problem with future releases, that whoever commits a change that *does* affect LTDL_VERSION_INFO notes it in NEWS so that I don't make another mistake when I'm searching back up ChangeLog from the previous release commit to find things that affect the API versioning. If you agree, I'll add a note to HACKING. Cheers, -- Gary V. Vaughan (g...@gnu.org) PGP.sig Description: This is a digitally signed message part
Re: [PATCH] Skip need_lib_prefix.at on systems without lib prefix on libraries.
Hi Ralf, Den 2010-09-24 06:20 skrev Ralf Wildenhues: > Hello Peter, > > * Peter Rosin wrote on Fri, Sep 17, 2010 at 08:44:43AM CEST: >> need_lib_prefix.at currently fails with MSVC. I think the test >> is there to ensure that "weird" systems continue to work even >> if the testsuite is running on a "normal" system. "weird" in >> this case are systems with need_lib_prefix set to "unknown" and >> "normal" are those with it set to "no". However, there are >> even weirder systems where need_lib_prefix should perhaps be >> set to "never" (i.e. MSVC) but that currently simply sets it >> to "no". "never" would perhaps be more appropriate since preopen >> doesn't work right if libs have a lib prefix. I think OS/2 is >> affected in the same way as MSVC, but I have no means to test >> that. >> >> The below patch makes the need_lib_prefix.at test skip for the >> even weirder systems, i.e. those where libname_spec does not >> prefix library names with lib. >> >> Ok to push? >> >> >> You may want to compare this patch with thread >> >> http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg00174.html >> >> which instead makes the test pass for the even weirder systems, >> but I don't think that is really desired. Why should the code be >> changed to accommodate a contrived test case? Because this would >> never happen in the wild, right? > > The part about this patch which I'm unsure about is this: > > Does the testsuite otherwise cover well enough the fact that users may > name their modules with or without leading 'lib' prefix (and with .la or > .dll or .so suffix or so)? > > IOW, I'd like to be sure we're not hiding anything here. But that is not a problem with *this* patch. That's one of those gigantic tasks that Chuck mentions from time to time. This is not like the low max_cmd_len test. In both cases the libtool script is rigged to simulate weird conditions, but the need_lib_prefix test is rigging something that never happens on platforms that never create a lib prefix. You should also not confuse this prefix with the name of the .la file, the .la files are always allowed to have a lib prefix, this is about the real libraries. We have plenty of tests that create -modules named module.la without the prefix, for example dlloader-api.la. I'm not sure what you mean by users naming their modules with various suffixes, as they must be named .la? I get the feeling that I'm saying things that you already know, so I'm probably missing something. What? > And yes, I think (part of) the log entry from the initial test addition > probably deserves to be a comment in the test, so we don't have to look > it up again. Probably a good idea. I'll add some words before pushing anything, but I'd like this settled before doing anything further with the patch. Cheers, Peter
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 |3 ++- tests/pdemo/foo.h | 27 +++ tests/pdemo/longer_file_name_foo.c |4 +--- tests/pdemo/longer_file_name_foo2.c
pre-release update of LTDL_VERSION_INFO (was: [SCM] GNU Libtool branch, master, updated. v2.4-1-gf0ba93d)
Hello Gary, * Gary V. Vaughan wrote on Fri, Sep 24, 2010 at 10:12:10AM CEST: > On 23 Sep 2010, at 00:30, Ralf Wildenhues wrote: > > * Gary V. Vaughan wrote on Wed, Sep 22, 2010 at 06:27:27PM CEST: > >>* libltdl/Makefile.inc (LTDL_VERSION_INFO): We've added the > >>static libprefix interface, so new version-info is C+1:0:R+1. > > > > libprefix is a *static* local undocumented variable, not public API. > > There was no reason to bump the API version for this. :-( > > Ugh. Sorry. Luckily there are still quite a lot of numbers left > before we run out. That's not the point. The point is that on systems which compute the major version as CURRENT rather than CURRENT - AGE, this bump means that all dependent libraries need to be rebuilt. For the respective distribution packagers, I expect this to be several hours of extra work. This affects for example FreeBSD and systems derived from it. Incompatible changes (bumping CURRENT) are *very* costly for distributions, and more importantly, they typically hurt the acceptance rate of the software. > I propose that to avoid this problem with future releases, that > whoever commits a change that *does* affect LTDL_VERSION_INFO notes > it in NEWS so that I don't make another mistake when I'm searching > back up ChangeLog from the previous release commit to find things > that affect the API versioning. > > If you agree, I'll add a note to HACKING. How about this alternative: the person doing the release posts the proposed patch to bump the version info for public review, it gets properly reviewed, then it gets committed? If you agree, I'm fine if that is documented in HACKING. The rationale for this approach is that it is easy to forget this requirement during development, both as developer and as reviewer, and it is less work to overlook all past changes at one time during the release. Of course API changes, compatible or not, should still be announced in NEWS, but they weren't this time, because there were none. It's part of release management to check and ensure that this is indeed correct. Thanks, Ralf
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] Skip need_lib_prefix.at on systems without lib prefix on libraries.
Hi Peter,
* Peter Rosin wrote on Fri, Sep 24, 2010 at 11:30:07AM CEST:
> Den 2010-09-24 06:20 skrev Ralf Wildenhues:
> > The part about this patch which I'm unsure about is this:
> >
> > Does the testsuite otherwise cover well enough the fact that users may
> > name their modules with or without leading 'lib' prefix (and with .la or
> > .dll or .so suffix or so)?
> >
> > IOW, I'd like to be sure we're not hiding anything here.
>
> But that is not a problem with *this* patch. That's one of those gigantic
> tasks that Chuck mentions from time to time.
>
> This is not like the low max_cmd_len test. In both cases the libtool
> script is rigged to simulate weird conditions, but the need_lib_prefix
> test is rigging something that never happens on platforms that never
> create a lib prefix. You should also not confuse this prefix with the
> name of the .la file, the .la files are always allowed to have a lib
> prefix, this is about the real libraries.
Ah, ok.
> We have plenty of tests that create -modules named module.la without the
> prefix, for example dlloader-api.la. I'm not sure what you mean by users
> naming their modules with various suffixes, as they must be named .la?
No, they don't. On GNU/Linux, you ought to be able to, say,
lt_dlopen("foo.so", ...)
if you like. There are users of libltdl that do this. Of course, that
requires the users to be aware of the system-specific naming issues, but
ideally, some way like this should work on other systems, too.
> I get the feeling that I'm saying things that you already know, so I'm
> probably missing something. What?
I don't think you are, apart from the above.
> > And yes, I think (part of) the log entry from the initial test addition
> > probably deserves to be a comment in the test, so we don't have to look
> > it up again.
>
> Probably a good idea. I'll add some words before pushing anything, but
> I'd like this settled before doing anything further with the patch.
In light of your response, and if my response above doesn't invalidate
your reasoning, the patch is ok with me, with that comment added.
Thanks,
Ralf
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.
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.
> 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.
* 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.
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.
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.
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.
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 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 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
