Re: [PATCH] tests: import variables for MSVC.

2010-09-27 Thread Peter Rosin
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 the case when the library consumer is using __declspec(dllimport)
even when linking statically.


Re: [PATCH] tests: import variables for MSVC.

2010-09-27 Thread Peter Rosin
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.

2010-09-25 Thread Roumen Petrov

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.

2010-09-24 Thread Peter Rosin
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 p...@lysator.liu.se
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) MSVC: 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 MSVC: 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 p...@lysator.liu.se
---
 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

Re: [PATCH] tests: import variables for MSVC.

2010-09-24 Thread 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).

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.

2010-09-24 Thread Charles Wilson
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.

2010-09-24 Thread Peter Rosin
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.

2010-09-24 Thread Ralf Wildenhues
 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.

2010-09-24 Thread Ralf Wildenhues
* 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.

2010-09-24 Thread Roumen Petrov

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.

2010-09-24 Thread Charles Wilson
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.

2010-09-24 Thread Charles Wilson
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.

2010-09-24 Thread Charles Wilson
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.

2010-09-24 Thread 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/.


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



[PATCH] tests: import variables for MSVC.

2010-09-23 Thread Peter Rosin
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 p...@lysator.liu.se
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 p...@lysator.liu.se
---
 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  p...@lysator.liu.se
+
+   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  ralf.wildenh...@gmx.de
 
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 bdf21d0..a78be74 100644
--- a/tests/depdemo/l4/l4.h

Re: [PATCH] tests: import variables for MSVC.

2010-09-23 Thread Vincent Torri



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



Re: [PATCH] tests: import variables for MSVC.

2010-09-23 Thread Ralf Wildenhues
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