Re: [patch #6448] [MSVC 7/7] Add MSVC Support
Ralf Wildenhues skrev: * Peter Rosin wrote on Wed, Aug 06, 2008 at 09:47:28AM CEST: Ralf Wildenhues skrev: * Peter Rosin wrote on Tue, Aug 05, 2008 at 10:38:14AM CEST: Peter Rosin skrev: 16: duplicate_conv.at:25 duplicate convenience archive names MS link doesn't have reloadable objects (i.e. like ld -r). Should be fixed by at-file support I hope. Unfortunately not, the test tries to link with -o cee.$OBJEXT which triggers some different code path using -r. I don't think's it's possible to have link.exe output an object file. Ah yes, I didn't look at the test. In this case, the test should just be skipped for systems where reload_cmds does not work. Speaking of which, I think you should set reload_cmds to 'false' or so, so that it's clear things fail. That could then be tested for as a SKIP criterion in the testsuite. Right, I'll prepare a patch later... Side note, the mainfest could perhaps be used to support some version of hardcode_libdir? I guess. Patches welcome, as they say. ;-) 30: static.at:357 ccache -all-static cl.exe spews out a banner on stderr which isn't [ignore]d I think that banner should just be ignored. Me too. Is the patch below sufficient? Yes, please push. Thanks! 46: lt_dladvise.at:28 lt_dlopenadvise library loading -avoid-version causes the names of the import lib and the static lib to be the same. But something elseTM also seems bad... Several issues: - the test is somewhat broken (also on other systems) - maybe we need to forbid enabling both static and shared at the same time - Or come up with a naming scheme that doesn't conflict with itself, but I don't know if that's possible... Not sure whether I understand this. With MSVC and -avoid-version, libtool will create both the import library and the old (static) libarary with the same name (at least when both static and shared are enabled at the same time). The current naming is: Shared library: foo-version.dll Import library: foo-version.lib Static library: foo.lib But naming the import library e.g. foo-imp.lib or something (the -version part isn't really needed), it wouldn't collapse into the name of the static lib when the -avoid-version option is given. But just tagging on -imp to the name will of course conflict with any library name ending with -imp. Don't know if that's a real problem... But pdemo still fails due to an exported variable. Sigh. Why does each and every damn test (well, almost) have to export a variable when that's listed as not portable in the docs? Because it can be done on most systems, and w32 is not supported anyway for many packages. Seems masochistic to me... Well, the testsuite is intended to test each possible code path. That doesn't mean things have to be tested more often than necessary, but there could be for example a bug with exporting variables in the ltmain code that is exercises by pdemo. It's just very hard to see other failures when so much stumbles on this issue. From my point of view, you may push the ltmain part of the patch. I'll test any improved m4 code when you have settled on how to enable it. However, your idea to check the help output will not work, dumpbin -? doesn't indicate any support whatsoever for @. Oh, I didn't mean to try 'dumpbin -?' Sorry, I should have been clearer: GNU binutils nm understands @FILE since fairly recently; more precisely, all binutils tools understand this syntax since version 2.17. It would be nice to also enable this for binutils. That would enable most real-world uses of 'ld -r' for good. Proposed patch below. However, we need to avoid enabling this for older GNU binutils. That could be done with a feature check or a check of --help output. Probably a feature check is more reliable. Part of my confusion here stemmed from the fact that, while on unixy systems, at-file support is a per-tool thing, on Windows it is actually a system feature (at least I think that it is that way). No, it's not a system feature as such, it's just that many tools implement it. My guess is that it is needed since the shell quoting is so b0rked :-) Proposed patch below. I changed my mind again and moved the nm_file_list_spec setting out from LT_PATH_NM. OK? Looks perfectly reasonable, but I'd change fi;if into elif, no need to run $NM --help when nm_file_list_spec is already '@'. But I don't care all that much... So, please push with or w/o elif. Cheers, Peter
Re: [patch #6448] [MSVC 7/7] Add MSVC Support
Ralf Wildenhues skrev: - (eval $AR -NOLOGO -OUT:conftest.lib conftest.$ac_objext conftest.err) + (eval $AR -NOLOGO -OUT:conftest.lib conftest.$ac_objext conftest.err 21) Hi Ralf, Is there a reason for this, I thought the log was there to help diagnose what went wrong, and that more information is better? Granted, the above cmd will produce a number of perhaps annoying lines of usage instructions on stderr on most systems. But hey, compare that to tossing the error message when you have a lib.exe that unexpectedly fails. All that said, I realize that lib.exe is fairly exotic, and I understand if you don't want the ar usage in the log, I just wanted to highlight that this is not the only thing you are cutting out. Cheers, Peter
Re: [patch #6448] [MSVC 7/7] Add MSVC Support
* Peter Rosin wrote on Wed, Aug 13, 2008 at 11:00:33AM CEST: Ralf Wildenhues skrev: - (eval $AR -NOLOGO -OUT:conftest.lib conftest.$ac_objext conftest.err) + (eval $AR -NOLOGO -OUT:conftest.lib conftest.$ac_objext conftest.err 21) Is there a reason for this, Yes, file 21 is completely equivalent to file except that the latter syntax is bash-specific and the former supported by all POSIX-compatible shells. I was testing with dash last night and saw ugly noise on (undirected) stderr when running configure. Cheers, Ralf
Re: [patch #6448] [MSVC 7/7] Add MSVC Support
Ralf Wildenhues skrev: * Peter Rosin wrote on Wed, Aug 13, 2008 at 11:00:33AM CEST: Ralf Wildenhues skrev: - (eval $AR -NOLOGO -OUT:conftest.lib conftest.$ac_objext conftest.err) + (eval $AR -NOLOGO -OUT:conftest.lib conftest.$ac_objext conftest.err 21) Is there a reason for this, Yes, file 21 is completely equivalent to file except that the latter syntax is bash-specific and the former supported by all POSIX-compatible shells. I was testing with dash last night and saw ugly noise on (undirected) stderr when running configure. Oops, and *blush* :-) Cheers, Peter
Re: [patch #6448] [MSVC 7/7] Add MSVC Support
Peter Rosin skrev: Ralf Wildenhues skrev: * Peter Rosin wrote on Wed, Aug 06, 2008 at 09:47:28AM CEST: Ralf Wildenhues skrev: * Peter Rosin wrote on Tue, Aug 05, 2008 at 10:38:14AM CEST: Peter Rosin skrev: 16: duplicate_conv.at:25 duplicate convenience archive names MS link doesn't have reloadable objects (i.e. like ld -r). Should be fixed by at-file support I hope. Unfortunately not, the test tries to link with -o cee.$OBJEXT which triggers some different code path using -r. I don't think's it's possible to have link.exe output an object file. Ah yes, I didn't look at the test. In this case, the test should just be skipped for systems where reload_cmds does not work. Speaking of which, I think you should set reload_cmds to 'false' or so, so that it's clear things fail. That could then be tested for as a SKIP criterion in the testsuite. Right, I'll prepare a patch later... Attached... 2008-08-13 Peter Rosin [EMAIL PROTECTED] * libltdl/m4/libtool.m4 (_LT_LINKER_SHLIBS) [cygwin, mingw, pw32, cegcc] cl*: Indicate that reloadable objects does not work. * tests/duplicate_conv.at: Skip last test if reloadable objects does not work. Cheers, Peter diff --git a/libltdl/m4/libtool.m4 b/libltdl/m4/libtool.m4 index 7ceec66..5149a45 100644 --- a/libltdl/m4/libtool.m4 +++ b/libltdl/m4/libtool.m4 @@ -4821,6 +4821,7 @@ _LT_EOF mt -manifest @[EMAIL PROTECTED] -outputresource:@[EMAIL PROTECTED]; $RM @[EMAIL PROTECTED]; fi' + reload_cmds=false ;; *) # Assume MSVC wrapper diff --git a/tests/duplicate_conv.at b/tests/duplicate_conv.at index ac5643b..be04005 100644 --- a/tests/duplicate_conv.at +++ b/tests/duplicate_conv.at @@ -25,6 +25,8 @@ AT_SETUP([duplicate convenience archive names]) AT_KEYWORDS([libtool]) +eval `$LIBTOOL --config | sed -n '/^reload_cmds=/,/^$/p'` + # We create two convenience archives with the same name, and _also_ # containing an object with the same name. This is necessary to detect # the failure with both 1.5.22 and HEAD, since the latter does not (did @@ -75,6 +77,8 @@ AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -o main main.$OBJEXT ./libce LT_AT_EXEC_CHECK([./main],[0],[ignore],[ignore]) $LIBTOOL --mode=clean rm -f libcee.la +AT_CHECK([test x$reload_cmds = xfalse exit 77], [1]) + # Test whether this works with reloadable objects as well. AT_CHECK([$LIBTOOL --mode=link --tag=CC $CC $CFLAGS $LDFLAGS -o cee.$OBJEXT c.lo a/liba.la b/liba.la], [0], [ignore], [ignore])
Re: [patch #6448] [MSVC 7/7] Add MSVC Support
Peter Rosin skrev:
Peter Rosin skrev:
Attached, I'll work through all the failures to try to find out why
they fail...
*snip*
72: stresstest.at:31 Link option thorough search test
Automatic path conversion in MSYS doesn't kick in for the argument
-OUT:/some/absolute/path so lib.exe barfs.
Commenting out absolute paths from the stress test reveals a couple of
other problems...
First, when l3='-rpath /nonexistent' and st='-no-install', I see no
reason for linking main using main-static.lo, so clear the mst variable
for this case.
So, fixing that, like this:
2008-08-13 Peter Rosin [EMAIL PROTECTED]
* tests/stresstest.at: Link with main.lo when liba is shared
and linking main with -no-install.
Second, there are the inevitable variables that need importing, like
this:
2008-08-13 Peter Rosin [EMAIL PROTECTED]
* tests/stresstest.at [MSVC]: dllimport all imported
variables.
Cheers,
Peter
diff --git a/tests/stresstest.at b/tests/stresstest.at
index 27e7ee9..8eadf10 100644
--- a/tests/stresstest.at
+++ b/tests/stresstest.at
@@ -253,7 +253,8 @@ do
for st in '' '-static' '-no-install'
do
case $st,$l3 in
- ,-rpath*) mst= ;;
+ -static,*) mst=-static ;;
+ *,-rpath*) mst= ;;
*) mst=-static ;;
esac
diff --git a/tests/stresstest.at b/tests/stresstest.at
index 27e7ee9..8eadf10 100644
--- a/tests/stresstest.at
+++ b/tests/stresstest.at
@@ -93,29 +93,35 @@ AT_DATA(main.c,
#if defined(LIBA_DLL_IMPORT)
# if defined(_WIN32) || defined(WIN32) || defined(__CYGWIN__)
#define LIBA_SCOPE extern __declspec(dllimport)
+#if defined(_MSC_VER)
+# define LIBA_SCOPE_VAR LIBA_SCOPE
+#endif
# endif
#endif
#if !defined(LIBA_SCOPE)
# define LIBA_SCOPE extern
#endif
+#if !defined(LIBA_SCOPE_VAR)
+# define LIBA_SCOPE_VAR extern
+#endif
#ifdef __cplusplus
extern C {
#endif
-extern int v1;
-extern int v3, v4;
+LIBA_SCOPE_VAR int v1;
+LIBA_SCOPE_VAR int v3, v4;
LIBA_SCOPE const int v5, v6;
-extern const char* v7;
-extern const char v8[];
-extern int v9(void);
-extern int (*v10) (void);
-extern int (*v11) (void);
+LIBA_SCOPE_VAR const char* v7;
+LIBA_SCOPE_VAR const char v8[];
+LIBA_SCOPE_VAR int v9(void);
+LIBA_SCOPE_VAR int (*v10) (void);
+LIBA_SCOPE_VAR int (*v11) (void);
LIBA_SCOPE int (*const v12) (void);
#ifdef __cplusplus
}
#endif
typedef struct { int arr[1000]; } large;
-extern large v13, v14, v15;
+LIBA_SCOPE_VAR large v13, v14, v15;
int main(void)
{
@@ -131,26 +137,32 @@ AT_DATA(dlself.c,
#if defined(LIBA_DLL_IMPORT)
# if defined(_WIN32) || defined(WIN32) || defined(__CYGWIN__)
#define LIBA_SCOPE extern __declspec(dllimport)
+#if defined(_MSC_VER)
+# define LIBA_SCOPE_VAR LIBA_SCOPE
+#endif
# endif
#endif
#if !defined(LIBA_SCOPE)
# define LIBA_SCOPE extern
#endif
+#if !defined(LIBA_SCOPE_VAR)
+# define LIBA_SCOPE_VAR extern
+#endif
#ifdef __cplusplus
extern C {
#endif
-extern int v1;
-extern int v3, v4;
+LIBA_SCOPE_VAR int v1;
+LIBA_SCOPE_VAR int v3, v4;
LIBA_SCOPE const int v5, v6;
-extern const char* v7;
-extern const char v8[];
-extern int v9(void);
-extern int (*v10) (void);
-extern int (*v11) (void);
+LIBA_SCOPE_VAR const char* v7;
+LIBA_SCOPE_VAR const char v8[];
+LIBA_SCOPE_VAR int v9(void);
+LIBA_SCOPE_VAR int (*v10) (void);
+LIBA_SCOPE_VAR int (*v11) (void);
LIBA_SCOPE int (*const v12) (void);
typedef struct { int arr[1000]; } large;
-extern large v13, v14, v15;
+LIBA_SCOPE_VAR large v13, v14, v15;
extern int w1;
extern int w3, w4;
Re: [patch #6448] [MSVC 7/7] Add MSVC Support
Peter Rosin skrev:
Peter Rosin skrev:
Attached, I'll work through all the failures to try to find out why
they fail...
*snip*
24: link-order.at:26 Link order test.
Exporting int c variable.
With MSVC, you can declare any variable with __decspec(dllimport), even
if you are not actually importing it. The only thing that happens is
that you get an extra indirection in the generated code. This is a
small price to pay, when the gain is that you don't need to compile
things differently depending on how you are going to link with the
library. The situation is quite similar to pic/non-pic, but the trouble
is that the difference is in the library consumer and not when
producing the library.
So, ignoring the runtime performance penalty, patch things up like
this:
2008-08-13 Peter Rosin [EMAIL PROTECTED]
* tests/link-order.at [MSVC]: Always dllimport exported
variables.
diff --git a/tests/link-order.at b/tests/link-order.at
index ce52cc9..8791df8 100644
--- a/tests/link-order.at
+++ b/tests/link-order.at
@@ -48,13 +48,29 @@ for i in old new; do
mkdir src
cat src/a_$i.c EOF
-extern int c;
+/* w32 fun, MSVC supports dllimport even if importing is not needed (static
+ * case). gnu has auto import.
+ */
+#ifdef _MSC_VER
+# define LIBCEE_SCOPE __declspec(dllimport)
+#else
+# define LIBCEE_SCOPE extern
+#endif
+LIBCEE_SCOPE int c;
extern int b_$i();
int a_$i() { return c + b_$i(); }
EOF
cat src/b_$i.c EOF
-extern int c;
+/* w32 fun, MSVC supports dllimport even if importing is not needed (static
+ * case). gnu has auto import.
+ */
+#ifdef _MSC_VER
+# define LIBCEE_SCOPE __declspec(dllimport)
+#else
+# define LIBCEE_SCOPE extern
+#endif
+LIBCEE_SCOPE int c;
int b_$i() { return 1 + c; }
EOF
