Re: [PATCH, take 3][cygwin|mingw] Control where win32 DLLs get installed.

2009-08-12 Thread Peter Rosin

Den 2009-08-11 12:30 skrev Dave Korn:

+AT_SETUP([dll basic test])
+
+AT_DATA([foo.c],[[
+int x=0;
+]])
+
+AT_DATA([baz.c],[[
+int y=0;
+]])
+
+AT_DATA([bar.c],[[
+extern int x;
+int bar(void);
+int bar() { return x;}
+]])
+
+AT_DATA([main.c],[[
+extern int x;
+extern int y;
+
+int main() {
+return x+y;
+}
+]])


*snip*


+AT_SETUP([dll install to bindir])
+
+AT_DATA([foo.c],[[
+int x=0;
+]])
+
+AT_DATA([bar.c],[[
+extern int x;
+int bar(void);
+int bar() { return x;}
+]])
+
+AT_DATA([baz.c],[[
+int y=0;
+]])


Missed this before, sorry. But since it is a well known fact that
exporting variables from libraries are bad and should be avoioded,
can we please stop adding more of that practice to the test suite?
There is enough of it already and in case it fails, other failures
might be hidden.

Cheers,
Peter





Re: [PATCH, take 3][cygwin|mingw] Control where win32 DLLs get installed.

2009-08-12 Thread Ralf Wildenhues
Hello Dave,

* Dave Korn wrote on Tue, Aug 11, 2009 at 09:07:12AM CEST:
>
>   Well, the bindir option exists only to support PE DLLs,

Bzzt.  First error.  If libtool provides -bindir, then it should accept
it on every system, so writing portable makefiles is made easy; and your
test addition should ensure that it does so.  Of course on non-PE
systems, it should just ignore the option silently.  libtool aims to
provide a uniform interface to users.

So it's only the PE-specific bits of the test that should be skipped on
systems where they don't apply.

Thus, bindir.at is a sensible name.  Do you intend for Automake to pass
-bindir to libtool --mode=link invocations automatically (maybe for
_LTLIBRARIES with  not equal to lib or libexec)?

* Dave Korn wrote on Tue, Aug 11, 2009 at 10:35:28AM CEST:
>   All rebuilt OK.  Checked docs with "make dvi info pdf" and viewing the
> generated file.  Testsuite re-run is still in progress, but I already checked
> that the new tests all still pass, so unless I post back saying otherwise in 
> the
> next couple of hours, assume the lot completed without regressions.

Tested on what system(s)?

* Dave Korn wrote on Tue, Aug 11, 2009 at 12:30:47PM CEST:
> 
> libtool/ChangeLog:
> 
>   * Makefile.am (TESTSUITE_AT): Add pe-dll-inst-bindir.at.
>   * libltdl/config/general.m4sh (func_relative_path): New function.
>   * libltdl/config/ltmain.m4sh (func_mode_link): Accept new -bindir
>   option and use it, if supplied, to place Windows DLLs.
>   * tests/pe-dll-inst-bindir.at: New file for DLL install tests.
>   * doc/libtool.texi (Link Mode): Update documentation.

BTW, even if the part going into GCC is covered by your GCC assignment,
the rest is still sufficiently nontrivial to warrant an assignment.

> diff --git a/Makefile.am b/Makefile.am
> old mode 100644
> new mode 100755

Your files suffer from mode changes.  They are of course not acceptable,
though I understand they are a w32 artifact.  Can git be made to ignore
them for you?

> index a18955e..fdeeffd
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -494,7 +494,8 @@ TESTSUITE_AT  = tests/testsuite.at \
> tests/configure-iface.at \
> tests/stresstest.at \
> tests/cmdline_wrap.at \
> -   tests/darwin.at
> +   tests/darwin.at \
> +   tests/pe-dll-inst-bindir.at

tests/bindir.at, as already noted above; and as this is about
'libtool' (the script) API, please sort it alongside the other API
tests, preferably before or after cwrapper.at.  Thanks.

> --- a/doc/libtool.texi
> +++ b/doc/libtool.texi
> @@ -1376,6 +1376,15 @@ Tries to avoid versioning (@pxref{Versioning}) for 
> libraries and modules,
>  i.e.@: no version information is stored and no symbolic links are created.
>  If the platform requires versioning, this option has no effect.
>  
> +...@item -bindir
> +When linking a DLL for Windows or another PE platform, this option tells

What does this have to do with PE?  All this is about is that there is
no real, independent $shlibpath_var beside PATH.  I'm OK with mentioning
that Windows is the sole current user of this, but please let's word
this in a way that doesn't require us to change the interface if some
other system requires it, too.  Ideally, neither the text.

> +libtool where to eventually install the @samp{.dll} file. The output path
> +is used to install the @samp{.la} control file, usually into a 
> @samp{.../lib/}
> +subdirectory of the @var{prefix}; except in the case of a dlopen()-able
> +module (@pxref{Modules for libltdl}), it is usually desirable to install the
> +DLL into a @samp{.../bin/} directory alongside. This option specifies the
> +absolute path to the @var{bindir}.

> @@ -1460,7 +1469,10 @@ the @option{-version-info} flag instead 
> (@pxref{Versioning}).
>  @item -rpath @var{libdir}
>  If @var{output-file} is a library, it will eventually be installed in
>  @var{libdir}.  If @var{output-file} is a program, add @var{libdir} to
> -the run-time path of the program.
> +the run-time path of the program.  If @var{output-file} is a Windows
> +(or other PE platform) DLL, the @samp{.la} control file will be
> +installed in @var{libdir}, but see @option{-bindir} above for the
> +eventual destination of the @samp{.dll} file itself.
>  
>  @item -R @var{libdir}
>  If @var{output-file} is a program, add @var{libdir} to its run-time

> --- a/libltdl/config/general.m4sh
> +++ b/libltdl/config/general.m4sh
> @@ -100,6 +100,76 @@ func_dirname_and_basename ()
>  
>  # Generated shell functions inserted here.
>  
> +# func_relative_path libdir bindir
> +# generates a relative path from LIBDIR to BINDIR, intended
> +# for supporting installation of windows DLLs into -bindir.

gnulib-tool has a function func_relativize.  Can we just reuse that
(and make it fast)?  Failing that, did you write this function from
scratch or took it from anywhere else.

I don't see any reason this function should be written spec

Re: [PATCH, take 3][cygwin|mingw] Control where win32 DLLs get installed.

2009-08-12 Thread Ralf Wildenhues
* Dave Korn wrote on Thu, Aug 13, 2009 at 12:50:22AM CEST:
> Dave Korn wrote:
> 
> > Now we twiddle our thumbs and wait for the paperwork, I guess!
> 
>   Hmm.  Or potentially not.

I'll have a review for the patch; at least for Libtool, it needs further
work.

Cheers,
Ralf




Re: [PATCH, take 3][cygwin|mingw] Control where win32 DLLs get installed.

2009-08-12 Thread Dave Korn
Dave Korn wrote:

> Now we twiddle our thumbs and wait for the paperwork, I guess!

  Hmm.  Or potentially not.  I just submitted the patch (backported to GCC's
forked libtool) to the gcc-patches list(*).  As far as I understand it, that
means I have now assigned my copyright in the patch to the FSF via my GCC
assign; or possibly I have expressed a willingness to assign my copyright
which would be completed by the patch being approved and committed; but either
way, the FSF now already is or is about to become the proper legal owner of
the copyright, no?

cheers,
  DaveK

-- 
(*) - http://gcc.gnu.org/ml/gcc-patches/2009-08/msg00661.html