Re: bug#13414: Valid DLL def file mangled by libtool
Erik van Pienbroek schreef op zo 20-01-2013 om 15:54 [+0100]: Peter Rosin schreef op zo 20-01-2013 om 08:32 [+0100]: But I will hold off the push a couple of days pending feedback from those actually using .def files for real things. Thanks for the patch. To help out with testing I can perform a test mass rebuild of all mingw packages which are currently in Fedora (about 135) against a libtool package which contains this patch. With this mass rebuild we can at least find out whether the patch introduces regressions or not. Running such a test mass rebuild will take about 36 hours to complete, so I'll give an update in about 2 days. Hi, The test mass rebuild of all Fedora MinGW packages against the latest libtool with the proposed patches was successful. The packages which required workarounds in order to get built can now be built without these workarounds, so the original issue is resolved with the proposed patches. All other packages were rebuilt successfully as well, so from my point of view I'm +1 to the proposed patches. Regards, Erik van Pienbroek Fedora MinGW SIG
Re: bug#13414: Valid DLL def file mangled by libtool
close 13414
thanks
Hi!
On 2013-01-22 19:45, Erik van Pienbroek wrote:
Erik van Pienbroek schreef op zo 20-01-2013 om 15:54 [+0100]:
Peter Rosin schreef op zo 20-01-2013 om 08:32 [+0100]:
But I will hold off the push a couple of days pending feedback from
those actually using .def files for real things.
Thanks for the patch. To help out with testing I can perform a test mass
rebuild of all mingw packages which are currently in Fedora (about 135)
against a libtool package which contains this patch. With this mass
rebuild we can at least find out whether the patch introduces
regressions or not. Running such a test mass rebuild will take about 36
hours to complete, so I'll give an update in about 2 days.
The test mass rebuild of all Fedora MinGW packages against the latest
libtool with the proposed patches was successful. The packages which
required workarounds in order to get built can now be built without
these workarounds, so the original issue is resolved with the proposed
patches. All other packages were rebuilt successfully as well, so from
my point of view I'm +1 to the proposed patches.
Ok, that's good enough for me! Thanks again for testing.
The patches tested by Erik where slightly different from the ones I
posted. I did the whitespace changes in a separate patch and fixed the
copyright year thing spotted by Gary. I also changed
IFS=' ''
'
into
IFS=$sp$nl
The patches tested where also slightly different from the ones I
actually pushed. The pushed version does not have the unneeded quotes
in the above assignment and I changed one instance of
if ! foo; then
...
fi
into
foo || {
...
}
since if ! foo isn't as portable according to Autoconf docs.
Anyway, those last changes are all harmless, I'm just putting all
cards on the table just in case...
Cheers,
Peter
Re: bug#13414: Valid DLL def file mangled by libtool
Peter Rosin schreef op zo 20-01-2013 om 08:32 [+0100]: But I will hold off the push a couple of days pending feedback from those actually using .def files for real things. Thanks for the patch. To help out with testing I can perform a test mass rebuild of all mingw packages which are currently in Fedora (about 135) against a libtool package which contains this patch. With this mass rebuild we can at least find out whether the patch introduces regressions or not. Running such a test mass rebuild will take about 36 hours to complete, so I'll give an update in about 2 days. Kind regards, Erik van Pienbroek Fedora MinGW SIG
Re: bug#13414: Valid DLL def file mangled by libtool
On 2013-01-19 06:12, Gary V. Vaughan wrote: Hi Peter, Thanks for working on this. On 19 Jan 2013, at 05:55, Peter Rosin p...@lysator.liu.se wrote: On 2013-01-12 01:26, Peter Rosin wrote: On 2013-01-11 12:34, Martin Doucha wrote: I'd like to report a bug in libtool 2.4 (including the latest git revision) which mangles valid DLL def files under MinGW and makes the linker barf. This issue has been reported before [1]. So, as hinted above, I'm following up with a pair of patches that appear to mend this. Ok to push? By inspection, these patches look good to me - presuming there are no regressions, please go ahead. I have found no regressions, and thanks for the speedy review! One nit: your new test has a Copyright notice starting at 2007 followed by written in 2013. The new code doesn't look derivative of existing tests, so I'd suggest deleting the years prior to 2013 before pushing. Done. Or are the white-space changes in the first patch too intrusive? If you would like to separate those into a separate patch then please feel free; but I'd rather see functional progress in Libtool development than being overly anal about changeset minutiae for potential future git archaeology at the expense of using your Libtool hacking time more wisely :) Splitting the commit in two is simple enough, takes little time to do and I don't feel obliged to test the intermediate state, so I just did it. But I will hold off the push a couple of days pending feedback from those actually using .def files for real things. Cheers, Peter
Re: bug#13414: Valid DLL def file mangled by libtool
Hi Martin, Erik, On 2013-01-12 01:26, Peter Rosin wrote: Hi Martin, Thanks for the report. On 2013-01-11 12:34, Martin Doucha wrote: Hi, I'd like to report a bug in libtool 2.4 (including the latest git revision) which mangles valid DLL def files under MinGW and makes the linker barf. The bug is caused by incorect check for EXPORTS keyword in the def file which libtool does this way: if test x`$SED 1q $export_symbols` != xEXPORTS; then ... [add another EXPORTS line at the beginning of file] This test is incorrect because the EXPORTS keyword does not have to appear on the very first line. You should replace the test with this: if test x`$GREP EXPORTS $export_symbols` != xEXPORTS; then ... Also note that this test appears on several places throughout libtool sources (both as xEXPORTS and just EXPORTS) so you need to fix all of them. This issue has been reported before [1]. It's been on my back burner for a while, but I've been held up by build system issues. At least, that's my excuse :-) Anyway, I think your suggested alternative with grep is a bit too relaxed, as any symbol involving the substring EXPORTS would trigger it. Also, it scans the whole file, which is suboptimal. I'm looking into a patch that uses $SED -n \ -e '/^[ ]*//' \ -e '/^;/D' \ -e '/^$/D' \ -e 's/^EXPORTS.*/DEF/p' \ -e 's/^LIBRARY.*/DEF/p' \ -e q \ $export_symbols instead (coupled with a test for DEF instead, naturally). Does anybody have any issues with such a beast? Yes, I know that it will not catch any valid .def file, but I don't fancy writing a full .def file parser for this [2]. The above steps past initial comment and whitespace lines waiting for the first real line, and triggers if it starts with EXPORTS or LIBRARY (at least, that's the intention...) [1] http://lists.gnu.org/archive/html/libtool/2012-02/msg00023.html [2] http://msdn.microsoft.com/en-us/library/h41zhe21(v=vs.110).aspx I'm back, with suggested changes against latest git, and I'm curious if it is enough to solve your problem? If you are not able to check for some reason, it might be possible for you to provide the .def file you had the problem with? (this question was mainly for Martin, Erik had enough specifics in his report) Also, while I recognize that my evaluation of Martin's patch was flawed in that his grep-based patch doesn't trigger on any symbol with EXPORTS as a substring (which I stated, I was using the mental model of my patch on his code, and stumbled), it still reads the whole .def file and doesn't catch .def files with a symbol on the same line as the EXPORTS keyword... So, as hinted above, I'm following up with a pair of patches that appear to mend this. Ok to push? Or are the white-space changes in the first patch too intrusive? Cheers, Peter
Re: bug#13414: Valid DLL def file mangled by libtool
Hi Peter, Thanks for working on this. On 19 Jan 2013, at 05:55, Peter Rosin p...@lysator.liu.se wrote: On 2013-01-12 01:26, Peter Rosin wrote: On 2013-01-11 12:34, Martin Doucha wrote: I'd like to report a bug in libtool 2.4 (including the latest git revision) which mangles valid DLL def files under MinGW and makes the linker barf. This issue has been reported before [1]. So, as hinted above, I'm following up with a pair of patches that appear to mend this. Ok to push? By inspection, these patches look good to me - presuming there are no regressions, please go ahead. One nit: your new test has a Copyright notice starting at 2007 followed by written in 2013. The new code doesn't look derivative of existing tests, so I'd suggest deleting the years prior to 2013 before pushing. Or are the white-space changes in the first patch too intrusive? If you would like to separate those into a separate patch then please feel free; but I'd rather see functional progress in Libtool development than being overly anal about changeset minutiae for potential future git archaeology at the expense of using your Libtool hacking time more wisely :) Cheers, -- Gary V. Vaughan (gary AT gnu DOT org)
