[ adding libtool-patches@; followups can remove libtool@ ]
* KO Myung-Hun wrote on Sun, Nov 28, 2010 at 07:20:32AM CET:
> I've enhanced and fixed libtool 2.4 for OS/2.
Thanks again for working on this.
Generally, we prefer one patch per logical change, and GNU-style
ChangeLog entries. Also, we should strive to expose bugs in the
testsuite, so that we don't regress.
I understand that just producing a patch at all can be hard work,
so we can help with things (just that takes time ...)
One thing is quite helpful though, and that's how well our testsuite
fares on your system (both without and with the patch).
Also, for nontrivial changes, the FSF needs copyright papers
(more on this off-list).
That said, let's try to get the easier things out of the way:
> --- Makefile.am.org 2010-09-21 16:07:22.000000000 +0900
> +++ Makefile.am 2010-11-27 00:19:56.000000000 +0900
> @@ -324,7 +324,7 @@
> dist_man1_MANS = $(srcdir)/doc/libtool.1
> $(srcdir)/doc/libtoolize.1
> MAINTAINERCLEANFILES += $(dist_man1_MANS)
> update_mans = \
> - PATH=.$(PATH_SEPARATOR)$$PATH; export PATH; \
> + PATH=".$(PATH_SEPARATOR)$$PATH"; export PATH; \
Good change.
> $(HELP2MAN) --output=$@
> $(srcdir)/doc/libtool.1: $(srcdir)/$(auxdir)/ltmain.sh
> $(update_mans) --help-option=--help-all libtool
> --- libltdl/config/general.m4sh.org 2010-09-01 15:02:44.000000000 +0900
> +++ libltdl/config/general.m4sh 2010-11-27 12:15:52.000000000 +0900
> @@ -296,10 +296,13 @@
> ;;
> *)
> save_IFS="$IFS"
> - IFS=:
> - for progdir in $PATH; do
> - IFS="$save_IFS"
> - test -x "$progdir/$progname" && break
> + for pathsep in : ";"; do
> + IFS="$pathsep"
> + for progdir in $PATH$pathsep; do
> + IFS="$save_IFS"
> + test -x "$progdir/$progname" && break
> + done
> + test -n "$progdir" && break
> done
> IFS="$save_IFS"
> test -n "$progdir" || progdir=`pwd`
I don't particularly like guessing here. Rather, let's store the
configure-computed PATH_SEPARATOR in the generated libtool script
(libtoolize already sets it anyway) and use that.
I'm applying the following patch in your name, and adding you to THANKS:
2010-12-15 KO Myung-Hun <k...@chollian.net> (tiny change)
Ralf Wildenhues <ralf.wildenh...@gmx.de>
Fix PATH_SEPARATOR handling for OS/2.
* Makefile.am (update_mans): Quote $(PATH_SEPARATOR).
* libltdl/m4/libtool.m4 (_LT_SETUP): Add _LT_DECL for
PATH_SEPARATOR.
* libltdl/config/general.m4sh: Use PATH_SEPARATOR when computing
$progpath.
* THANKS: Update.
diff --git a/Makefile.am b/Makefile.am
index 66f38b1..4be353c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -330,7 +330,7 @@ $(srcdir)/doc/notes.txt: $(srcdir)/doc/notes.texi
dist_man1_MANS = $(srcdir)/doc/libtool.1 $(srcdir)/doc/libtoolize.1
MAINTAINERCLEANFILES += $(dist_man1_MANS)
update_mans = \
- PATH=.$(PATH_SEPARATOR)$$PATH; export PATH; \
+ PATH=".$(PATH_SEPARATOR)$$PATH"; export PATH; \
$(HELP2MAN) --output=$@
$(srcdir)/doc/libtool.1: $(srcdir)/$(auxdir)/ltmain.sh
$(update_mans) --help-option=--help-all libtool
diff --git a/libltdl/config/general.m4sh b/libltdl/config/general.m4sh
index 44a7ce9..40d5413 100644
--- a/libltdl/config/general.m4sh
+++ b/libltdl/config/general.m4sh
@@ -296,7 +296,7 @@ case $progpath in
;;
*)
save_IFS="$IFS"
- IFS=:
+ IFS=${PATH_SEPARATOR-:}
for progdir in $PATH; do
IFS="$save_IFS"
test -x "$progdir/$progname" && break
diff --git a/libltdl/m4/libtool.m4 b/libltdl/m4/libtool.m4
index 1f61140..ab3e16f 100644
--- a/libltdl/m4/libtool.m4
+++ b/libltdl/m4/libtool.m4
@@ -146,6 +146,8 @@ AC_REQUIRE([AC_CANONICAL_BUILD])dnl
AC_REQUIRE([_LT_PREPARE_SED_QUOTE_VARS])dnl
AC_REQUIRE([_LT_PROG_ECHO_BACKSLASH])dnl
+_LT_DECL([], [PATH_SEPARATOR], [0], [The PATH separator for the build
system])dnl
+dnl
_LT_DECL([], [host_alias], [0], [The host system])dnl
_LT_DECL([], [host], [0])dnl
_LT_DECL([], [host_os], [0])dnl
> @@ -564,6 +567,10 @@
(in func_show_eval)
> my_cmd="$1"
> my_fail_exp="${2-:}"
>
> + # pdksh 5.2.14-bin-2 for OS/2 does not remove trailing CR
> + # when a line length is 1022. Maybe 1022 is a magic number ?
> + my_cmd=`$ECHO "$my_cmd" | $SED s/\r$//`
Ouch. Where did you hit this? Can't you fix pdksh instead?
This change unconditionally costs two forks and one exec on almost every
command that libtool issues. Also, \r is not a portable sed regex.
Does something like this work instead?
# pdksh 5.2.14-bin-2 for OS/2 does not remove trailing CR
# when a line length is 1022.
case $my_cmd in *$'\r')
my_cmd=`$ECHO "$my_cmd" | $SED s/\r$//` ;;
esac
What about this?
cr=$'\r'
case $my_cmd in *$cr)
my_cmd=`$ECHO "$my_cmd" | $SED s/\r$//` ;;
esac
Then we still need to factor setting of $cr, but at least it's not quite
so expensive on other systems.
> ${opt_silent-false} || {
> func_quote_for_expand "$my_cmd"
> eval "func_echo $func_quote_for_expand_result"
> --- libltdl/config/ltmain.m4sh.org 2010-09-22 23:45:18.000000000 +0900
> +++ libltdl/config/ltmain.m4sh 2010-11-27 11:14:12.000000000 +0900
> @@ -395,7 +395,7 @@
> test "$opt_debug" = : || func_append preserve_args " --debug"
>
> case $host in
> - *cygwin* | *mingw* | *pw32* | *cegcc*)
> + *cygwin* | *mingw* | *pw32* | *cegcc* | *os2*)
ok.
> # don't eliminate duplications in $postdeps and $predeps
> opt_duplicate_compiler_generated_deps=:
> ;;
> @@ -1638,6 +1638,7 @@
> -rpath LIBDIR the created library will eventually be installed in
> LIBDIR
> -R[ ]LIBDIR add LIBDIR to the runtime path of programs and libraries
> -shared only do dynamic linking of libtool libraries
> + -shortname NAME specify a short name for a DLL(effect on OS/2 only)
> -shrext SUFFIX override the standard shared library file extension
> -static do not do any dynamic linking of uninstalled libtool
> libraries
> -static-libtool-libs
This change and associated other hunks should be in a separate patch on
its own, with an accompanying NEWS entry and addition to
doc/libtool.texi.
> @@ -2221,8 +2222,17 @@
> # so we also need to try rm && ln -s.
> for linkname
> do
> - test "$linkname" != "$realname" \
> - && func_show_eval "(cd $destdir && { $LN_S -f $realname
> $linkname || { $RM $linkname && $LN_S $realname $linkname; }; })"
> + if test "$linkname" != "$realname"; then
> + case $host_os in
> + os2*)
> + # Create import libraries instead of links on OS/2
> + func_show_eval "(emximp -o $destdir/$linkname
> $dir/${linkname%%_dll.$libext}.def)"
Can this instead be handled in a similar manner to how import libraries
are handled on MinGW and Cygwin?
Also, hard-coding 'emximp' does not seem like a good idea. It should be
either searchable or overridable, and the relevant commands should be in
a *_cmds variable in libtool.m4 (see above).
If the DLL is put in the bindir (is that common on OS/2?) then should
the import library reside in libdir (as is common on w32) or alongside
the DLL?
> + ;;
> + *)
> + func_show_eval "(cd $destdir && { $LN_S -f $realname
> $linkname || { $RM $linkname && $LN_S $realname $linkname; }; })"
> + ;;
> + esac
> + fi
> done
> fi
>
> @@ -4628,6 +4638,11 @@
> prev=
> continue
> ;;
> + shortname)
> + shortname_cmds="$ECHO $arg | cut -b -8"
Why do you use a cutoff here? It would seem to me that if the user
wanted to hard-code the name, then she should have the freedom to set
the length. Otherwise, I'd prefer to at least not hardcode the 8 here,
but use an _LT_DECL shortname_max or so and set it from libtool.m4
(with 0 denoting no limit, for example).
> + prev=
> + continue
> + ;;
> shrext)
> shrext_cmds="$arg"
> prev=
> @@ -4947,6 +4962,14 @@
> continue
> ;;
>
> + -shortname)
> + # OS/2 limits a length of a DLL basename up to 8 characters.
> + # So there is need to use a short name instead of a original name
> + # longer than 8 characters.
This comment would rather belong to libtool.m4 where shortname_cmds is
set.
> + prev=shortname
> + continue
> + ;;
> +
> -shrext)
> prev=shrext
> continue
[... rest will be reviewed in another mail ...]
Cheers,
Ralf
