Re: [PATCH] [cygwin|mingw] Fix compile warnings when -std=c89.
Ralf Wildenhues wrote:
> I don't see a need to skip the test elsewhere. Here's what I'd do:
> transform $LIBTOOL to have CFLAGS and LTCFLAGS also contain -std=c89
> -Werror. (The test would be even cleaner with a re-configured libtool,
> but let's not go overboard here.)
Being a total novice with autotest, I couldn't figure out how to do the
former, so I did the latter. Attached is my first attempt. It probably
should be named something other than 'c89 test' because that name
implies building all of libtool (including libltdl) with -std=c89. But I
couldn't think of anything else to call it.
If you'd like to knock it into better shape, I wouldn't mind...
--
Chuck
diff --git a/Makefile.am b/Makefile.am
index 790..726b898 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -467,6 +467,7 @@ TESTSUITE_AT= tests/testsuite.at \
tests/indirect_deps.at \
tests/archive-in-archive.at \
tests/execute-mode.at \
+ tests/c89.at \
tests/infer-tag.at \
tests/localization.at \
tests/install.at \
--- a/tests/c89.at 2006-11-30 19:00:00.0 -0500
+++ b/tests/c89.at 2009-01-21 21:32:01.79210 -0500
@@ -0,0 +1,85 @@
+# c89.at -- test compliance with c89 standard -*- Autotest -*-
+
+# Copyright (C) 2009 Free Software Foundation, Inc.
+# Written by Charles Wilson, 2009
+#
+# This file is part of GNU Libtool.
+#
+# GNU Libtool is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# GNU Libtool is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GNU Libtool; see the file COPYING. If not, a copy
+# can be downloaded from http://www.gnu.org/licenses/gpl.html,
+# or obtained by writing to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+
+AT_SETUP([c89 test])
+AT_KEYWORDS([libtool])
+
+# make sure existing libtool is configured for shared libraries
+AT_CHECK([$LIBTOOL --features | grep 'disable shared libraries' && (exit 77)],
+[1], [ignore])
+
+# make sure CFLAGS -std=c89 -Werror do not cause a failure
+# themselves (e.g. because a non-gcc compiler doesn't support
+# those flags).
+AT_DATA([trivial.c],
+[[
+int main (void)
+{
+ return 0;
+}
+]])
+AT_CHECK([$LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -std=c89 -Werror -c
trivial.c || exit 77],[0],[ignore],[ignore])
+
+AT_DATA([configure.ac],
+[[AC_INIT([app-uses-sharedlib-demo], ]AT_PACKAGE_VERSION[,
]AT_PACKAGE_BUGREPORT[)
+AC_CONFIG_AUX_DIR([config])
+AC_CONFIG_MACRO_DIR([m4])
+AM_INIT_AUTOMAKE([foreign])
+LT_INIT([win32-dll disable-static])
+AC_CONFIG_FILES([Makefile])
+AC_OUTPUT
+]])
+AT_DATA([Makefile.am],
+[[ACLOCAL_AMFLAGS = -I m4
+AUTOMAKE_OPTIONS = 1.7
+lib_LTLIBRARIES = liba.la
+liba_la_LDFLAGS = -version-info 0:0:0 -no-undefined
+liba_la_SOURCES = liba.c
+bin_PROGRAMS = usea
+usea_SOURCES = usea.c
+usea_LDADD = liba.la
+]])
+AT_DATA([liba.c],
+[[int liba_func1 (int arg)
+{
+ return arg + 1;
+}
+]])
+AT_DATA([usea.c],
+[[extern int liba_func1(int arg);
+int main (void)
+{
+ int a = 2;
+ int b = liba_func1 (a);
+ if (b == 3) return 0;
+ return 1;
+}
+]])
+
+LT_AT_BOOTSTRAP([--copy --force], [-I m4 --force], [ignore],
+ [--add-missing --copy --force-missing], [--force], [CFLAGS='-std=c89
-Werror'], [])
+AT_CHECK([test -f liba.la])
+LT_AT_EXEC_CHECK([./usea], [0], [ignore], [ignore], [])
+
+AT_CLEANUP
Re: "Run tests with low max_cmd_len" on MSYS/MSVC
On Wed, 21 Jan 2009, Ralf Wildenhues wrote: We may need to think about speeding up func_to_host_path, e.g., by not forking for arguments that don't need conversion, or by converting several paths with a constant amount of forks. But that can be done separately. There is also the possibility of building a translation cache which is preserved across libtool invokations. The only concern is if the mounts get changed after the cache is generated. Bob == Bob Friesenhahn bfrie...@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/ GraphicsMagick Maintainer,http://www.GraphicsMagick.org/
Re: spaces
Ralf Wildenhues wrote: > OK so it seems there are more voices for spaces. > Can we agree to make the switch only after 2.2.8 though, > I would like to avoid unnecessary churn ATM. Works for me. -- Chuck
Re: "Run tests with low max_cmd_len" on MSYS/MSVC
* Charles Wilson wrote on Wed, Jan 21, 2009 at 11:11:22PM CET:
> Ralf Wildenhues wrote:
>
> > We may need to think about speeding up func_to_host_path, e.g., by not
> > forking for arguments that don't need conversion, or by converting
> > several paths with a constant amount of forks. But that can be done
> > separately.
>
> FWIW, as part of re-writing this:
> [PATCH] [cygwin]: Add cross-compile support to cwrapper
> http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg6.html
> I plan to split up the various case $host/case $build clauses in the
> func_to_host_path functions into separate functions. Then, using a
> configure test determine which function should be called via a libtool
> variable $build_to_host_path_cmd and $build_to_host_pathlist_cmd.
>
> The default value will be a function that simply assigns
> build_to_host_path_result=$1
>
> Then, we can target specific functions for speedups as needed.
>
> Ok plan?
Sounds like an idea.
Note that in sh one can also just define functions conditionally:
if $cond; then
func_foo ()
{
...
}
else
func_foo ...
also note that we have a section in libtool.m4 where we already define
some per-system functions which are then added literally to libtool.
That way, libtool doesn't ever contain the ones not intended for this
setup.
Cheers,
Ralf
Re: spaces
* Charles Wilson wrote on Tue, Jan 20, 2009 at 07:29:52PM CET: > Bob Friesenhahn wrote: > > For many years I have had my editor configured to always use > > spaces. This ensures WYSIWYG for everyone involved. > > Agree 100%. I try to manually match whatever sp/tab convention is in > place -- using vi if I have to -- but much prefer all spaces. OK so it seems there are more voices for spaces. Can we agree to make the switch only after 2.2.8 though, I would like to avoid unnecessary churn ATM. Thanks, Ralf
Re: "Run tests with low max_cmd_len" on MSYS/MSVC
Ralf Wildenhues wrote: > We may need to think about speeding up func_to_host_path, e.g., by not > forking for arguments that don't need conversion, or by converting > several paths with a constant amount of forks. But that can be done > separately. FWIW, as part of re-writing this: [PATCH] [cygwin]: Add cross-compile support to cwrapper http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg6.html I plan to split up the various case $host/case $build clauses in the func_to_host_path functions into separate functions. Then, using a configure test determine which function should be called via a libtool variable $build_to_host_path_cmd and $build_to_host_pathlist_cmd. The default value will be a function that simply assigns build_to_host_path_result=$1 Then, we can target specific functions for speedups as needed. Ok plan? -- Chuck
Re: Unify line endings in localization test
* Peter Rosin wrote on Tue, Jan 20, 2009 at 12:34:57AM CET: > Den 2009-01-19 21:35 skrev Ralf Wildenhues: >> This looks a bit hackish. We already have a handful of places which we >> fixed up in order to avoid line ending issues. This one looks hackish >> enough to deserve being wrapped in a macro (in testsuite.at, below >> LT_AT_HOST_DATA?), so that future instances of this issue can easily be >> handled likewise. > > Something like the attached? Yes, with nits below addressed. > --- a/tests/testsuite.at > +++ b/tests/testsuite.at > @@ -204,6 +204,19 @@ case $host_os in mingw*) > esac]) > > > +# LT_AT_UNIFY_NL(FILE, [RESULT-FILE]) > +# Let's underline the whole text. :-) > +# Ensure (text) file has predicable line endings. FILE. Please describe RESULT-FILE. Typo "predictable". > +m4_define([LT_AT_UNIFY_NL], > +[case $host_os in > + mingw*) > + tr -d '\015' < $1 > m4_ifval([$2], [$2], [$1.t > + mv -f $1.t $1]) ;; m4_ifval([$2], [ > + *) > + mv -f $1 $2 ;;]) > +esac]) 1 or 2 characters of indentation? ;-) An even easier-to-use interface would be an LT_AT_CHECK_FF or so, which would check after converting to a standard file format. But the above is good enough, and easy enough to use, and the full solution would require us to hack up more, with LT_AT_EXEC_CHECK and all. So let's use this. Thanks, Ralf
Re: [PATCH] GNU/kOpenSolaris port
* Robert Millan wrote on Tue, Jan 20, 2009 at 11:09:06AM CET: > > Btw, do you expect a new release soon? Hopefully in the not too distant future; we cannot promise any time frame, though. There are a few issues to address yet. Cheers, Ralf
Re: "Run tests with low max_cmd_len" on MSYS/MSVC
Hi Peter, * Peter Rosin wrote on Wed, Jan 21, 2009 at 02:26:36AM CET: > This patch together with [1] and [2] will make "Run tests with > low max_cmd_len" on MSYS/MSVC behave the same as the individual > tests. > > The patch fixes a couple more of the /abs/path issues already > fixed in [1] and [2]. However, I fear that these hunks may be > more controversial, as they touch code that affects other > platforms as well. Well, for non-w32 systems the patch is pretty obviously ok, as func_to_host_path doesn't do much there, and not slowly. :-) For Cygwin and MinGW, it will likely make things much slower, since this will cause lots of forks. From a correctness perspective, the patch looks good to go, though. We may need to think about speeding up func_to_host_path, e.g., by not forking for arguments that don't need conversion, or by converting several paths with a constant amount of forks. But that can be done separately. Thanks, Ralf > [1] http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg00090.html > [2] http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg00092.html > > 2009-01-21 Peter Rosin > > Convert paths to host format > * libltdl/config/ltmain.m4sh (func_mode_link): When exporting > symbols, linking and creating archives using command files (or > response files), make sure that both the name of the command > file and the content are made up of file names in a format > appropriate for the host. Fixes stresstest.at on MSYS/MSVC when > run with low command line length.
Re: [PATCH] [cygwin|mingw] Fix compile warnings when -std=c89.
* Charles Wilson wrote on Wed, Jan 21, 2009 at 10:10:08PM CET: > Ralf Wildenhues wrote: > > Part (1) is easy to review: it is obvious that regressions are very > > unlikely to be system-dependent. One does get the impression that it > > might just be more efficient to let libtool save the cwrapper text > > somewhere and the program just cat that. But still, this part is ok, > > please apply. > > This part pushed... Thanks. > > Why is this patch not accompanied by a testsuite addition using > > -std=c89 -Werror on a program that creates a C wrapper? > > ...but without an additional test. Ralf, how should such a test be > structured? Do we need (like Darwin) a separate category of windows-ish > tests, that are skipped elsewhere, or what? I don't see a need to skip the test elsewhere. Here's what I'd do: transform $LIBTOOL to have CFLAGS and LTCFLAGS also contain -std=c89 -Werror. (The test would be even cleaner with a re-configured libtool, but let's not go overboard here.) With that, compile a library, and a program linked against it (so that, on w32, a wrapper is compiled). In order to avoid false failures due to non-GCC or so, you can also compile a trivial program and skip if the above flags cause an error. Cheers, Ralf
Re: [PATCH] [cygwin|mingw] Fix compile warnings when -std=c89.
Ralf Wildenhues wrote: > Part (1) is easy to review: it is obvious that regressions are very > unlikely to be system-dependent. One does get the impression that it > might just be more efficient to let libtool save the cwrapper text > somewhere and the program just cat that. But still, this part is ok, > please apply. This part pushed... > Why is this patch not accompanied by a testsuite addition using > -std=c89 -Werror on a program that creates a C wrapper? ...but without an additional test. Ralf, how should such a test be structured? Do we need (like Darwin) a separate category of windows-ish tests, that are skipped elsewhere, or what? The bits removed from this commit are attached as "cygwin-cwrapper-cleanups.patch". I did not run a full test suite with just the pushed bits; however, those bits plus the cygwin-cwrapper-cleanups did pass, and the pushed bits alone passed a bootstrap/configure/make cycle and some (old) testsuite spot checks. Also, the generated cwrapper code compiled when -std=c89 without error (cygwin). As pushed (0010--cygwin-mingw-Fix-compile-warnings-when-std-c89.patch-pushed): [cygwin|mingw] Fix compile warnings when -std=c89. * libltdl/config/ltmain.m4sh (func_emit_wrapper_part1): move contents to... (func_emit_wrapper_part2): move contents to... (func_emit_wrapper): here. (func_emit_cwrapperexe_src) [file scope]: Remove variables script_text_part1 and script_text_part2. (func_emit_cwrapperexe_src) [lt_dump_script]: New function. (func_emit_cwrapperexe_src) [main]: Call it. -- Chuck diff --git a/libltdl/config/ltmain.m4sh b/libltdl/config/ltmain.m4sh index 760f647..8728a7c 100644 --- a/libltdl/config/ltmain.m4sh +++ b/libltdl/config/ltmain.m4sh @@ -2746,18 +2716,11 @@ EOF # include # include # include -# define setmode _setmode #else # include # include # ifdef __CYGWIN__ # include -# define HAVE_SETENV -# ifdef __STRICT_ANSI__ -char *realpath (const char *, char *); -int putenv (char *); -int setenv (const char *, const char *, int); -# endif # endif #endif #include @@ -2771,6 +2734,33 @@ int setenv (const char *, const char *, int); #include #include +/* declarations of non-ANSI functions */ +#if defined(__MINGW32__) +# ifdef __STRICT_ANSI__ +int _putenv (const char *); +# endif +#elif defined(__CYGWIN__) +# ifdef __STRICT_ANSI__ +char *realpath (const char *, char *); +int putenv (char *); +int setenv (const char *, const char *, int); +# endif +#endif + +/* portability #defines */ +#if defined(_MSC_VER) || defined(__MINGW32__) +# ifndef __MINGW32CE__ +# define setmode _setmode +# define stat_stat +# define chmod _chmod +# define getcwd _getcwd +# define putenv _putenv +# endif +#endif +#ifdef __CYGWIN__ +# define HAVE_SETENV +#endif + #if defined(PATH_MAX) # define LT_PATHMAX PATH_MAX #elif defined(MAXPATHLEN) @@ -2788,7 +2778,6 @@ int setenv (const char *, const char *, int); #ifdef _MSC_VER # define S_IXUSR _S_IEXEC -# define stat _stat # ifndef _INTPTR_T_DEFINED # define intptr_t int # endif @@ -2841,7 +2830,7 @@ int setenv (const char *, const char *, int); } while (0) #undef LTWRAPPER_DEBUGPRINTF -#if defined DEBUGWRAPPER +#if defined LT_DEBUGWRAPPER # define LTWRAPPER_DEBUGPRINTF(args) ltwrapper_debugprintf args static void ltwrapper_debugprintf (const char *fmt, ...) diff --git a/ChangeLog b/ChangeLog index b702907..8d83a7b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,17 @@ 2009-01-21 Charles Wilson + [cygwin|mingw] Fix compile warnings when -std=c89. + * libltdl/config/ltmain.m4sh (func_emit_wrapper_part1): + move contents to... + (func_emit_wrapper_part2): move contents to... + (func_emit_wrapper): here. + (func_emit_cwrapperexe_src) [file scope]: Remove + variables script_text_part1 and script_text_part2. + (func_emit_cwrapperexe_src) [lt_dump_script]: New function. + (func_emit_cwrapperexe_src) [main]: Call it. + +2009-01-21 Charles Wilson + Minor cygwin cleanup * libltdl/config/ltmain.m4sh (func_generate_dlsyms): Correct case pattern for cygwin. diff --git a/libltdl/config/ltmain.m4sh b/libltdl/config/ltmain.m4sh index 760f647..3f1a30c 100644 --- a/libltdl/config/ltmain.m4sh +++ b/libltdl/config/ltmain.m4sh @@ -2308,15 +2308,23 @@ func_extract_archives () } - -# func_emit_wrapper_part1 [arg=no] +# func_emit_wrapper [arg=no] # -# Emit the first part of a libtool wrapper script on stdout. -# For more information, see the description associated with -# func_emit_wrapper(), below. -func_emit_wrapper_part1 () +# Emit a libtool wrapper script on stdout. +# Don't directly open a file because we may want to +# incorporate the script contents within a cygwin/mingw +# wrapper executable. Must ONLY be called from within +# func_mode_link because it depends on a number of variables +# set therein. +# +# ARG is the value that the WRAPPER_SCRIPT_BELONGS_IN_OBJDIR +# variable wi
Re: [PATCH] [cygwin|mingw] Fix compile warnings when -std=c89.
Ralf Wildenhues wrote: > * Charles Wilson wrote on Wed, Jan 21, 2009 at 08:47:42PM CET: >> Part of my tendency to include minor -- easy to review -- changes with >> larger ones is due to (a) see it, fix it, otherwise it'll be forgotten >> and (b) EVERY separate patchset requires an independent full testsuite >> run. > > Ah, ok. I can feel the pain. Let's relieve that a wee bit, without > compromising quality too much: > (a) can be addressed with git. Really, git's is flexible enough to > allow for doing many many small commits, even ugly ones, and cleaning up > afterwards. AIUI git is available for Cygwin and MinGW. Yep. I'm using it on cygwin. For mingw stuff I usually make dist on cygwin, and send the tarball across to mingw. > (b) There is no need for full testsuite runs for every patch. If two > patches are clearly independent, then one run with both of them should > suffice. If you have a (not too huge) patch series, where things belong > together, and the end point passes the testsuite, then while that is not > ideal, it is still a lot better than nothing; in that case, please note > this, and we can still ask for results of intermediate states if > necessary. Well that'd be easier. > Which however makes testsuite additions all the more important: that > way, at least, when we run the suite before a release, we can find > potential regressions. Think of testsuite additions as a way to shift > some of the testing grunt work from yourself over to other users. This only matters if an full and successful testsuite run is not required after each patch (sequence?), or if all you're worried about is when patchA (submitted for unix, or cygwin) might break mingw but the original submitter only tested on "his" platform. Which is an important case, of course. But if you're fixing a cygwin bug, you need to test on cygwin (full testsuite? see below...) > Just to be sure: you are aware of limiting the testsuite runs to just > those bits that you are adding? > old suite: > make check TESTSUITEFLAGS=-V TESTS="tests/foo.test tests/bar.test" \ > VERBOSE=yes > new suite: > make check-local TESTSUITEFLAGS='-v -d -x 27-33' Sure, but how do you then *know* that your change didn't break something else? At some point you still have to run the full testsuite before committing to master, at least on the platform for which you're fixing a bug. These shortcuts can speed up development while coding the fix, but not the final submission/review process for the change. -- Chuck
Re: [PATCH] [cygwin|mingw] Fix compile warnings when -std=c89.
* Charles Wilson wrote on Wed, Jan 21, 2009 at 08:47:42PM CET: > Ralf Wildenhues wrote: > > I am very sorry that reviewing takes so long. Mostly this is due to > > time constraints on my side. > > On the plus side, your reviews are usually insightful, and lead to ideas > for better code, like the libfile_$(transliterated implib name) thing. Thanks. > > You can make things easier by splitting them into logically independent > > (hopefully small) pieces. I acknowledge that some of your other patches > > may not be splittable further. > > Part of my tendency to include minor -- easy to review -- changes with > larger ones is due to (a) see it, fix it, otherwise it'll be forgotten > and (b) EVERY separate patchset requires an independent full testsuite > run. Ah, ok. I can feel the pain. Let's relieve that a wee bit, without compromising quality too much: (a) can be addressed with git. Really, git's is flexible enough to allow for doing many many small commits, even ugly ones, and cleaning up afterwards. AIUI git is available for Cygwin and MinGW. (b) There is no need for full testsuite runs for every patch. If two patches are clearly independent, then one run with both of them should suffice. If you have a (not too huge) patch series, where things belong together, and the end point passes the testsuite, then while that is not ideal, it is still a lot better than nothing; in that case, please note this, and we can still ask for results of intermediate states if necessary. > Not fun. And I don't get many blocks of 5 hours to waste listening for > the Windows Popup Alert sound. This has little to do with any delays in > reviewing -- but does tend to make the reviewing process harder. And > take longer. Which means more !...@#$ testsuite runs. It's a vicious > cycle, honestly. I can feel the pain. > Recent improvements in cygwin-1.7 have once again tamed the popup > problem on vista -- so at least I don't have to personally attend each > testsuite run. It's still 5 hours of useless computer tho. Good. > MinGW/Msys "native" test runs are still an exercise in whack-a-mole with > the popups. Which however makes testsuite additions all the more important: that way, at least, when we run the suite before a release, we can find potential regressions. Think of testsuite additions as a way to shift some of the testing grunt work from yourself over to other users. > Actually, for 2.4 I think the cwrapper should no longer include the > --lt-dump-script option; it is no longer really necessary. But I'm > extremely hesistant to remove it before then; we've had enough > destabilizing changes in a "stable" release already. OK. > > Why is this patch not accompanied by a testsuite addition using > > -std=c89 -Werror on a program that creates a C wrapper? > > Because I am well trained to be allergic to making the test suite take > even longer. I know it is not fully rational, because tests help us > avoid these problems in the future. But sweet mother of god is it painful. We need to work against this allergy. Hope the above to be a first step. Just to be sure: you are aware of limiting the testsuite runs to just those bits that you are adding? old suite: make check TESTSUITEFLAGS=-V TESTS="tests/foo.test tests/bar.test" \ VERBOSE=yes new suite: make check-local TESTSUITEFLAGS='-v -d -x 27-33' (another mail coming up for the rest) Cheers, Ralf
Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 4
Charles Wilson wrote: Reviewing my own submission... > (func_cygming_dll_for_implib_core): New function. This function is actually called func_cygming_dll_for_implib_fallback_core Need to correct log history. > (func_cygming_implib_p): New function. Confusing. There is already a func_win32_implib_p which is less specific (returns true when [effectively] func_cygming_implib_p || func_cygming_ms_implib_p || (any other kind of implib) This one should be called func_cygming_gnu_implib_p as a parallel with the _cygming_ms_ one. -- Chuck
Re: [PATCH] [cygwin|mingw] Fix compile warnings when -std=c89.
Ralf Wildenhues wrote: > I am very sorry that reviewing takes so long. Mostly this is due to > time constraints on my side. On the plus side, your reviews are usually insightful, and lead to ideas for better code, like the libfile_$(transliterated implib name) thing. > You can make things easier by splitting them into logically independent > (hopefully small) pieces. I acknowledge that some of your other patches > may not be splittable further. Part of my tendency to include minor -- easy to review -- changes with larger ones is due to (a) see it, fix it, otherwise it'll be forgotten and (b) EVERY separate patchset requires an independent full testsuite run. Until recently, that was 5 hours of sitting in front of my computer waiting for popups, while that computer was completely useless for anything else (100% cpu). Not fun. And I don't get many blocks of 5 hours to waste listening for the Windows Popup Alert sound. This has little to do with any delays in reviewing -- but does tend to make the reviewing process harder. And take longer. Which means more !...@#$ testsuite runs. It's a vicious cycle, honestly. Anyway, these 5 hour torture sessions are NOT something I want to do twice -- or five times -- when once will verify that both (or all five) separate changes are ok -- but not *necessarily* verify that part #2 is ok when parts #1, #3, #4, and #5 are not immediately committed along with it. Even if it seems "obvious" that part #2 is independent. Recent improvements in cygwin-1.7 have once again tamed the popup problem on vista -- so at least I don't have to personally attend each testsuite run. It's still 5 hours of useless computer tho. MinGW/Msys "native" test runs are still an exercise in whack-a-mole with the popups. > This particular patch does two logically separate things: > 1) repair the creation of the splitted wrapper script. > 2) reorganize portability macros inside the wrapper C code. > > Part (1) is easy to review: it is obvious that regressions are very > unlikely to be system-dependent. One does get the impression that it > might just be more efficient to let libtool save the cwrapper text > somewhere and the program just cat that. But still, this part is ok, > please apply. Will do. Actually, for 2.4 I think the cwrapper should no longer include the --lt-dump-script option; it is no longer really necessary. But I'm extremely hesistant to remove it before then; we've had enough destabilizing changes in a "stable" release already. > Part (2) is a bit unobvious when merely looking at the diff. The > reorganized lines look ok, but not like a clear improvement from > the earlier one: both are not fully logical. And then, why is > realpath only declared if > __CYGWIN__ and __STRICT_ANSI__ && !defined _MSC_VER > before the patch, but > __CYGWIN__ and __STRICT_ANSI__ && !defined __MINGW32__ > after the patch? Two reasons: (1) the _MSC_VER was incorrect. It was originally a proxy for "win32 but not cygwin" -- but it isn't, really. It should have been #if _MSC_VER || __MINGW32__ (stuff) #else (other stuff) #endif. (2) But that's really not right either, and characterizing the #ifdef sequence as you do above is accurate, but misleading. The reason it is misleading is that things like __MINGW32__, __CYGWIN__, etc are platform defines, and only one should ever be true at a time (otherwise, you have far worse issues that we can deal with inside libtool). Thus: #if __PLATFORM1__ ...stuff-1 #elif __PLATFORM2__ ...stuff-2 #elif __PLATFORM3__ ...stuff-3 #endif is really no different than #if __PLATFORM1__ ...stuff-1 #endif #if __PLATFORM2__ ...stuff-2 #endif #if __PLATFORM3__ ...stuff-3 #endif In the latter case, you wouldn't complain that "stuff-3" only happens when __PLATFORM3__ && !__PLATFORM2__ && !__PLATFORM1__, even though (because of the exclusivity of platform macros) it is, in fact, accurate to say so. But the former case is effectively the same because of that same exclusivity -- with one benefit: The former case, but not the latter, allows a final #else ...default-stuff... clause. That's the structure I was going for, but it wasn't clear because (a) we only have three [*] platforms (__MINGW32__, __CYGWIN__, and _MSC_VER as a proxy for native win32 with msvc compiler) -- but the last case does not require any function declarations, so it's missing from the /* declarations of non-ANSI functions */ section. It's hard to see the pattern with only two examples. [*] counting wince and mingw64, maybe five platforms. but libtool-developer support for those two is thin. Technically, the /* portability #defines */ section should be structured similarly, but I missed that one, so the #if defined(_MSC_VER) || defined(__MINGW32__) clause ends with an #endif, not an #elif __CYGWIN__. But it probably should. Also, in this section, I tried -- with no actual knowledge of the platform -- to make it easier for the wince guys to add their hooks. But I didn't know (and still d
Re: [PATCH] [cygwin|mingw] Fix compile warnings when -std=c89.
Hello Charles, * Charles Wilson wrote on Wed, Jan 21, 2009 at 04:21:01PM CET: > Thanks, Peter; your validation on msvc is useful and encouraging. Now > I'm just waiting for a go/no-go from one of the four libtool > maintainers: Gary Vaughan, Peter O'Gorman, Ralf Wildenhues, or Bob > Friesenhahn. I am very sorry that reviewing takes so long. Mostly this is due to time constraints on my side. You can make things easier by splitting them into logically independent (hopefully small) pieces. I acknowledge that some of your other patches may not be splittable further. This particular patch does two logically separate things: 1) repair the creation of the splitted wrapper script. 2) reorganize portability macros inside the wrapper C code. Part (1) is easy to review: it is obvious that regressions are very unlikely to be system-dependent. One does get the impression that it might just be more efficient to let libtool save the cwrapper text somewhere and the program just cat that. But still, this part is ok, please apply. Part (2) is a bit unobvious when merely looking at the diff. The reorganized lines look ok, but not like a clear improvement from the earlier one: both are not fully logical. And then, why is realpath only declared if __CYGWIN__ and __STRICT_ANSI__ && !defined _MSC_VER before the patch, but __CYGWIN__ and __STRICT_ANSI__ && !defined __MINGW32__ after the patch? FWIW, realpath is used only if !defined S_ISLNK. This makes me wince (no pun intended), thinking that just maybe not all systems this is relevant have been duly tested (what about MinGW plus -std=c89?). Why is this patch not accompanied by a testsuite addition using -std=c89 -Werror on a program that creates a C wrapper? Thanks for all your work on this. Cheers, Ralf
Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 4
Charles Wilson wrote: > Test suite on cygwin/native in progress. Assumming test suite passes, OK? > Comments, Review, Discussion? All tests pass (cygwin/native): Old suite: === All 113 tests passed (11 tests were not run) === New suite: 76 tests behaved as expected. 5 tests were skipped. -- Chuck
Re: [PATCH] Minor cygwin cleanup
On Tue, 20 Jan 2009, Charles Wilson wrote:
libltdl/config/ltmain.m4sh (func_generate_dlsyms): Correct
case pattern for cygwin.
---
Ok to push?
Looks fine to me.
Bob
libltdl/config/ltmain.m4sh |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libltdl/config/ltmain.m4sh b/libltdl/config/ltmain.m4sh
index 6be529a..760f647 100644
--- a/libltdl/config/ltmain.m4sh
+++ b/libltdl/config/ltmain.m4sh
@@ -1987,7 +1987,7 @@ extern \"C\" {
eval '$GREP -f "$output_objdir/$outputname.exp" < "$nlist" >
"$nlist"T'
eval '$MV "$nlist"T "$nlist"'
case $host in
- *cygwin | *mingw* | *cegcc* )
+ *cygwin* | *mingw* | *cegcc* )
eval "echo EXPORTS "'> "$output_objdir/$outputname.def"'
eval 'cat "$nlist" >> "$output_objdir/$outputname.def"'
;;
--
1.6.0.4
==
Bob Friesenhahn
bfrie...@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer,http://www.GraphicsMagick.org/
Re: [PATCH] [cygwin|mingw] Fix compile warnings when -std=c89.
Peter Rosin wrote: > Den 2009-01-16 15:15 skrev Charles Wilson: >> Charles Wilson wrote: >>> Charles Wilson wrote: * libltdl/config/ltmain.m4sh: Update copyright date. (func_emit_wrapper_part1): move contents to... (func_emit_wrapper_part2): move contents to... (func_emit_wrapper): here. (func_emit_cwrapperexe_src) [file scope]: re-organized includes and portability macros. Avoid oldnames on MINGW32 and MSVC for setmode/stat/chmod/getcwd/putenv. Declare _putenv on MINGW32 when -ansi. Use namespaced macro LT_DEBUGWRAPPER. Remove variables script_text_part1 and script_text_part2. (func_emit_cwrapperexe_src) [lt_dump_script]: New function. (func_emit_cwrapperexe_src) [main]: Call it. >>> Ping? >>> >> Ping x 2? > > I tried this patch on the pr-msvc-support branch and it seems > to work just fine there as well (tested with MSVC 2005). Thanks, Peter; your validation on msvc is useful and encouraging. Now I'm just waiting for a go/no-go from one of the four libtool maintainers: Gary Vaughan, Peter O'Gorman, Ralf Wildenhues, or Bob Friesenhahn. Status: This patch was successfully tested by Roumen Petrov (on master, unix->mingw cross) http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg00012.html and by Peter Rosin (on pr-msvc-support branch, msys/msvc2005 "native") http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg00060.html -- and me, msys/mingw native http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg5.html -- Chuck
[PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 4
* libltdl/config/general.m4sh: Update copyright year.
(func_tr_sh): New function.
* libltdl/config/ltmain.m4sh (func_generate_dlsyms) [cygwin|mingw]:
Obtain DLL name corresponding to import library by using value
stored in unique variable libfile_$(transliterated implib name).
If that fails, use $sharedlib_from_linklib_cmd to extract DLL
name from import library directly. Also, properly extract dlsyms
from the import library.
(func_mode_link) [cygwin|mingw]: Prefer to dlpreopen DLLs
over static libs when both are available. When dlpreopening
DLLs, use linklib (that is, import lib) as dlpreopen file,
rather than DLL. Store name of associated la file in
unique variable libfile_$(transliterated implib name)
for later use.
(func_win32_libid): Accomodate pei-i386 import libs
as well as pe-i386.
(func_cygming_dll_for_implib): New function.
(func_cygming_dll_for_implib_fallback): New function.
(func_cygming_dll_for_implib_core): New function.
(func_cygming_implib_p): New function.
(func_cygming_ms_implib_p): New function.
* libltdl/m4/libtool.m4 (_LT_CMD_GLOBAL_SYMBOLS): Adjust sed
expressions for lt_cv_sys_global_symbol_to_c_name_address and
lt_cv_sys_global_symbol_to_c_name_address_lib_prefix
as trailing space after module name is optional.
(_LT_LINKER_SHLIBS) [cygwin|mingw][C++]:
Set exclude_expsyms correctly for $host. Simplify regular
expression in export_symbols_cmds.
(_LT_LINKER_SHLIBS) [cygwin|mingw|pw32][C]: Set exclude_expsyms
correctly for $host. Enable export_symbols_cmds to identify
DATA exports by _nm_ prefix.
(_LT_CHECK_SHAREDLIB_FROM_LINKLIB): New macro sets
sharedlib_from_linklib_cmd variable.
(_LT_DECL_DLLTOOL): New macro ensures DLLTOOL is always set.
---
This updated patch represents the promised combination of "take 2"
http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg2.html
and "take 3"
http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg00056.html
for easier reviewing, along with a minor change to func_tr_sh similar
to that suggested by Ralf W, and rebased to current master.
Test suite on cygwin/native in progress. Assumming test suite passes, OK?
Comments, Review, Discussion?
libltdl/config/general.m4sh | 19 -
libltdl/config/ltmain.m4sh | 226 +++
libltdl/m4/libtool.m4 | 62 +++-
3 files changed, 282 insertions(+), 25 deletions(-)
diff --git a/libltdl/config/general.m4sh b/libltdl/config/general.m4sh
index 4bc304c..e839070 100644
--- a/libltdl/config/general.m4sh
+++ b/libltdl/config/general.m4sh
@@ -1,6 +1,6 @@
m4_if([general.m4sh -- general shell script boiler plate -*- Autoconf -*-
- Copyright (C) 2004, 2005, 2007, 2008 Free Software Foundation, Inc.
+ Copyright (C) 2004, 2005, 2007, 2008, 2009 Free Software Foundation, Inc.
Written by Gary V. Vaughan, 2004
This file is part of GNU Cvs-utils.
@@ -412,5 +412,22 @@ func_show_eval_locale ()
fi
fi
}
+
+# func_tr_sh
+# Turn $1 into a string suitable for a shell variable name.
+# Result is stored in $func_tr_sh_result. All characters
+# not in the set a-zA-Z0-9_ are replaced with '_'. Further,
+# if $1 begins with a digit, a '_' is prepended as well.
+func_tr_sh ()
+{
+ case "$1" in
+ [0-9]* | *[!a-zA-Z0-9_]*)
+func_tr_sh_result=`$ECHO "$1" | $SED 's/^\([0-9]\)/_\1/;
s/[^a-zA-Z0-9_]/_/g'`
+;;
+ * )
+func_tr_sh_result=$1
+;;
+ esac
+}
]])
diff --git a/libltdl/config/ltmain.m4sh b/libltdl/config/ltmain.m4sh
index 760f647..984abd2 100644
--- a/libltdl/config/ltmain.m4sh
+++ b/libltdl/config/ltmain.m4sh
@@ -2000,10 +2000,49 @@ extern \"C\" {
func_verbose "extracting global C symbols from \`$dlprefile'"
func_basename "$dlprefile"
name="$func_basename_result"
- $opt_dry_run || {
- eval '$ECHO ": $name " >> "$nlist"'
- eval "$NM $dlprefile 2>/dev/null | $global_symbol_pipe >> '$nlist'"
- }
+ case $host in
+ *cygwin* | *mingw* | *cegcc* )
+ # if an import library, we need to obtain dlname
+ if func_win32_import_lib_p "$dlprefile"; then
+ func_tr_sh "$dlprefile"
+ eval "curr_lafile=\$libfile_$func_tr_sh_result"
+ dlprefile_dlbasename=""
+ if test -n "$curr_lafile" && func_lalib_p "$curr_lafile"; then
+ # Use subshell, to avoid clobbering current variable values
+ dlprefile_dlname=`source "$curr_lafile" && echo "$dlname"`
+ if test -n "$dlprefile_dlname" ; then
+ func_basename "$dlprefile_dlname"
+ dlprefile_dlbasename="$func_basename_result"
+ else
+ # no lafile. user explicitly requested -dlpreopen .
+ eval '$sharedlib_from_linklib "$dlprefile"'
+ dlprefile_dlbasename=$sharedlib_from_linklib_result
+ fi
+ fi
+ $opt_dry_run || {
+ i
Re: [PATCH] Minor cygwin cleanup
Ralf Wildenhues wrote: > * Charles Wilson wrote on Tue, Jan 20, 2009 at 11:31:43PM CET: >> libltdl/config/ltmain.m4sh (func_generate_dlsyms): Correct >> case pattern for cygwin. >> --- >> Ok to push? > > Yes, thanks! Pushed. -- Chuck
