Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 7)

2010-07-06 Thread Charles Wilson
On 7/4/2010 2:29 PM, Ralf Wildenhues wrote:
 * Charles Wilson wrote on Sun, Jul 04, 2010 at 06:15:06AM CEST:
 So...this is what I intend to push, barring objections.
 
 No objections from me, please wait however long I asked you to wait
 (basically long enough so others have had a chance to chime in).

After waiting the requested amount, pushed.  Thanks for the reviews,
folks.  Up next: the cross-compile stuff, and a test case or two
covering the dlpreopen issues.

--
Chuck



Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 8?)

2010-07-05 Thread Gary V. Vaughan
Hi Chuck,

On 3 Jul 2010, at 22:34, Charles Wilson wrote:
 It's non-timely and off-point reviews that I tire of.
 
 The non-timely bit is just a reflection of the manpower and free time
 issues that all open source projects are subject to, so that kinda goes
 with the territory.  Nobody likes it, but...you just gotta live with it.
 
 By off-point I mean discussing non-germane or wishlist items as part
 of a review.  If the reviewer isn't VERY careful, such off-topic dicta
 can appear to imply that acceptance of a particular patch is predicated
 on solving longstanding wishlist items or software design misfeatures
 that long predate the patch in question.
 
 Most potential contributors -- myself included -- are scratching their
 own itch: X doesn't work, and I want to fix it.  It's discouraging to be
 told (or THINK you are told) we won't fix X until you or somebody else
 fixes huge, gaping, gargantuan problems Y and Z. Those problems are
 really hard to solve, and have existed for years. They are SO hard that
 none of US experts have even tried to tackle it.  BUT...we won't accept
 your patch for X until somebody steps up to the plate for Y and Z
 
 Which...sounds a whole lot like We appreciate the submission of your
 manuscript The Life and Times of an New York Meter Maid. However, at
 this time there are no opportunities for additional entries in our New
 York True Life book series. Thank you for your interest in Bob's
 Publishing Company, and Keep Writing!
 
 Unless the contributor of patch X goes off to scratch YOUR itch
 regarding Y and Z. That's not the way free software contributors are
 best motivated.
 
 On 6/28/2010 3:23 AM, Gary V. Vaughan wrote:
 Nevertheless, please do remember that it is a *review*, and if you find
 yourself disagreeing with something (excepting an outright veto of course),
 Ralf and I are both acutely aware that you are the one doing the work on
 these patches and the last thing we want to do is retard the progress of
 Libtool on Windows, so please don't be afraid to say on balance, I'd
 rather see this patch move Libtool forward in some small way without
 addressing issue X right now.  Often, we'll concede in exchange for a
 TODO item! :)
 
 That's an...interesting take.  I've never assumed that ANY contribution
 would be acceptable unless it received an actual approval by a
 maintainer.  I mean, really: here's this patch, and no single maintainer
 has endorsed it without some significant objection -- and I should feel
 free as a non-maintainer to say well, I disagree, so I'm pushing anyway??

Just to clarify, that isn't what I meant -- rather, well, I disagree, and
as you can see, despite its faults, patch X alone already makes things
less bad than they were previously.  Is it okay if I push X in its current
state, and then tackle Y later? I've no inclination to work on Z, but
I'll put a reference to this thread in libtool/TODO so it isn't entirely
forgotten.

And I believe that we'll often say sure, or good point, go ahead.

 Maybe you mean after a few more rounds of negotiation on the mailing
 list, maintainers may acquiesce with reservation to an un-revised
 version of particular patch...

That too.

 On 28 Jun 2010, at 13:10, Ralf Wildenhues wrote:
 * Charles Wilson wrote on Mon, Jun 28, 2010 at 12:05:40AM CEST:
 On 6/27/2010 4:43 PM, Ralf Wildenhues wrote:
 I don't see this method as the new method of choice.  We already have a
 mechanism for years to transport values to the libtool script with
 _LT_DECLs and _LT_TAGDECLs, and at least for small code snippets,
 
 Yeah, that's the problem.
 
 I wrote _LT_DECL and _LT_TAGDECL to propagate shell variable declarations to
 the libtool script, and I fear it will behave badly if we try to use that
 mechanism for shoehorning anything else through, especially because it works
 by doing a *lot* of booking-keeping at m4 time.
 
 That's what I thought.  Windows postinstall_cmds is pretty much the
 outer limit, IMO:
 
 postinstall_cmds=base_file=\\\`basename \\\${file}\\\`~
  dlpath=\\\`\$SHELL 21 -c '. \$dir/'\\\${base_file}'i; echo
 \\\$dlname'\\\`~
  dldir=\$destdir/\\\`dirname \\\$dlpath\\\`~
  test -d \\\$dldir || mkdir -p \\\$dldir~
  \$install_prog \$dir/\$dlname \\\$dldir/\$dlname~
  chmod a+x \\\$dldir/\$dlname~
  if test -n '\$stripme'  test -n '\$striplib'; then
eval '\$striplib \\\$dldir/\$dlname' || exit \\\$?;
  fi

Agreed.

 _LT_PROG_XSI_REPLACE is designed for swapping out fallback implementations
 of full functions (suitably decorated) for more efficient implementations
 based on the build-time environment.  I think that is exactly what you're
 trying to do, but it seems to me like you might be able to work more
 effectively in reverse: by putting the full Windows required implementations
 into ltmain.m4sh, and using _LT_PROG_XSI_REPLACE to replace them with
 stubs when configure is not building on (or for!!) a Windows machine?
 
 Well, my version _LT_PROG_FUNCTION_REPLACE is 

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 8?)

2010-07-04 Thread Bob Friesenhahn

On Sat, 3 Jul 2010, Charles Wilson wrote:



No one is threatening your committer status.


I didn't say they were.  But if I *did* misbehave -- well, I could
hardly be surprised by the inevitable consequences, could I? Doesn't
take a genius to predict those consequences, either.


Misbehavior is characterized by intent and subsequent acceptance of 
responsibility.  If someone commits a change which de-stabilizes the 
software, then they should expect to take responsibility for the 
clean-up.  Likewise, committing a change which will knowingly 
de-stabilize the software is harmful, unless there is a plan made in 
advance for how the issue will soon be rectified.  Sometimes temporary 
instability is ok because the end justifies the means.


Bob
--
Bob Friesenhahn
bfrie...@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer,http://www.GraphicsMagick.org/



Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 7)

2010-07-04 Thread Ralf Wildenhues
* Charles Wilson wrote on Sun, Jul 04, 2010 at 06:15:06AM CEST:
 On 6/26/2010 2:51 PM, Ralf Wildenhues wrote:
  OK.  Here's my take on this: if you fix all nits I noted inline below,
  post the updated and tested patch (you decide what testing is needed),
  then you are OK to commit after 72 hours of waiting.  FWIW, I'm likely
  not available most of next week; if we find issues later, then I guess
  they'll just have to be fixed afterwards.
 
 As discussed previously, this version fixes (almost) all of the noted
 issues. I didn't change the eval stuff, deferring instead to Ralf's
 promised patch to take care of all of that at once.

Thanks for the re-do.

 112: override pic_flag at configure time FAILED (pic_flag.at:48)

I will fix this failure.  The test has a few issues, also on other
systems.

 112 appears to be a new test, and is ELF specific (-fpic/-fPIC has no
 meaning on cygwin).  It probably should be skipped.
 
 
 So...this is what I intend to push, barring objections.

No objections from me, please wait however long I asked you to wait
(basically long enough so others have had a chance to chime in).

Thanks,
Ralf



Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 8?)

2010-07-04 Thread Ralf Wildenhues
Hi Charles,

* Charles Wilson wrote on Sat, Jul 03, 2010 at 06:10:57PM CEST:
 On 6/28/2010 2:10 AM, Ralf Wildenhues wrote:
  * Charles Wilson wrote on Mon, Jun 28, 2010 at 12:05:40AM CEST:
   It obviously isn't SUPPOSED to be dead -- or it wouldn't be there.
  
  Well, I wouldn't put my money on that reasoning.
 
 g.  Except that I do recall that back-in-the-day, this code WAS
 actually used.  That's why I had to change it as I did, before your
 suggestion in Jan 2009 to use the save-.la-name-in-a-custom-variable
 approach.

OK, thanks.

  I feel (more) discouraged now (than usual), having wasted so much time
  addressing a criticism of a patch that wasn't meant to be taken seriously.
  
  I would like to apologize for this comment making you do this extra
  work.  Again, that review of mine was more sloppy than it should have
  been.
 
 Accepted (Although you didn't actually 'make' me do the extra work.
 Your review did not actually REQUIRE it -- but your increasing
 unhappiness with $host-specific code to ltmain.m4sh made it appear to me
 that tackling it now -- before the cross-compile patch comes up for
 review again -- was a good idea.  It wasn't.)
 
 I apologize also for letting my frustration overtake my good sense. I
 shouldn't have complained as...vociferously. You're just doing your job:
 reviewing code to make sure libtool is as good as it can be.

No hard feelings at all on my side.  Except that I do have somewhat of a
bad conscience for letting all the w32 stuff go on for so long.

Let's hope we can improve that in the future, too.

 I guess the issue is, the shared library model of PE/COFF is just so
 different than ELF that the differences, to me, just don't seem to be
 the kind of thing that can be handled by m4 code -- at least, given the
 current architecture of the libtool script.  Now, if the ENTIRE body of
 'libtool' were generated from libtool.m4, rather than the bulk of it
 being presented in ltmain.m4sh...then maybe the skeleton could be more
 platform-agnostic.
 
 But, two things: (1) this means moving a LOT of what we probably
 consider generic code into libtool.m4 (imagine what that m4 would have
 to look like, to eliminate ALL case $host statements), and (2) you'd
 basically end up with, effectively, two DIFFERENT scripts that each CALL
 themselves libtool.  The ELF-ish one would not look anything like
 the PE/COFF-ish one.
 
 Maybe that's the right thing to do...long term.
 
 But that's a long-term project...I was just trying to fix a single
 regression (that turned into a rabbit hole).

Yep.  I agree that a bigger cleanup could help here, and I agree that
it's better to tackle that as an orthogonal issue.  Maybe in the end
a different structuring will even be easier once all the w32 stuff
works, because then we can maybe see the bigger picture.

  Anyway, if we're going to try and nail down these aspects of the API, I
  think that's a good thing to do for libltdl2 (whether Gary's or some
  other brainstorm).
  
  Yep, I guess.
 
 I guess that's what I'm getting at: I think some of this ugliness is
 unavoidable given the major architectural differences between PE/COFF
 and ELF -- and the EXISTING division of labor between libtool.m4 and
 ltmain.m4sh.  Fixing it is going to require...*major* changes.
 
 Given that...unless we plan to DO those major rewrites now...harping on
 them with regards to w32 is counter-productive. Peter and I will
 certainly try to put code into libtool.m4, but...it's not clear exactly
 how successful it is possible for us to be, without beginning that major
 rewrite process.

OK.  It is certainly helpful for review if you mention this with some
code that could not end up in libtool.m4 that way.

I realize I'm violently agreeing with you here as well, because your
patch postings are usually very detailedly explained, as to which
approaches you took and why and why not.

  I'm sorry if review is painful to accept, and I
  don't on purpose try to review in a way making you do double work.
  That that has happened now is bad, sorry about that.
 
 No, I think you misunderstand. *Review* is good. Critical review is even
 better.
 
 But...the reason you found it so hard to review this patch -- I mean,
 really, having to review six different discussions spread out over two
 years? -- that's ridiculous! Who would expect THAT to happen quickly?
 But how did we GET to that point: it was because in each of those
 previous six attempts, the review process got stalled.

Yep.

 And that's the issue -- putting a hold on a patch or patch series with a
 I need to think about this...and then not actually following up. I'm
 not blameless: after a week or two of silence, I'd usually moved on to
 something else, and it might be a while before I come back to libtool.
  If *I* had kept up the pressure maybe we all would have been able to
 keep the details of the various discussions in our primary memory bank,
 AND resolved the issue(s) in just a month or two, rather than 25 

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 8?)

2010-07-03 Thread Charles Wilson
On 6/28/2010 3:23 AM, Gary V. Vaughan wrote:
 Hi Chuck,
 
 Thanks for persevering with the Windows support in Libtool.
 
 Regarding our patch review process, I honestly find the tough reviews
 valuable in keeping up the quality of my patches, not least because it
 makes me more careful not to leave loose ends in my submissions.

Sure. Tough reviews are fine.

It's non-timely and off-point reviews that I tire of.

The non-timely bit is just a reflection of the manpower and free time
issues that all open source projects are subject to, so that kinda goes
with the territory.  Nobody likes it, but...you just gotta live with it.

By off-point I mean discussing non-germane or wishlist items as part
of a review.  If the reviewer isn't VERY careful, such off-topic dicta
can appear to imply that acceptance of a particular patch is predicated
on solving longstanding wishlist items or software design misfeatures
that long predate the patch in question.

Most potential contributors -- myself included -- are scratching their
own itch: X doesn't work, and I want to fix it.  It's discouraging to be
told (or THINK you are told) we won't fix X until you or somebody else
fixes huge, gaping, gargantuan problems Y and Z. Those problems are
really hard to solve, and have existed for years. They are SO hard that
none of US experts have even tried to tackle it.  BUT...we won't accept
your patch for X until somebody steps up to the plate for Y and Z

Which...sounds a whole lot like We appreciate the submission of your
manuscript The Life and Times of an New York Meter Maid. However, at
this time there are no opportunities for additional entries in our New
York True Life book series. Thank you for your interest in Bob's
Publishing Company, and Keep Writing!

Unless the contributor of patch X goes off to scratch YOUR itch
regarding Y and Z. That's not the way free software contributors are
best motivated.

 Nevertheless, please do remember that it is a *review*, and if you find
 yourself disagreeing with something (excepting an outright veto of course),
 Ralf and I are both acutely aware that you are the one doing the work on
 these patches and the last thing we want to do is retard the progress of
 Libtool on Windows, so please don't be afraid to say on balance, I'd
 rather see this patch move Libtool forward in some small way without
 addressing issue X right now.  Often, we'll concede in exchange for a
 TODO item! :)

That's an...interesting take.  I've never assumed that ANY contribution
would be acceptable unless it received an actual approval by a
maintainer.  I mean, really: here's this patch, and no single maintainer
has endorsed it without some significant objection -- and I should feel
free as a non-maintainer to say well, I disagree, so I'm pushing anyway??

That seems like a fast way to lose committer status, IMO.

Maybe you mean after a few more rounds of negotiation on the mailing
list, maintainers may acquiesce with reservation to an un-revised
version of particular patch...

 On 28 Jun 2010, at 13:10, Ralf Wildenhues wrote:
 * Charles Wilson wrote on Mon, Jun 28, 2010 at 12:05:40AM CEST:
 On 6/27/2010 4:43 PM, Ralf Wildenhues wrote:
 I don't see this method as the new method of choice.  We already have a
 mechanism for years to transport values to the libtool script with
 _LT_DECLs and _LT_TAGDECLs, and at least for small code snippets,

 Yeah, that's the problem.
 
 I wrote _LT_DECL and _LT_TAGDECL to propagate shell variable declarations to
 the libtool script, and I fear it will behave badly if we try to use that
 mechanism for shoehorning anything else through, especially because it works
 by doing a *lot* of booking-keeping at m4 time.

That's what I thought.  Windows postinstall_cmds is pretty much the
outer limit, IMO:

postinstall_cmds=base_file=\\\`basename \\\${file}\\\`~
  dlpath=\\\`\$SHELL 21 -c '. \$dir/'\\\${base_file}'i; echo
\\\$dlname'\\\`~
  dldir=\$destdir/\\\`dirname \\\$dlpath\\\`~
  test -d \\\$dldir || mkdir -p \\\$dldir~
  \$install_prog \$dir/\$dlname \\\$dldir/\$dlname~
  chmod a+x \\\$dldir/\$dlname~
  if test -n '\$stripme'  test -n '\$striplib'; then
eval '\$striplib \\\$dldir/\$dlname' || exit \\\$?;
  fi

 _LT_PROG_XSI_REPLACE is designed for swapping out fallback implementations
 of full functions (suitably decorated) for more efficient implementations
 based on the build-time environment.  I think that is exactly what you're
 trying to do, but it seems to me like you might be able to work more
 effectively in reverse: by putting the full Windows required implementations
 into ltmain.m4sh, and using _LT_PROG_XSI_REPLACE to replace them with
 stubs when configure is not building on (or for!!) a Windows machine?

Well, my version _LT_PROG_FUNCTION_REPLACE is pretty slow: given the
issues I had with using sed to insert really complex function bodies
with internal quoting and their own sed scripts, I had to use the old
copy part of the lt output, 

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 8?)

2010-07-03 Thread Charles Wilson
On 6/28/2010 2:10 AM, Ralf Wildenhues wrote:
 * Charles Wilson wrote on Mon, Jun 28, 2010 at 12:05:40AM CEST:
 So...we APPEAR to have a bunch of dead code.
 
 I wasn't aware of that.  Sorry about the sloppy review.
 
  It obviously isn't
 SUPPOSED to be dead -- or it wouldn't be there.
 
 Well, I wouldn't put my money on that reasoning.

g.  Except that I do recall that back-in-the-day, this code WAS
actually used.  That's why I had to change it as I did, before your
suggestion in Jan 2009 to use the save-.la-name-in-a-custom-variable
approach.

 Which?
 
 I'd say the part that is easier for you, so I guess that would be
 committing all the code, including presumably-dead.  And maybe in a
 future patch adding a testsuite test that exercises the code.

I'll provide a test in the future that exercises building a dlpreopen
app against an 'installed' library for which there is no .la file.  It
won't actually exercise the dead code -- until it is made alive again.
So...the test will probably XFAIL.

But we'll approach that separately.

As for THIS patch, I'll revise version 7 to address your other comments,
post that as version 9 and then wait 72 hours, as:

http://lists.gnu.org/archive/html/libtool-patches/2010-06/msg00174.html
 OK.  Here's my take on this: if you fix all nits I noted inline below,
 post the updated and tested patch (you decide what testing is needed),
 then you are OK to commit after 72 hours of waiting.

 I feel (more) discouraged now (than usual), having wasted so much time
 addressing a criticism of a patch that wasn't meant to be taken seriously.
 
 I would like to apologize for this comment making you do this extra
 work.  Again, that review of mine was more sloppy than it should have
 been.

Accepted (Although you didn't actually 'make' me do the extra work.
Your review did not actually REQUIRE it -- but your increasing
unhappiness with $host-specific code to ltmain.m4sh made it appear to me
that tackling it now -- before the cross-compile patch comes up for
review again -- was a good idea.  It wasn't.)

I apologize also for letting my frustration overtake my good sense. I
shouldn't have complained as...vociferously. You're just doing your job:
reviewing code to make sure libtool is as good as it can be.

 In fact, I have often wondered if the reason many of my patches -- and
 Peter's -- tend to languish so long is because of these aesthetic
 objections
 
 Of course code maintenance aspects and long-term slowdown of the code
 are a part of code quality.

As they should be.

I guess the issue is, the shared library model of PE/COFF is just so
different than ELF that the differences, to me, just don't seem to be
the kind of thing that can be handled by m4 code -- at least, given the
current architecture of the libtool script.  Now, if the ENTIRE body of
'libtool' were generated from libtool.m4, rather than the bulk of it
being presented in ltmain.m4sh...then maybe the skeleton could be more
platform-agnostic.

But, two things: (1) this means moving a LOT of what we probably
consider generic code into libtool.m4 (imagine what that m4 would have
to look like, to eliminate ALL case $host statements), and (2) you'd
basically end up with, effectively, two DIFFERENT scripts that each CALL
themselves libtool.  The ELF-ish one would not look anything like
the PE/COFF-ish one.

Maybe that's the right thing to do...long term.

But that's a long-term project...I was just trying to fix a single
regression (that turned into a rabbit hole).

 Anyway, if we're going to try and nail down these aspects of the API, I
 think that's a good thing to do for libltdl2 (whether Gary's or some
 other brainstorm).
 
 Yep, I guess.

I guess that's what I'm getting at: I think some of this ugliness is
unavoidable given the major architectural differences between PE/COFF
and ELF -- and the EXISTING division of labor between libtool.m4 and
ltmain.m4sh.  Fixing it is going to require...*major* changes.

Given that...unless we plan to DO those major rewrites now...harping on
them with regards to w32 is counter-productive. Peter and I will
certainly try to put code into libtool.m4, but...it's not clear exactly
how successful it is possible for us to be, without beginning that major
rewrite process.

 Yes, the literal you tried is already split into words, so word
 splitting of the shell would not make any difference.

Ah.

 And yes, it may happen that I spend a large amount of time trying to
 review patches well.  I didn't spend so much time this time, i.e., I
 did glance at the 6 takes and discussion threads of this patch
 beforehand, but not in large detail.  You can't have both, good review
 and timely review. 

g

 I'm sorry if review is painful to accept, and I
 don't on purpose try to review in a way making you do double work.
 That that has happened now is bad, sorry about that.

No, I think you misunderstand. *Review* is good. Critical review is even
better.

But...the reason you found it so hard to review this 

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 8?)

2010-07-03 Thread Bob Friesenhahn

On Sat, 3 Jul 2010, Charles Wilson wrote:


That's an...interesting take.  I've never assumed that ANY contribution
would be acceptable unless it received an actual approval by a
maintainer.  I mean, really: here's this patch, and no single maintainer
has endorsed it without some significant objection -- and I should feel
free as a non-maintainer to say well, I disagree, so I'm pushing anyway??


I think that you are attributing to much special status to official 
maintainers.  It should not matter where approval comes from as long 
as the approval is from a sufficiently qualified person with knowledge 
of the subject who has done a proper review and/or actual testing. 
The official maintainers are sometimes not qualified to properly 
review a patch.



That seems like a fast way to lose committer status, IMO.


While I am sure that it is possible to lose committer status due to 
misbehavior, I don't recall this happening in libtool history for any 
reason other than the person disappeared from the Internet or 
requested that their committer status be removed.  No one is 
threatening your committer status.


Bob
--
Bob Friesenhahn
bfrie...@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer,http://www.GraphicsMagick.org/



Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 8?)

2010-07-03 Thread Charles Wilson
On 7/3/2010 7:05 PM, Bob Friesenhahn wrote:
 On Sat, 3 Jul 2010, Charles Wilson wrote:

 That's an...interesting take.  I've never assumed that ANY contribution
 
 I think that you are attributing to much special status to official
 maintainers.  It should not matter where approval comes from as long
 as the approval is from a sufficiently qualified person with knowledge
 of the subject who has done a proper review and/or actual testing. The
 official maintainers are sometimes not qualified to properly review a
 patch.

Maybe so...

 That seems like a fast way to lose committer status, IMO.
 
 While I am sure that it is possible to lose committer status due to
 misbehavior, I don't recall this happening in libtool history for any
 reason other than the person disappeared from the Internet or requested
 that their committer status be removed.  

Of course not; we wouldn't be very likely to grant such status in the
first place to people liable to abuse it.

 No one is threatening your
 committer status.

I didn't say they were.  But if I *did* misbehave -- well, I could
hardly be surprised by the inevitable consequences, could I? Doesn't
take a genius to predict those consequences, either.

--
Chuck



Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 7)

2010-07-03 Thread Charles Wilson
On 6/26/2010 2:51 PM, Ralf Wildenhues wrote:
 OK.  Here's my take on this: if you fix all nits I noted inline below,
 post the updated and tested patch (you decide what testing is needed),
 then you are OK to commit after 72 hours of waiting.  FWIW, I'm likely
 not available most of next week; if we find issues later, then I guess
 they'll just have to be fixed afterwards.

As discussed previously, this version fixes (almost) all of the noted
issues. I didn't change the eval stuff, deferring instead to Ralf's
promised patch to take care of all of that at once.

Test results (cygwin):

==
All 122 tests passed
(2 tests were not run)
==

 78: dlloader APIFAILED (dlloader-api.at:422)
112: override pic_flag at configure time FAILED (pic_flag.at:48)

ERROR: 110 tests were run,
5 failed (3 expected failures).
6 tests were skipped.


78 is expected at this time (patch to fix it hasn't been pushed yet).
112 appears to be a new test, and is ELF specific (-fpic/-fPIC has no
meaning on cygwin).  It probably should be skipped.


So...this is what I intend to push, barring objections.

[cygwin|mingw] fix dlpreopen with --disable-static

* libltdl/config/general.m4sh (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_fallback_core): New function.
(func_cygming_gnu_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.

--
Chuck
diff --git a/libltdl/config/general.m4sh b/libltdl/config/general.m4sh
index ab79f05..5d7bef7 100644
--- a/libltdl/config/general.m4sh
+++ b/libltdl/config/general.m4sh
@@ -594,4 +594,21 @@ 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 d2676f9..dc08696 100644
--- a/libltdl/config/ltmain.m4sh
+++ b/libltdl/config/ltmain.m4sh
@@ -1987,10 +1987,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 import library.
+	$sharedlib_from_linklib $dlprefile
+	

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 8?)

2010-06-28 Thread Ralf Wildenhues
Hello Charles,

* Charles Wilson wrote on Mon, Jun 28, 2010 at 12:05:40AM CEST:
 On 6/27/2010 4:43 PM, Ralf Wildenhues wrote:
  * Charles Wilson wrote on Sun, Jun 27, 2010 at 02:51:21PM CEST:
  So...can I get a verdict?  Is -dlpreopen not-an-la-file supposed to work?
  
  I think Pierre's report was about using -dlpreopen file.la at link time,
  but then not wanting to need the .la file at run time.  I think that is
  desirable.  At link edit time, having a .la file is a reasonable thing I
  think.
 
 So...I *don't* need to worry about -dlpreopen not-an-.la?  The issue is
 that I can't figure out how *current* libtool EVER gets here:
 
 (current master ltmain.m4sh:1984:func_generate_dlsyms)
 
   for dlprefile in $dlprefiles; do
 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'
 }
   done
 
 in that situation, with anything IN $dlprefiles -- because in
 ltmain.m4sh, we have:
 
 5764 if test $linkmode = prog; then
 5765   dlfiles=$newdlfiles
 5766 fi
 5767 if test $linkmode = prog || test $linkmode = lib; then
 5768   dlprefiles=$newdlprefiles
 5769 fi
 
 and at this point, both newdlfiles and newdlfiles are empty, when the
 argument to libtool's -dlpreopen option is not a libtool .la library.
 
 So...we APPEAR to have a bunch of dead code.

I wasn't aware of that.  Sorry about the sloppy review.

  It obviously isn't
 SUPPOSED to be dead -- or it wouldn't be there.

Well, I wouldn't put my money on that reasoning.

  So, I can either keep
 (that is, commit) all of my new stuff in this patch, some of which will
 also be dead code, in anticipation of somebody tracking down WHY it and
 these existing snippets are (currently) dead, and brings them back to life.
 
 Or I can NOT commit any new (dead) code and commit only those bits that
 are presently live, and wait until after the existing dead code is
 resurrected, and THEN add those particular bits that I'd've held back.
 
 Which?

I'd say the part that is easier for you, so I guess that would be
committing all the code, including presumably-dead.  And maybe in a
future patch adding a testsuite test that exercises the code.

  I tried to use Gary's _LT_PROG_XSI_REPLACE function, but using a sed
  script to create a sed script and all the quoting nightmares just made
  my head hurt.  So, I have _LT_PROG_FUNCTION_REPLACE that uses the old
  'copy half the script, emit the new function content, copy the rest of
  the script' algorithm.
  
  I don't see this method as the new method of choice.  We already have a
  mechanism for years to transport values to the libtool script with
  _LT_DECLs and _LT_TAGDECLs, and at least for small code snippets,
 
 Yeah, that's the problem. You complained that these functions added a
 lot of parse time to all the other platforms that would never use them,
 presumably because they were BIG functions and there were several of
 them.  Presumably, the parse-time cost of small functions is low, unless
 there are a TON.

I think you can measure parse time in script length plus number of
here-documents (for old shells).

 But please, in the future, don't complain so strongly ([your patch]
 makes me cringe) about architectural issues if you don't actually want
 to see them fixed: system-specific code in ltmain...rather than in
 libtool.m4 where it belongs.
 
 I feel (more) discouraged now (than usual), having wasted so much time
 addressing a criticism of a patch that wasn't meant to be taken seriously.

I would like to apologize for this comment making you do this extra
work.  Again, that review of mine was more sloppy than it should have
been.

 In fact, I have often wondered if the reason many of my patches -- and
 Peter's -- tend to languish so long is because of these aesthetic
 objections

Of course code maintenance aspects and long-term slowdown of the code
are a part of code quality.

 Anyway, this patch AND that upcoming cross-compile patch both add
 several large system-specific new functions to ltmain.sh.  Since you
 objected to them now, I figured I'd hear it again THEN, so...I took this
 opportunity to TRY and create the appropriate infrastructure to handle
 LARGE system-specific functions from m4.  (Few if any of these functions
 are suitable candidates for single-line $foo_cmd _LT_DECL-style
 treatment, or even just few-line $foo_cmd)
 
 
 I won't bother to do that in the future.

Again, sorry for making you do extra work.

  because at least currently, the second entry in the
  LTX_preloaded_symbols array is cygfoo-N.dll in those circumstances,
  not libfoo.a.
  
  Well yeah, this confusion and interface non-well-definedness is bad, no?
 
 Sure it is.  But some of these considerations are hard to accommodate on
 win32 if the .la file is not available at runtime, AND the caller
 doesn't 

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 8?)

2010-06-28 Thread Gary V. Vaughan
Hi Chuck,

Thanks for persevering with the Windows support in Libtool.

Regarding our patch review process, I honestly find the tough reviews
valuable in keeping up the quality of my patches, not least because it
makes me more careful not to leave loose ends in my submissions.

Nevertheless, please do remember that it is a *review*, and if you find
yourself disagreeing with something (excepting an outright veto of course),
Ralf and I are both acutely aware that you are the one doing the work on
these patches and the last thing we want to do is retard the progress of
Libtool on Windows, so please don't be afraid to say on balance, I'd
rather see this patch move Libtool forward in some small way without
addressing issue X right now.  Often, we'll concede in exchange for a
TODO item! :)

On 28 Jun 2010, at 13:10, Ralf Wildenhues wrote:
 * Charles Wilson wrote on Mon, Jun 28, 2010 at 12:05:40AM CEST:
 On 6/27/2010 4:43 PM, Ralf Wildenhues wrote:
 * Charles Wilson wrote on Sun, Jun 27, 2010 at 02:51:21PM CEST:
 I tried to use Gary's _LT_PROG_XSI_REPLACE function, but using a sed
 script to create a sed script and all the quoting nightmares just made
 my head hurt.  So, I have _LT_PROG_FUNCTION_REPLACE that uses the old
 'copy half the script, emit the new function content, copy the rest of
 the script' algorithm.
 
 I don't see this method as the new method of choice.  We already have a
 mechanism for years to transport values to the libtool script with
 _LT_DECLs and _LT_TAGDECLs, and at least for small code snippets,
 
 Yeah, that's the problem.

I wrote _LT_DECL and _LT_TAGDECL to propagate shell variable declarations to
the libtool script, and I fear it will behave badly if we try to use that
mechanism for shoehorning anything else through, especially because it works
by doing a *lot* of booking-keeping at m4 time.

_LT_PROG_XSI_REPLACE is designed for swapping out fallback implementations
of full functions (suitably decorated) for more efficient implementations
based on the build-time environment.  I think that is exactly what you're
trying to do, but it seems to me like you might be able to work more
effectively in reverse: by putting the full Windows required implementations
into ltmain.m4sh, and using _LT_PROG_XSI_REPLACE to replace them with
stubs when configure is not building on (or for!!) a Windows machine?

(At that point, we should come up with a better name, and changing the
decorator strings to match. The XSI is already a misnomer now that I'm
using it for `+=' and ${foo:n:m} constructions.)

Cheers,
-- 
Gary V. Vaughan (g...@gnu.org)  



Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 8?)

2010-06-27 Thread Charles Wilson
On 6/26/2010 2:51 PM, Ralf Wildenhues wrote:
 * Charles Wilson wrote on Fri, Jun 25, 2010 at 04:57:15AM CEST:
 However, with this patch, helldl.exeS.c has:
 
 IOW, all the spurious declarations are gone?  That'd be cool.

Correct.

 (*) Note that you only need to determine the dll name for an import
 lib using dlltool --identify or the fallback, IF and ONLY IF you are
 linking to a library WITHOUT a corresponding .la file.

After trying unsuccesfully for hours to convince modern libtool to
actually allow me to DO this -- for testing purposes -- I wonder if
   -dlpreopen /usr/lib/somelib
where somelib is an .so or .a or .dll.a is ALLOWED at all.  I'm pretty
sure it USED to work -- like five or six years ago.  But it doesn't
appear to be documented to work that way, and I certainly can't figure
out how to get the libtool to even let me do it. It simply adds the
specified library to the link command, but never extracts a symbol table
for it.

If this is NOT actually supported, then I can simplify this patch quite
a bit I think. We'd no longer need ANY of the figure out the .dll name
from the .dll.a functions.

OTOH, if this mode IS supposed to be supported...then I think I've found
yet another bug, not covered by the test suite.  There's also this hint
that 1.5 years ago, I didn't explore this modality (-dlpreopen
not-an-la-file) very much:

http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg00056.html
 Limitation: although I have beat this sed-fu

sed-fu: refers to the giant sed script in
func_cygming_dll_for_implib_fallback_core

 to death *outside*
 of libtool, and am pretty confident it works well, there is no
 actual test of that code in the testsuite. This is because well-
 behaved libtool clients -- and our tests are actually well-behaved
 in this regard -- will only -dlpreopen *libtool*-built libraries.

e.g. .la files

 In that case, Ralf's suggested libfile_$(transliterated implib name)
 is used, because we have the .la file available which allows that
 shortcut.  The only time we need `dlltool --identify' is when
 dlpreopening a non-libtool implib, where we have no .la file.
 And the sed-fu is a fallback to THAT fallback.


So...can I get a verdict?  Is -dlpreopen not-an-la-file supposed to work?

 If you DO have
 a .la file for the library you're linking to, then the save the .la
 file using a magic shell variable approach is used.

 OK.  Here's my take on this: if you fix all nits I noted inline below,
 post the updated and tested patch (you decide what testing is needed),
 then you are OK to commit after 72 hours of waiting.  FWIW, I'm likely
 not available most of next week; if we find issues later, then I guess
 they'll just have to be fixed afterwards.

OK.

 I'll note though that this patch makes me cringe: it introduces more
 system-specific code in ltmain that is just bloat on other systems,
 rather than in libtool.m4 where it belongs. 

Well, I did manage to come up with a mechanism to move most of

func_cygming_dll_for_implib
func_cygming_dll_for_implib_fallback
func_cygming_dll_for_implib_fallback_core
func_cygming_gnu_implib_p
func_cygming_ms_implib_p

into libtool.m4 (attached; only lightly tested pending Q above).
However, I can't see how to move the other mods in func_generate_dlsyms
and func_mode_link there.

I tried to use Gary's _LT_PROG_XSI_REPLACE function, but using a sed
script to create a sed script and all the quoting nightmares just made
my head hurt.  So, I have _LT_PROG_FUNCTION_REPLACE that uses the old
'copy half the script, emit the new function content, copy the rest of
the script' algorithm.

 It has the potential to
 regress non-cygwin non-mingw w32 systems, and since we effectively have
 no cegcc maintainer that is a problem for cegcc (dunno if for us).
 The patch doesn't introduce a new test, so I assume it either fixes a
 current testsuite failure in either the default setup or in the
   ./configure --disable-static

It fixed an OLD failure in demo-shared/demo-make/demo-exec.  However,
this patch originally addressed 4 separate issues that were clustered
together -- but the demo-shared test failure actually exposed only one
of them.  In the interim, that particular issue was been, well, not
fixed I don't think, but rather avoided.  So, NOW, there is no actual
failure that this patch fixes.  The symbol list in demo-shared's .exe is
really ugly, but the test doesn't FAIL because of that.

 setup of Libtool.  If not, or if only the latter, then a testsuite
 addition in a followup patch would be nice.

OK, I'll look into that for a followup.

 Does this patch have any relation to Pierre Ossman's Preloading without
 .la patch?  http://thread.gmane.org/gmane.comp.gnu.libtool.general/7071
 His copyright papers are through now, so we can look at that patch.

Well, I *think* Pierre's patch would break cygwin -- but that'd be true
with or without this patch.  (The following statement in libltdl is not
true, for cygwin when -dlpreload and --disable-static):

/* 

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 8?)

2010-06-27 Thread Ralf Wildenhues
* Charles Wilson wrote on Sun, Jun 27, 2010 at 02:51:21PM CEST:
  In that case, Ralf's suggested libfile_$(transliterated implib name)
  is used, because we have the .la file available which allows that
  shortcut.  The only time we need `dlltool --identify' is when
  dlpreopening a non-libtool implib, where we have no .la file.
  And the sed-fu is a fallback to THAT fallback.
 
 
 So...can I get a verdict?  Is -dlpreopen not-an-la-file supposed to work?

I think Pierre's report was about using -dlpreopen file.la at link time,
but then not wanting to need the .la file at run time.  I think that is
desirable.  At link edit time, having a .la file is a reasonable thing I
think.

  I'll note though that this patch makes me cringe: it introduces more
  system-specific code in ltmain that is just bloat on other systems,
  rather than in libtool.m4 where it belongs. 
 
 Well, I did manage to come up with a mechanism to move most of
 
 func_cygming_dll_for_implib
 func_cygming_dll_for_implib_fallback
 func_cygming_dll_for_implib_fallback_core
 func_cygming_gnu_implib_p
 func_cygming_ms_implib_p
 
 into libtool.m4 (attached; only lightly tested pending Q above).
 However, I can't see how to move the other mods in func_generate_dlsyms
 and func_mode_link there.
 
 I tried to use Gary's _LT_PROG_XSI_REPLACE function, but using a sed
 script to create a sed script and all the quoting nightmares just made
 my head hurt.  So, I have _LT_PROG_FUNCTION_REPLACE that uses the old
 'copy half the script, emit the new function content, copy the rest of
 the script' algorithm.

I don't see this method as the new method of choice.  We already have a
mechanism for years to transport values to the libtool script with
_LT_DECLs and _LT_TAGDECLs, and at least for small code snippets, that
should be used.  I'd probably be more confident with code in ltmain that
you did test rather than a new transplanting method that has not been
tested well, and thus by definition has bugs.

  Does this patch have any relation to Pierre Ossman's Preloading without
  .la patch?  http://thread.gmane.org/gmane.comp.gnu.libtool.general/7071
  His copyright papers are through now, so we can look at that patch.
 
 Well, I *think* Pierre's patch would break cygwin -- but that'd be true
 with or without this patch.  (The following statement in libltdl is not
 true, for cygwin when -dlpreload and --disable-static):
 
 /* Preloaded modules are always named according to their old
archive name. */
 
 because at least currently, the second entry in the
 LTX_preloaded_symbols array is cygfoo-N.dll in those circumstances,
 not libfoo.a.

Well yeah, this confusion and interface non-well-definedness is bad, no?

 The title of Pierre's thread is a bit confusing (at least to me). What
 he is talking about is using libltdl at runtime, with a variety of names
 refering to the same library (module, module.la, module.a, etc).  That's
 why HE means by Preloading without .la. My Question above is about
 *build* time, when you're trying to specify -dlpreopen not-a-.la.

OK.

  +func_tr_sh ()
  +{
  +  case $1 in
  
  No double-quotes needed here.
 
 Even if $1 might have spaces? 

Yes.  The shell does not do word splitting on the right hand side of an
assignment and in the argument to 'case'.  Just try it:

  foo=with space; case $foo in *space*) echo whoo;; esac

  It's a pathname:
 
   func_tr_sh $dlprefile
 
 But, I guess, since dlprefile obtained as a member of a space-separated
 list, it BETTER not have any spaces in it. OK.

Irrelevant.

  +  eval '$ECHO : $dlprefile_dlbasename  $nlist'
  +  eval '$ECHO : $name   $nlist'
  
  Likewise.
 
 Those are (copies, adaptations of) pre-existing code:
 
 - $opt_dry_run || {
 -   eval '$ECHO : $name   $nlist'
 -   eval $NM $dlprefile 2/dev/null | $global_symbol_pipe 
 '$nlist'
 
 I don't feel comfortable folding in a change like that as part of this
 patch, but I'd be happy to supply a separate follow-on patch to change
 them all at once.

Don't worry.  I'm working on a related change anyway that fixes a lot of
the eval stuff.

  +fi
  +eval $NM $dlprefile 2/dev/null | $global_symbol_pipe |
  +  $SED -e '/I __imp/d' -e 's/I __nm_/D /;s/_nm__//'  
  '$nlist'
  
  This can probably have the outer double quotes removed, eval moved to
  $global_symbol_pipe, and quotes around $nlist removed.  Actually, don't
  change this, because it might be the eval isn't needed at all; I will
  check this and change all uses in libtool then.
 
 Again, this is a copy of pre-existing code, so...I'll leave this to you.

OK.

  +# func_cygming_dll_for_implib_fallback_core SECTION_NAME LIBNAMEs

  +# Echos the name of the DLL associated with the
  +# specified import library.
  
  You'd save a fork if this function stores its result in a variable.
  I'd just use sharedlib_from_linklib_result for that.
 
 I can't actually do this.  The problem is that the function 

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 8?)

2010-06-27 Thread Bob Friesenhahn

On Sun, 27 Jun 2010, Ralf Wildenhues wrote:


* Charles Wilson wrote on Sun, Jun 27, 2010 at 02:51:21PM CEST:

In that case, Ralf's suggested libfile_$(transliterated implib name)
is used, because we have the .la file available which allows that
shortcut.  The only time we need `dlltool --identify' is when
dlpreopening a non-libtool implib, where we have no .la file.
And the sed-fu is a fallback to THAT fallback.



So...can I get a verdict?  Is -dlpreopen not-an-la-file supposed to work?


I think Pierre's report was about using -dlpreopen file.la at link time,
but then not wanting to need the .la file at run time.  I think that is
desirable.  At link edit time, having a .la file is a reasonable thing I
think.


Probably I have not been paying enough attention to this topic stream 
and I might be misinterpreting the above.  While it is nice to dream 
about not having the .la files, the module loading can not work 
completely reliably without it.  A program may depend on preopened 
modules, dynamically loaded modules, and dynamically loaded DLLs which 
are loaded as dependencies.  The .la files contain the dependency 
chain information.  Using them allows a dependency library to be 
changed without breaking things.


Bob
--
Bob Friesenhahn
bfrie...@simple.dallas.tx.us, http://www.simplesystems.org/users/bfriesen/
GraphicsMagick Maintainer,http://www.GraphicsMagick.org/



Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static (take 7)

2010-06-26 Thread Ralf Wildenhues
Hi Charles,

* Charles Wilson wrote on Fri, Jun 25, 2010 at 04:57:15AM CEST:
[...]
 Without this patch, when --disable-static on PE/COFF platforms,
 dlpreopen symbols are extracted incorrectly (because libtool uses
 the same algorithm for extracting symbols from import libs as
 from static libs; when both are present, the static lib is used
 to determine the symbol names even when linking dynamically --
 which is another mistake).

 However, with this patch, helldl.exeS.c has:
 
 /* External symbol declarations for the compiler. */
 extern int foo();
 extern int hello();
 extern char nothing;

IOW, all the spurious declarations are gone?  That'd be cool.

 Which is exactly what we want.  Along the way to developing
 this patch, a number of other mis-features and errors were
 discovered and fixed -- mostly as a result of issues pointed
 out by reviewers on this list. These are the reasons for the
 somewhat large scope of this patch.  For all the background,
 see the following threads:

 (*) Note that you only need to determine the dll name for an import
 lib using dlltool --identify or the fallback, IF and ONLY IF you are
 linking to a library WITHOUT a corresponding .la file.  If you DO have
 a .la file for the library you're linking to, then the save the .la
 file using a magic shell variable approach is used.

OK.  Here's my take on this: if you fix all nits I noted inline below,
post the updated and tested patch (you decide what testing is needed),
then you are OK to commit after 72 hours of waiting.  FWIW, I'm likely
not available most of next week; if we find issues later, then I guess
they'll just have to be fixed afterwards.

I'll note though that this patch makes me cringe: it introduces more
system-specific code in ltmain that is just bloat on other systems,
rather than in libtool.m4 where it belongs.  It has the potential to
regress non-cygwin non-mingw w32 systems, and since we effectively have
no cegcc maintainer that is a problem for cegcc (dunno if for us).
The patch doesn't introduce a new test, so I assume it either fixes a
current testsuite failure in either the default setup or in the
  ./configure --disable-static

setup of Libtool.  If not, or if only the latter, then a testsuite
addition in a followup patch would be nice.

Does this patch have any relation to Pierre Ossman's Preloading without
.la patch?  http://thread.gmane.org/gmane.comp.gnu.libtool.general/7071
His copyright papers are through now, so we can look at that patch.

 --- a/libltdl/config/general.m4sh
 +++ b/libltdl/config/general.m4sh
 @@ -559,4 +559,21 @@ 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

No double-quotes needed here.

 +  [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
 +}
  ]])

 --- a/libltdl/config/ltmain.m4sh
 +++ b/libltdl/config/ltmain.m4sh
 @@ -2012,10 +2012,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 import 
 library.
 + eval '$sharedlib_from_linklib $dlprefile'

redundant eval: remove eval and single quotes.

 + dlprefile_dlbasename=$sharedlib_from_linklib_result
 +   fi
 + fi
 + $opt_dry_run || {
 +   if test -n $dlprefile_dlbasename ; then
 + eval '$ECHO : $dlprefile_dlbasename  $nlist'

Likewise.

 +   else
 + func_warning Could not compute DLL name from $name
 + eval '$ECHO : $name   $nlist'

Likewise.

 +   fi
 +   eval $NM $dlprefile 2/dev/null | $global_symbol_pipe |
 

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 6

2009-07-12 Thread Charles Wilson
Charles Wilson wrote:
 * 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_fallback_core): New function.
 (func_cygming_gnu_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.
 ---
 Reposted without change from -take5, here:
 http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg00232.html
 
 This patch has been in use in the cygwin distribution since 2009-02-20.

Ping?

FYI, the cygwin distribution recently (Jul 4) released an updated
binutils package (2.19.51) for cygwin-1.7.  This version supports the
  --identify
and
  --identify-strict
options -- so, at least on cygwin-1.7 we won't be using the
func_cygming_dll_for_implib_fallback function.

Now, with the redesign proposed by Ralf back in January and implemented
(take 3? take 4? I forget), we ordinarily use NEITHER
  func_cygming_dll_for_implib_fallback
nor
  dlltool --identify-strict
because, in the ordinary case, we have a libtool .la file and use it to
track implib-dll.  NOW, the only time either of these two methods is
used, is when someone specifies explicitly an implib or DLL on the link
command passed to libtool:

  libtool mode=link  /my/path/foo.dll.a -lbar-3.dll

BUT...it's still nice that the (faster, less kludgy) dlltool method can
now be used on cygwin-1.7. (No, the standard toolchain from the
mingw.org guys doesn't yet support --identify-strict, so it still uses
the fallback).

--
Chuck




Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 4

2009-02-13 Thread Charles Wilson
Charles Wilson wrote:

 The attached, re-re-re-re-revised patch addresses these two issues, but
 is otherwise the same as take 4. 

Ping.
Most recent version is the take 5 attachment, in this message from two
weeks ago:
http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg00232.html

ChangeLog repeated (with slight revisions) for convenience:

* 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_fallback_core): New function.
(func_cygming_gnu_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.

--
Chuck




Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 4

2009-01-21 Thread Charles Wilson
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] [cygwin|mingw] fix dlpreopen with --disable-static take 4

2009-01-21 Thread Charles Wilson
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 dlpreopen with --disable-static take 2

2009-01-20 Thread Charles Wilson
Ralf Wildenhues wrote:
 +# func_tr_sh
 +# turn $1 into a string suitable for a shell variable name
 +# result is stored in $func_tr_sh_result
 +func_tr_sh ()
 +{
 +  func_tr_sh_result=`echo $1 | $SED -e 's/[^A-Za-z0-9_]/_/g'`
 +  # ensure result begins with non-digit
 +  case $func_tr_sh_result in
 +[A-Za-z_][A-Za-z0-9_] ) ;;
 +* ) func_tr_sh_result=_$func_tr_sh_result ;;
 +  esac
 +}
  ]])
 
 Let's not waste processes when we don't have to, with something like
 this untested bit:
 
 func_tr_sh ()
 {
   case $1 in
 [!a-zA-Z_]* | *[!a-zA-Z_0-9]*)
   func_tr_sh_result=`$ECHO $1 | $SED 's/^[^a-zA-Z]/_/; 
 s/[^a-zA-Z0-9]/_/g'`
   ;;
 *)
   func_tr_sh_result=$1
   ;;
   esac
 }

Your version will confuse '1dumblibraryname.a' and '2dumblibraryname.a'
by turning both into '_dumblibraryname_a'.  How about this:

# 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-Z_0-9]*)
func_tr_sh_result=`$ECHO $1 | $SED 's/^\([0-9]\)/_\1/;
s/[^A-Za-z0-9]/_/g'`
;;
  * )
func_tr_sh_result=$1
;;
  esac
}
Which makes it clear exactly what we're trying to do:
  1) replace all bad chars with _
  2) prepend _ if $1 begins with a digit
There is still a slight inefficiency in the above: IF $1 has all valid
characters, but happens to begin with a digit, then we fork a sed simply
to prepend the '_'. I doubt this will occur much. or ever -- and if it
does, the penalty is an extra fork, not wrong behavior.

 --- a/libltdl/config/ltmain.m4sh
 +++ b/libltdl/config/ltmain.m4sh
 
 @@ -1988,7 +1988,7 @@ extern \C\ {
eval '$GREP -f $output_objdir/$outputname.exp  $nlist  
 $nlistT'
eval '$MV $nlistT $nlist'
case $host in
 -*cygwin | *mingw* | *cegcc* )
 +*cygwin* | *mingw* | *cegcc* )
eval echo EXPORTS ' $output_objdir/$outputname.def'
eval 'cat $nlist  $output_objdir/$outputname.def'
;;
 
 Is this fixing a bug?  If yes, then it should be a separate patch,
 documented in the ChangeLog entry, done likely in all other such
 instances of missing '*' (I haven't found any), and would be obviously
 correct and ok to push.  Please, please don't mix heavy patches with
 such cleanups.  It only leads to cleanups being delayed.

It is not a bug fix, exactly. I just noticed the lack of symmetry, AND
that '*cygwin' never appears anywhere else, and figured it was a typo.
The actual cygwin $host patterns we see AFAIK all match *cygwin -- but
by convention we (libtool) allow extensions on triples for variant
$hosts. This violated that convention, but wasn't /exactly/ a bug.

I can prepare a separate patch and push, if you prefer.

 In your take 3 of this patch series, you have this hunk:
 
 | @@ -2217,7 +2217,7 @@ func_win32_libid ()
 |  ;;
 |*ar\ archive*) # could be an import, or static
 |  if eval $OBJDUMP -f $1 | $SED -e '10q' 2/dev/null |
 | -   $EGREP 'file format (pe-i386(.*architecture: 
 i386)?|pe-arm-wince|pe-x86-64)' /dev/null; then
 | +   $EGREP 'file format (pei?-i386(.*architecture: 
 i386)?|pe-arm-wince|pe-x86-64)' /dev/null; then
 |win32_nmres=`eval $NM -f posix -A $1 |
 | $SED -n -e '
 | 1,100{
 
 Now, my memory is really bad about win32 semantics, but wasn't it
 exactly pei-i386 libraries that we wanted to not match here?

Originally (before the introduction of [func_]win32_libid()), we had
pei*-i386:

 cygwin* | mingw* | pw32*)
  lt_cv_deplibs_check_method='file_magic file format
pei*-i386(.*architecture: i386)?'

When win32_libid() was first introduced, we moved the pei*-i386
incantation as-is into win32_libid():
grep -E 'file format pei*-i386(.*architecture: i386)?' /dev/null ; then
in 6da15e03aa1127eb42652a1f7e15ee42633dbfdf Thu Oct 31 00:52:39 2002

This was changed to
grep -E 'file format pe-i386(.*architecture: i386)?' /dev/null ; then
in 709bbb17317c67d28cf7ec8f0baaef16c4137ad0 Mon Feb 17 18:55:45 2003
Supposedly, this was part of a rewrite to improve speed. Looking at
the mailing list history from that era:
http://lists.gnu.org/archive/html/libtool-patches/2003-02/msg00048.html
No explanation was given for that particular change (my fault).

Looking at the original version of win32_libid() -- after 6da15e03 but
before 709bbb17 -- it originally did this:

  if eval $OBJDUMP -f $1 2/dev/null | \
 grep -E 'file format pei+-i386(.*architecture: i386)?' /dev/null ;
then
win32_libid_type=x86 DLL
  else
if eval $OBJDUMP -f $1 2/dev/null | \
  grep -E 'file format pei*-i386(.*architecture: i386)?' /dev/null
 then
  win32_libid_type=x86
  if eval file $1 2/dev/null | \
 grep -E 'ar archive' /dev/null; then
win32_libid_type=$win32_libid_type archive
if eval 

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 3

2009-01-19 Thread Ralf Wildenhues
Hi Charles,

* Charles Wilson wrote on Fri, Jan 16, 2009 at 02:51:21PM CET:
 The unexpected failure was
  36: execute mode FAILED (execute-mode.at:193)
 but it is unrelated; it's a problem in cygwin-1.7's dos-style path
 detection...That's not a path!
 
 --- /dev/null   2006-11-30 19:00:00.0 -0500
 +++
 /usr/src/packages/libtool/git/build-cygwin-dlpreopen-fix-take2/tests/testsui
 te.dir/at-groups/36/stderr  2009-01-15 23:50:30.98180 -0500
 @@ -0,0 +1,6 @@
 +cygwin warning:
 +  MS-DOS style path detected: d\e
 +  Preferred POSIX equivalent is: d/e
 +  CYGWIN environment variable option nodosfilewarning turns off this
 warning.
 +  Consult the user's guide for more details about POSIX paths:
 +http://cygwin.com/cygwin-ug-net/using.html#using-pathnames
 stdout:
 abc
 d\e
 f\g
 xyz
 36. execute-mode.at:25: 36. execute mode (execute-mode.at:25): FAILED 
 (execute-mode.at:193)

Thanks.  Applying this to avoid this failure.

Cheers,
Ralf

Avoid failure due to Cygwin path detection bug.
* tests/execute-mode.at (execute mode): Ignore noise on stderr;
Cygwin might consider `d\e' to be a DOS-style path and warn.
Report by Charles Wilson.

diff --git a/tests/execute-mode.at b/tests/execute-mode.at
index c3370da..a73cada 100644
--- a/tests/execute-mode.at
+++ b/tests/execute-mode.at
@@ -1,6 +1,6 @@
 # execute-mode.at -- libtool --mode=execute -*- Autotest -*-
 #
-#   Copyright (C) 2008 Free Software Foundation, Inc.
+#   Copyright (C) 2008, 2009 Free Software Foundation, Inc.
 #   Written by Ralf Wildenhues, 2008
 #
 #   This file is part of GNU Libtool.
@@ -190,7 +190,7 @@ do
   if test -z $arg1; then
 arg1=$arg2; continue
   fi
-  AT_CHECK([$LIBTOOL --mode=execute ./foo abc $arg1 $arg2 xyz], [], 
[stdout])
+  AT_CHECK([$LIBTOOL --mode=execute ./foo abc $arg1 $arg2 xyz], [], 
[stdout], [ignore])
   AT_CHECK([$FGREP $arg1 stdout], [], [ignore])
   AT_CHECK([$FGREP $arg2 stdout], [], [ignore])
   AT_CHECK([test `sed -n '/^abc$/,/^xyz$/p' stdout | wc -l` -eq 4])




Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 2

2009-01-19 Thread Ralf Wildenhues
Hello Charles,

I haven't looked at your patches in detail yet, but a couple of things
caught my eye:

* Charles Wilson wrote on Sat, Jan 03, 2009 at 02:39:15AM CET:
 diff --git a/libltdl/config/general.m4sh b/libltdl/config/general.m4sh
 index 4bc304c..c4de91a 100644
 --- a/libltdl/config/general.m4sh
 +++ b/libltdl/config/general.m4sh
[...]
 +# func_tr_sh
 +# turn $1 into a string suitable for a shell variable name
 +# result is stored in $func_tr_sh_result
 +func_tr_sh ()
 +{
 +  func_tr_sh_result=`echo $1 | $SED -e 's/[^A-Za-z0-9_]/_/g'`
 +  # ensure result begins with non-digit
 +  case $func_tr_sh_result in
 +[A-Za-z_][A-Za-z0-9_] ) ;;
 +* ) func_tr_sh_result=_$func_tr_sh_result ;;
 +  esac
 +}
  ]])

Let's not waste processes when we don't have to, with something like
this untested bit:

func_tr_sh ()
{
  case $1 in
[!a-zA-Z_]* | *[!a-zA-Z_0-9]*)
  func_tr_sh_result=`$ECHO $1 | $SED 's/^[^a-zA-Z]/_/; 
s/[^a-zA-Z0-9]/_/g'`
  ;;
*)
  func_tr_sh_result=$1
  ;;
  esac
}

 --- a/libltdl/config/ltmain.m4sh
 +++ b/libltdl/config/ltmain.m4sh

 @@ -1988,7 +1988,7 @@ extern \C\ {
 eval '$GREP -f $output_objdir/$outputname.exp  $nlist  
 $nlistT'
 eval '$MV $nlistT $nlist'
 case $host in
 - *cygwin | *mingw* | *cegcc* )
 + *cygwin* | *mingw* | *cegcc* )
 eval echo EXPORTS ' $output_objdir/$outputname.def'
 eval 'cat $nlist  $output_objdir/$outputname.def'
 ;;

Is this fixing a bug?  If yes, then it should be a separate patch,
documented in the ChangeLog entry, done likely in all other such
instances of missing '*' (I haven't found any), and would be obviously
correct and ok to push.  Please, please don't mix heavy patches with
such cleanups.  It only leads to cleanups being delayed.


In your take 3 of this patch series, you have this hunk:

| @@ -2217,7 +2217,7 @@ func_win32_libid ()
|  ;;
|*ar\ archive*) # could be an import, or static
|  if eval $OBJDUMP -f $1 | $SED -e '10q' 2/dev/null |
| -   $EGREP 'file format (pe-i386(.*architecture: 
i386)?|pe-arm-wince|pe-x86-64)' /dev/null; then
| +   $EGREP 'file format (pei?-i386(.*architecture: 
i386)?|pe-arm-wince|pe-x86-64)' /dev/null; then
|win32_nmres=`eval $NM -f posix -A $1 |
| $SED -n -e '
| 1,100{

Now, my memory is really bad about win32 semantics, but wasn't it
exactly pei-i386 libraries that we wanted to not match here?

More generally, I have a feeling that this function is badly
conditioned: it needs adjustment fairly often, it is unclear to me which
cases exactly it tries to exclude (for starters: why is the file format
test needed at all?), and when things fail here, they do so very
unobviously for the libtool user.  These issues could IMVHO be
ameliorated by having some test cases that show the fine line between
import libraries that are acceptable, and static ones that are not.
So that when we port this to the next w32 system, we just have to run
the test suite and fix it until it passes.  What do you think?

Thanks,
Ralf




Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 3

2009-01-16 Thread Charles Wilson
Charles Wilson wrote:

 Full test suite on cygwin in progress. Assuming it passes,
 ok for squash and push?

Results: old test suite:
===
All 113 tests passed
(11 tests were not run)
===

New test suite:
ERROR: 76 tests were run,
4 failed (3 expected failures).
5 tests were skipped.

The unexpected failure was
 36: execute mode FAILED (execute-mode.at:193)
but it is unrelated; it's a problem in cygwin-1.7's dos-style path
detection...That's not a path!

--- /dev/null   2006-11-30 19:00:00.0 -0500
+++
/usr/src/packages/libtool/git/build-cygwin-dlpreopen-fix-take2/tests/testsui
te.dir/at-groups/36/stderr  2009-01-15 23:50:30.98180 -0500
@@ -0,0 +1,6 @@
+cygwin warning:
+  MS-DOS style path detected: d\e
+  Preferred POSIX equivalent is: d/e
+  CYGWIN environment variable option nodosfilewarning turns off this
warning.
+  Consult the user's guide for more details about POSIX paths:
+http://cygwin.com/cygwin-ug-net/using.html#using-pathnames
stdout:
abc
d\e
f\g
xyz
36. execute-mode.at:25: 36. execute mode (execute-mode.at:25): FAILED
(execute-m
ode.at:193)

OK?

--
Chuck




Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 2

2009-01-15 Thread Peter Rosin

Den 2009-01-13 16:41 skrev Charles Wilson:

Peter Rosin wrote:

Den 2009-01-06 02:06 skrev Charles Wilson:

Maybe under that name. But a libbfd-ified version of impgen (as a
replacement for the IMO totally broken -- but part of mingw-utils-0.3 --
reimp program), that happens to also supply an --identify foo
--identify-ms functionality? Not so far-fetched.

Right, but it still seems as if this new fixed impgen tool is closer
to MinGW than to MSYS proper. However, as you say, better ask on a better
list...


After playing with this idea for a while, it made more sense actually to
separate the impgen2 functionality from the dllname stuff. It's not yet
ready for prime time (and I'm trying to keep it in sync with on-going
changes to dlltool in binutils HEAD), but I'll send my latest version of
 these new tools to you offlist.  They compile with both cygwin-gcc
(using libiberty and libbfd from 20080624), and with mingw-gcc-3.4.5
(not sure what binutils version I have; one of the more recent releases
from mingw/sourceforge I'm sure).


Works for import libs built by MSVC 2005, didn't test any other version.


I'm only saying that from the binutils p.o.v. it makes at least some
sence to report all imported dlls. At least optionally, but again, this
was just a minor point...


A new patch to binutils' dlltool was accepted that makes the following
changes to --identify:
  1) automatically determines -- and operates with -- MS-style or
binutils-style implibs


I thought that I did build three dependent libraries A-B-C using MSVC
and MinGW alternatingly  (MSVC for A and C, MinGW for B, or the other way
around) just for testing interoperability, but that was a long
time ago... And I'm not sure I really did it or if it worked. I guess
I should do it again (if I did it, otherwise I should just do it).


  2) by default, lists all imported DLLs
  3) new --identify-strict flag causes multiple imported DLLs to be
reported as an error
http://sourceware.org/ml/binutils/2009-01/msg00152.html


Cool.

I'm hoping for this to get into MSYS 1.0.11 (or 12...) so that we can
rely on its presence.

Cheers,
Peter





Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 2

2009-01-13 Thread Charles Wilson
Peter Rosin wrote:
 Den 2009-01-06 02:06 skrev Charles Wilson:
 Maybe under that name. But a libbfd-ified version of impgen (as a
 replacement for the IMO totally broken -- but part of mingw-utils-0.3 --
 reimp program), that happens to also supply an --identify foo
 --identify-ms functionality? Not so far-fetched.
 
 Right, but it still seems as if this new fixed impgen tool is closer
 to MinGW than to MSYS proper. However, as you say, better ask on a better
 list...

After playing with this idea for a while, it made more sense actually to
separate the impgen2 functionality from the dllname stuff. It's not yet
ready for prime time (and I'm trying to keep it in sync with on-going
changes to dlltool in binutils HEAD), but I'll send my latest version of
 these new tools to you offlist.  They compile with both cygwin-gcc
(using libiberty and libbfd from 20080624), and with mingw-gcc-3.4.5
(not sure what binutils version I have; one of the more recent releases
from mingw/sourceforge I'm sure).

 Also, it will not fail for Vfw32.Lib, it will instead list the three
 dlls it imports (AVICAP32.dll, AVIFIL32.dll, MSVFW32.dll).
 Well, we probably want it to fail. 
 
 So, for the sake of argument, I agree that it should fail. But then I
 think it should fail in libtool, not in the tool that digs out the
 dll name(s) from the import library. But that's a minor point...

 Well, see this and the following thread:
 http://sourceware.org/ml/binutils/2008-11/msg00078.html
 
 I'm only saying that from the binutils p.o.v. it makes at least some
 sence to report all imported dlls. At least optionally, but again, this
 was just a minor point...

A new patch to binutils' dlltool was accepted that makes the following
changes to --identify:
  1) automatically determines -- and operates with -- MS-style or
binutils-style implibs
  2) by default, lists all imported DLLs
  3) new --identify-strict flag causes multiple imported DLLs to be
reported as an error
http://sourceware.org/ml/binutils/2009-01/msg00152.html

--
Chuck





Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 2

2009-01-05 Thread Charles Wilson
Peter Rosin wrote:
 Den 2009-01-05 06:24 skrev Charles Wilson:
 Interesting! Meanwhile, I have done some experiments on my own, as I
 don't like the dependence on anything that comes with MinGW when
 dealing with libtool and MSVC.

I kind of suspected that. What about the attached?  This version needs
to link against libbfd and its dependencies -- so has to be compiled
using mingw gcc.  But if this executable was included as part of msys?

 (Speaking of dependencies, I don't think the current MinGW code
  in libtool requires 'file' to be present, and I don't think it is
  part of a minimal MSYS install. It's not in my install anyway.)

Only because its been over two years since msys 1.11 will be ready
RSN. It is intended that the file package be included in the minimal
1.11 install.

 I have found that for MSVC import libraries the simplest thing is
 to list the archive members to get to the dll name. I have tried
 with:
   lib -nologo -list $implib | grep -v '\.obj$' | sort -u
 or, in gnu speak:
   ar t $implib | grep -v '\.obj$' | sort -u

I noticed that, but wasn't sure if self-compiled (using MSVC) import
libraries were the same.

 This works for all troublesome implibs that you have listed above
 (at least those that I have easy access to, but I have at least one
 from each class of problems) and a few others. Except for MAPI.lib,
 but my MS provided dumpbin.exe (VS 2005) says
   MAPI.lib : warning LNK4048: Invalid format file; ignored
 for that one so I too think it is a pathological case.

Ack.

 Also, it will not fail for Vfw32.Lib, it will instead list the three
 dlls it imports (AVICAP32.dll, AVIFIL32.dll, MSVFW32.dll).

Well, we probably want it to fail. dlpreopen is supposed to work like
dlopen -- and you'd need to dlopen each of the three DLLs separately.
But if you -dlpreopen Vfw32.Lib, you'd need to determine which symbols
IN Vfw32.Lib came from which DLL, and generate three different groups in
you LT_LTX_*[] structure, to register those symbols with their effective
dlopen source:
   { AVICAP32.dll, 0 }
   {  symbols  }
   { AVIFIL32.dll, 0 }
   {  symbols  }
   { MSVFW32.dll, 0 }
   {  symbols  }
I don't think this is going to work well.

 Using MS tools (instead of file or objdump -f) to identify if a .lib
 is an import lib or a static lib seems to be trickier. One thing
 that appears to work is to look for an __IMPORT_DESCRIPTOR_* symbol,
 but that can obviously be thwarted by a devious (or ignorant) user...

That's ok. Rule #486: don't deliberately try to undermine your tools,
and then expect them to work.

 BTW, those symbols also identifies the imported dll (but that breaks
 when that which is imported isn't named foo.dll). E.g.
   dumpbin -symbols $lib | grep '| __IMPORT_DESCRIPTOR_'
 
 (output for Vfw32.Lib
   001  SECT2  notype   External |
 __IMPORT_DESCRIPTOR_MSVFW32
   001  SECT2  notype   External |
 __IMPORT_DESCRIPTOR_AVIFIL32
   001  SECT2  notype   External |
 __IMPORT_DESCRIPTOR_AVICAP32
 )
 
 or, in gnu speak:
   nm $lib | grep 'I __IMPORT_DESCRIPTOR_'
 (output for Vfw32.Lib
    I __IMPORT_DESCRIPTOR_MSVFW32
    I __IMPORT_DESCRIPTOR_AVIFIL32
    I __IMPORT_DESCRIPTOR_AVICAP32
 )

Yeah, I wanted to avoid assuming that non-libtool shared libraries
always in in *.dll, because many packages (especially ones that do
dlopen/dlpreopen) still name their modules foo.so even on
cygwin/mingw. Take ImageMagick, for instance.

 But...I also dislike for fixes to existing bugs, in existing platforms,
 to be held up by not-yet-in-master support for other compilers. So, can
 we get back to discussing the original patch, under the predicates of
 cygwin and mingw (not msvc) $hosts?
 
 Hey, I'm not opposed to the patch, I didn't intend to make that impression
 either, sorry if I did. I'm just trying to determine what needs to be
 done in the MSVC branch to keep up. Just poking and asking questions,
 so thank you very much for your valuable input!

Oh, ok.  Thanks.

--
Chuck

/* dllname.c -- tool to extract the name of the DLL associated with
   an import library.
   Copyright 2008 Charles S. Wilson

   This program 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 3 of the License, or
   (at your option) any later version.

   This program 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 this program; if not, write to the Free Software
   Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA
   02110-1301, USA.  */

#include stdio.h
#include stdarg.h
#include bfd.h
#include ansidecl.h
#include unistd.h   

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 2

2009-01-05 Thread Peter Rosin

Den 2009-01-05 06:24 skrev Charles Wilson:

Charles Wilson wrote:

Charles Wilson wrote:

Peter Rosin wrote:

I'm primarily trying to determine what impact this has on my
MSVC branch...

Ran some experiments on the libraries shipped with the Windows SDK. The
attached script worked ok on most of them. After eliminating the static
libraries and the import libraries to '*.dll' and '*.DLL' (241
successes in all), I'm left with the following 13 odd ducks:

These are correct, but are a reminder that import libraries exist for
objects other than those named foo.dll. So that's 5 more successes:

KSProxy.Lib :x86 archive import FOR ksproxy.ax
bthprops.lib :x86 archive import FOR bthprops.cpl
irprops.lib :x86 archive import FOR irprops.cpl
NetSh.Lib :x86 archive import FOR NETSH.EXE
WinSpool.Lib :x86 archive import FOR WINSPOOL.DRV

These are the true failures:

For the following 6 libraries, it is the LAST archive member with a
.idata$6 section, not the first one, that specifies the DLL.

GdiPlus.lib :x86 archive import FOR u.GdiplusStartup
MSTask.Lib :x86 archive import FOR .._setnetscheduleaccountinformat...@12
WS2_32.Lib :x86 archive import FOR ..inet_pton
ksuser.lib :x86 archive import FOR ..KsCreateTopologyNode
shell32.lib :x86 archive import FOR =.WOWShellExecute
windowscodecs.lib :x86 archive import FOR q.WICSetEncoderFormat_Proxy

This one imports symbols from multiple DLLs. One of them happens to be
in the last archive member, but...

Vfw32.Lib :x86 archive import FOR -.StretchDIB

I have no idea what this one is. objdump can't grok it:

MAPI.Lib :MAPI.Lib: Microsoft Visual C library
$ objdump -f MAPI.Lib
objdump: MAPI.Lib: File format not recognized

So that's 246 PASS, 8 FAIL.


So, I've prepared a patch for dlltool which adds an '--identify-ms'
flag, which modifies the behavior of the '--identify implib' option.
It searches for .idata$6 instead of .idata$7, AND attempts to
disambiguate between the one that specifies the DLL name and all the
other ones that list the symbols by inspecting the flags.

In almost all cases, the one that specifies the DLL name has the flag
value 0x0123
   SEC_ALLOC | SEC_LOAD | SEC_DATA | SEC_HAS_CONTENTS
The other ones have flag values 0x00204103
   SEC_ALLOC | SEC_LOAD | SEC_HAS_CONTENTS | SEC_IN_MEMORY | SEC_KEEP

This version of dlltool was able to operate on all of the import
libraries in the Windows SDK, except for:

MAPI.Lib: MAPI.Lib: Microsoft Visual C library
  === again, because bfd has no idea how to parse this one

Vfw32.Lib: Import library `Vfw32.Lib' specifies two or more dlls:
`MSVFW32.dll' and `AVIFIL32.dll'

And the following:

WebPost.Lib: x86 archive import FOR WEBPOST.DLL
ddao35.lib: x86 archive import FOR ddao35.dll
ddao35d.lib: x86 archive import FOR ddao35d.dll
ddao35u.lib: x86 archive import FOR ddao35u.dll
ddao35ud.lib: x86 archive import FOR ddao35ud.dll
More on these, later.

Note that this dlltool /succeeded/ on
GdiPlus.lib
MSTask.Lib
WS2_32.Lib
ksuser.lib  
shell32.lib
windowscodecs.lib
where the script in my previous post failed. Dlltool can handle the case
where the one that specifies the DLL name is not first.

The five new failures (where the script succeeded) are interesting. In
each case, the rule above (the one has flag value 0x0123, and
the others do not) was incorrect:


$ ~/dlltool.exe --identify WebPost.Lib --identify-ms
flags: 0x0123   datasize: 000c  data: 'WEBPOST.DLL'
  5745 4250 4f53 542e 444c 4c00   WEBPOST.DLL.

flags: 0x0123   datasize: 0010  data: ''
  0400 5770 4269 6e64 546f 5369 7465 4100 ..WpBindToSiteA.

/home/cwilson/dlltool: Import library `WebPost.Lib' specifies two or
more dlls: `WEBPOST.DLL' and `'


The error message is a little confusing: `WEBPOST.DLL' and `'? The
empty name is because the data contains the unprintable character \004
followed by '\0'.  Recall for symbols, the first two bytes are a
little-endian count. So this is symbol 0x0004. I *guess* I could check
that both offset 0 and offset 1 contain printable characters. But that's
still just a heuristic, because a really big DLL might have
  01my_symbol
where the first two bytes are 0x30 0x31 ('01') representing symbol
number 0x3130 or 12352.

But this wierd case occurs only when the import library appears to NOT
follow the pattern most of them do. In fact, in these five import
libraries, ALL of the .idata$6 sections have flag 0x0123, not just
the one we want.

But, what are they? Do we care?

The ddao35 libraries are the Microsoft JET 3.5 DAO C++ libraries, for
DOS-Win16 (!).  Microsoft shipped the Jet 4.0 libraries with WinME and
W2k, and recommends against using ones older than that. Do we care that
you won't be able to dlpreopen (or dlltool --identify) these ancient
import libraries?)

Webpost.Lib (-- webpost.dll) seems to be part of the Web Publishing
Wizard API. I 

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 2

2009-01-05 Thread Peter Rosin

Den 2009-01-05 15:08 skrev Charles Wilson:

Peter Rosin wrote:

Den 2009-01-05 06:24 skrev Charles Wilson:
Interesting! Meanwhile, I have done some experiments on my own, as I
don't like the dependence on anything that comes with MinGW when
dealing with libtool and MSVC.


I kind of suspected that. What about the attached?  This version needs
to link against libbfd and its dependencies -- so has to be compiled
using mingw gcc.  But if this executable was included as part of msys?


Works for me (also works for import libs produced with the msvc branch).
How likely is dllname to make it into msys 1.11? Or will that have to
wait until 1.12? (I'm asking what you think, I know that definitive
answers to such questions are in short supply...)

However, from where I'm sitting adding a tool to MSYS proper that really
only benefits MSVC users (MinGW users could just as well have it
distributed with MinGW) seems a bit far fetched. Or? There are ways to
dig out the info using the dumpbin and lib programs, so it's not a
showstopper if this is not added to MSYS, even if the code in libtool
will be a wee bit longer. When there is an alternative, it seems even
more far fetched to have dllname added to MSYS. But *I* wouldn't say no
to it of course...

BTW, my libiberty is probably old, I had to take out the expandargv
call and add this CONST_STRNEQ definition:
#define CONST_STRNEQ(STR1,STR2) (strncmp (STR1, STR2 , sizeof (STR2) - 1) == 
0)


(Speaking of dependencies, I don't think the current MinGW code
 in libtool requires 'file' to be present, and I don't think it is
 part of a minimal MSYS install. It's not in my install anyway.)


Only because its been over two years since msys 1.11 will be ready
RSN. It is intended that the file package be included in the minimal
1.11 install.


Oh, ok. Good enough for me.


I have found that for MSVC import libraries the simplest thing is
to list the archive members to get to the dll name. I have tried
with:
  lib -nologo -list $implib | grep -v '\.obj$' | sort -u
or, in gnu speak:
  ar t $implib | grep -v '\.obj$' | sort -u


I noticed that, but wasn't sure if self-compiled (using MSVC) import
libraries were the same.


They are (at least mine...)


Also, it will not fail for Vfw32.Lib, it will instead list the three
dlls it imports (AVICAP32.dll, AVIFIL32.dll, MSVFW32.dll).


Well, we probably want it to fail. dlpreopen is supposed to work like
dlopen -- and you'd need to dlopen each of the three DLLs separately.
But if you -dlpreopen Vfw32.Lib, you'd need to determine which symbols
IN Vfw32.Lib came from which DLL, and generate three different groups in
you LT_LTX_*[] structure, to register those symbols with their effective
dlopen source:
   { AVICAP32.dll, 0 }
   {  symbols  }
   { AVIFIL32.dll, 0 }
   {  symbols  }
   { MSVFW32.dll, 0 }
   {  symbols  }
I don't think this is going to work well.


I don't really see why this is not going to work well, but I'm not a
heavy libltdl user (yet...) so I'm pretty ignorant on the subject.

So, for the sake of argument, I agree that it should fail. But then I
think it should fail in libtool, not in the tool that digs out the
dll name(s) from the import library. But that's a minor point...

Cheers,
Peter




Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 2

2009-01-05 Thread Peter Rosin

Den 2009-01-04 03:35 skrev Charles Wilson:

Peter Rosin wrote:

I'm primarily trying to determine what impact this has on my
MSVC branch...

Den 2009-01-03 02:39 skrev Charles Wilson:
*snip*

+*cygwin* | *mingw* | *cegcc* )

We should strive to have fewer of these in ltmain.m4sh, not more.


Yep. But the problem is, there are really two BIG categories of
platforms: those that support ELF-semantics for shared libraries, and
those that support PE-DLL semantics. The differences between, say, HP
and Linux are in this regard much less significant than the differences
between win32 (cygwin, mingw, msvc, even wince) and any *nixoid. And
vice verse: cygwin and msvc are much more similar *with regards to the
construction of shared libraries* than they are different (even though
the implib/static lib formats are non-interchangeable).

I'm not sure it would be an improvement, exactly, but we could have a
libtool variable $LT_HOST_SUPPORTS_PE_DLL (or a function that takes
$host), and replace many of these
   case $host in
 *cygwin* | *mingw* | *cegcc* ) ... ;;
 everything else ) ... ;;
   esac
occurences with 'if host_supports_pe_dll ; then ... ; else ... ; fi'
Still ugly, but it means you only have to fix the case $host pattern
in one place.


I think it should be like it is for everything else, a separate control-
ling variable for stuff that seem orthogonal. Many of the case $host
constructs should probably be if test $LT_HOST_SUPPORTS_PE_DLL = yes
(with the variable in lower case to conform), but I'm sure there are
examples where the controlling variable should be named something else.

I think there is value in separating these things, it serves as
documentation of what pieces of ltmain.m4sh are connected to each
other. It also helps when something like the MSVC branch is added
as many, but not all, things are equal between MSVC and MinGW.

A central function would be a step back IMHO, as a MSVC exception
(or whatever exception) for some specific snippet of code would
probably get uglier.

It's not as if we are talking hundreds of new variables, my guess
would be ten or so. But that's without looking at the code for
quite some time...

That said, I'm still not objecting to this patch as is, one more
case $host is not going to kill us.

Cheers,
Peter





Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 2

2009-01-05 Thread Peter Rosin

Den 2009-01-06 02:06 skrev Charles Wilson:

Maybe under that name. But a libbfd-ified version of impgen (as a
replacement for the IMO totally broken -- but part of mingw-utils-0.3 --
reimp program), that happens to also supply an --identify foo
--identify-ms functionality? Not so far-fetched.


Right, but it still seems as if this new fixed impgen tool is closer
to MinGW than to MSYS proper. However, as you say, better ask on a better
list...


Also, it will not fail for Vfw32.Lib, it will instead list the three
dlls it imports (AVICAP32.dll, AVIFIL32.dll, MSVFW32.dll).

Well, we probably want it to fail. dlpreopen is supposed to work like
dlopen -- and you'd need to dlopen each of the three DLLs separately.
But if you -dlpreopen Vfw32.Lib, you'd need to determine which symbols
IN Vfw32.Lib came from which DLL, and generate three different groups in
you LT_LTX_*[] structure, to register those symbols with their effective
dlopen source:
   { AVICAP32.dll, 0 }
   {  symbols  }
   { AVIFIL32.dll, 0 }
   {  symbols  }
   { MSVFW32.dll, 0 }
   {  symbols  }
I don't think this is going to work well.

I don't really see why this is not going to work well, but I'm not a
heavy libltdl user (yet...) so I'm pretty ignorant on the subject.


No, what I meant was: IF you constructed all that then it would work.
BUT there is no support currently in libtool for generating that kind of
thing from a single implib.  It would be very ugly indeed.  So, at
present if func_msvc_dll_for_implib returns a list of DLLs, what will
the caller actually do?  How will the caller know which symbols should
go with which DLL? Does it need to be stateful?

But weigh the cost/benefit: LOADS of new code to write, test, and debug
-- so that a pathological import lib that specifies imports from more
than one DLL can be -dlpreopened.  Is it worth it? What's the
opportunity cost -- what /other/ more useful things could that
developer-time be spent on?


Agreed.


So, for the sake of argument, I agree that it should fail. But then I
think it should fail in libtool, not in the tool that digs out the
dll name(s) from the import library. But that's a minor point...


Well, see this and the following thread:
http://sourceware.org/ml/binutils/2008-11/msg00078.html


I'm only saying that from the binutils p.o.v. it makes at least some
sence to report all imported dlls. At least optionally, but again, this
was just a minor point...

Cheers,
Peter




Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 2

2009-01-04 Thread Charles Wilson
Charles Wilson wrote:
 Peter Rosin wrote:
 I'm primarily trying to determine what impact this has on my
 MSVC branch...

Ran some experiments on the libraries shipped with the Windows SDK. The
attached script worked ok on most of them. After eliminating the static
libraries and the import libraries to '*.dll' and '*.DLL' (241
successes in all), I'm left with the following 13 odd ducks:

These are correct, but are a reminder that import libraries exist for
objects other than those named foo.dll. So that's 5 more successes:

KSProxy.Lib :x86 archive import FOR ksproxy.ax
bthprops.lib :x86 archive import FOR bthprops.cpl
irprops.lib :x86 archive import FOR irprops.cpl
NetSh.Lib :x86 archive import FOR NETSH.EXE
WinSpool.Lib :x86 archive import FOR WINSPOOL.DRV

These are the true failures:

For the following 6 libraries, it is the LAST archive member with a
.idata$6 section, not the first one, that specifies the DLL.

GdiPlus.lib :x86 archive import FOR u.GdiplusStartup
MSTask.Lib :x86 archive import FOR .._setnetscheduleaccountinformat...@12
WS2_32.Lib :x86 archive import FOR ..inet_pton
ksuser.lib :x86 archive import FOR ..KsCreateTopologyNode
shell32.lib :x86 archive import FOR =.WOWShellExecute
windowscodecs.lib :x86 archive import FOR q.WICSetEncoderFormat_Proxy

This one imports symbols from multiple DLLs. One of them happens to be
in the last archive member, but...

Vfw32.Lib :x86 archive import FOR -.StretchDIB

I have no idea what this one is. objdump can't grok it:

MAPI.Lib :MAPI.Lib: Microsoft Visual C library
$ objdump -f MAPI.Lib
objdump: MAPI.Lib: File format not recognized

So that's 246 PASS, 8 FAIL.


But I wonder if these 8 failures are just pathological cases, and the
code embodied in the attached script is good enough -- assuming an
msvc-configured libtool is allowed to use file, objdump, nm, etc.

Doctor, func_dll_from_imp fails for library shell32.lib, so I can't
dlpreopen it...Ok, don't do that then.

NOTE:
I had to change the grep expression:
  *ar\ archive*) # could be an import, or static
if objdump -f $1 | sed -e '10q' 2/dev/null |
  grep -E 'file format pei?-i386(.*architecture: i386)?' /dev/null
; then

from ...pe-i386(... to ...pei?-i386(... so that some of the import
libraries were recognized as such. I'm not sure what the distinction
between pe-i386 and pei-i386 is, or what the implications of this
particular change would be.

FWIW, I think this sed script is safer than the other one, even at the
cost of an extra fork. Keeping the contents of each archive member on a
separate line, rather than merging them all together, just seems better.

--
Chuck
#!/bin/sh

DLLNAME_SECTION=.idata\$6

# mostly the same as libtool's function of the same name,
# except that it stores the result in an explicitly-accessible
# *_result variable. Still echos, tho.  Also, the grep -E 
# expression is slightly different: pe-i386 - pei?-i386
func_win32_libid ()
{
  func_win32_libid_result=unknown
  win32_fileres=`file -L $1 2/dev/null`
  case $win32_fileres in
  *ar\ archive\ import\ library*) # definitely import
func_win32_libid_result=x86 archive import
;;
  *ar\ archive*) # could be an import, or static
if objdump -f $1 | sed -e '10q' 2/dev/null |
  grep -E 'file format pei?-i386(.*architecture: i386)?' /dev/null ; then
  win32_nmres=`eval nm -f posix -A $1 2/dev/null |
sed -n -e '
1,100{
/ I /{
s,.*,import,
p
q
}
}'`
  case $win32_nmres in
import*) func_win32_libid_result=x86 archive import ;;
*)   func_win32_libid_result=x86 archive static;;
  esac
fi
;;
  *DLL*)
func_win32_libid_result=x86 DLL
;;
  *executable*) # but shell scripts are executable too...
case $win32_fileres in
*MS\ Windows\ PE\ Intel*)
  func_win32_libid_result=x86 DLL
  ;;
esac
;;
  esac
  echo $func_win32_libid_result
}

# same as libtool's, from master
func_win32_import_lib_p ()
{
case `func_win32_libid $1 2/dev/null | sed -e 10q` in
*import*) : ;;
*) false ;;
esac
}

# Odd. For some reason I can't capture this directly in
# backticks. So, use a level of indirection:
func_dll_for_imp_core ()
{
  objdump -s --section $DLLNAME_SECTION $1 2/dev/null |
sed '/^Contents of section '$DLLNAME_SECTION':/{
  # Place marker at beginning of archive member dllname section
  s/.*/MARK/
  p
  d
}
# These lines can sometimes be longer than 43 characters, but
# are always uninteresting
/:[ \t]*file format pe[i]\{,1\}-i386$/d
/^In archive [^:]*:/d
# Ensure marker is printed
/^MARK/p
# Remove all lines with less than 43 characters
/^.\{43\}/!d
# From remoaining lines, remove first 43 characters
s/^.\{43\}//' |
sed -n '
  # Join marker and all lines until next marker into a single line
  

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 2

2009-01-04 Thread Charles Wilson
Charles Wilson wrote:
 Charles Wilson wrote:
 Peter Rosin wrote:
 I'm primarily trying to determine what impact this has on my
 MSVC branch...
 
 Ran some experiments on the libraries shipped with the Windows SDK. The
 attached script worked ok on most of them. After eliminating the static
 libraries and the import libraries to '*.dll' and '*.DLL' (241
 successes in all), I'm left with the following 13 odd ducks:
 
 These are correct, but are a reminder that import libraries exist for
 objects other than those named foo.dll. So that's 5 more successes:
 
 KSProxy.Lib :x86 archive import FOR ksproxy.ax
 bthprops.lib :x86 archive import FOR bthprops.cpl
 irprops.lib :x86 archive import FOR irprops.cpl
 NetSh.Lib :x86 archive import FOR NETSH.EXE
 WinSpool.Lib :x86 archive import FOR WINSPOOL.DRV
 
 These are the true failures:
 
 For the following 6 libraries, it is the LAST archive member with a
 .idata$6 section, not the first one, that specifies the DLL.
 
 GdiPlus.lib :x86 archive import FOR u.GdiplusStartup
 MSTask.Lib :x86 archive import FOR .._setnetscheduleaccountinformat...@12
 WS2_32.Lib :x86 archive import FOR ..inet_pton
 ksuser.lib :x86 archive import FOR ..KsCreateTopologyNode
 shell32.lib :x86 archive import FOR =.WOWShellExecute
 windowscodecs.lib :x86 archive import FOR q.WICSetEncoderFormat_Proxy
 
 This one imports symbols from multiple DLLs. One of them happens to be
 in the last archive member, but...
 
 Vfw32.Lib :x86 archive import FOR -.StretchDIB
 
 I have no idea what this one is. objdump can't grok it:
 
 MAPI.Lib :MAPI.Lib: Microsoft Visual C library
 $ objdump -f MAPI.Lib
 objdump: MAPI.Lib: File format not recognized
 
 So that's 246 PASS, 8 FAIL.

So, I've prepared a patch for dlltool which adds an '--identify-ms'
flag, which modifies the behavior of the '--identify implib' option.
It searches for .idata$6 instead of .idata$7, AND attempts to
disambiguate between the one that specifies the DLL name and all the
other ones that list the symbols by inspecting the flags.

In almost all cases, the one that specifies the DLL name has the flag
value 0x0123
   SEC_ALLOC | SEC_LOAD | SEC_DATA | SEC_HAS_CONTENTS
The other ones have flag values 0x00204103
   SEC_ALLOC | SEC_LOAD | SEC_HAS_CONTENTS | SEC_IN_MEMORY | SEC_KEEP

This version of dlltool was able to operate on all of the import
libraries in the Windows SDK, except for:

MAPI.Lib: MAPI.Lib: Microsoft Visual C library
  === again, because bfd has no idea how to parse this one

Vfw32.Lib: Import library `Vfw32.Lib' specifies two or more dlls:
`MSVFW32.dll' and `AVIFIL32.dll'

And the following:

WebPost.Lib: x86 archive import FOR WEBPOST.DLL
ddao35.lib: x86 archive import FOR ddao35.dll
ddao35d.lib: x86 archive import FOR ddao35d.dll
ddao35u.lib: x86 archive import FOR ddao35u.dll
ddao35ud.lib: x86 archive import FOR ddao35ud.dll
More on these, later.

Note that this dlltool /succeeded/ on
GdiPlus.lib
MSTask.Lib
WS2_32.Lib
ksuser.lib  
shell32.lib
windowscodecs.lib
where the script in my previous post failed. Dlltool can handle the case
where the one that specifies the DLL name is not first.

The five new failures (where the script succeeded) are interesting. In
each case, the rule above (the one has flag value 0x0123, and
the others do not) was incorrect:


$ ~/dlltool.exe --identify WebPost.Lib --identify-ms
flags: 0x0123   datasize: 000c  data: 'WEBPOST.DLL'
  5745 4250 4f53 542e 444c 4c00   WEBPOST.DLL.

flags: 0x0123   datasize: 0010  data: ''
  0400 5770 4269 6e64 546f 5369 7465 4100 ..WpBindToSiteA.

/home/cwilson/dlltool: Import library `WebPost.Lib' specifies two or
more dlls: `WEBPOST.DLL' and `'


The error message is a little confusing: `WEBPOST.DLL' and `'? The
empty name is because the data contains the unprintable character \004
followed by '\0'.  Recall for symbols, the first two bytes are a
little-endian count. So this is symbol 0x0004. I *guess* I could check
that both offset 0 and offset 1 contain printable characters. But that's
still just a heuristic, because a really big DLL might have
  01my_symbol
where the first two bytes are 0x30 0x31 ('01') representing symbol
number 0x3130 or 12352.

But this wierd case occurs only when the import library appears to NOT
follow the pattern most of them do. In fact, in these five import
libraries, ALL of the .idata$6 sections have flag 0x0123, not just
the one we want.

But, what are they? Do we care?

The ddao35 libraries are the Microsoft JET 3.5 DAO C++ libraries, for
DOS-Win16 (!).  Microsoft shipped the Jet 4.0 libraries with WinME and
W2k, and recommends against using ones older than that. Do we care that
you won't be able to dlpreopen (or dlltool --identify) these ancient
import libraries?)

Webpost.Lib (-- webpost.dll) seems to be part of the Web Publishing
Wizard API. I can't find 

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 2

2009-01-03 Thread Peter Rosin

Hi Chuck,

I'm primarily trying to determine what impact this has on my
MSVC branch...

Den 2009-01-03 02:39 skrev Charles Wilson:
*snip*

+   *cygwin* | *mingw* | *cegcc* )


We should strive to have fewer of these in ltmain.m4sh, not more...


+  func_warn Using fallback code to determine dllname for $1; consider updating 
binutils to version 2.20 (2.19.50.20081115), or newer.


I fail to find that version of binutils in Cygwin setup, so I guess the
fallback code will get exercised. The comments say that gcc adds the
dll name in .idata$7, and the fallback code makes use of this fact. How
stable is that? What happens if you generate an import library late (if
you only have the dll) with something that is not gcc? What if you have
an import library created by e.g. MSVC? Looking at a few import libs in
the Platform SDK suggests that MSVC uses .idata$6 instead. To me it
seems as if the dllname is in the last .idata segment, can't that be
scripted for a better fallback?

Hmm, looking at the the --identify patch for binutils, it seems that it
too simply dumps out .idata$7. That seems to be plain wrong for a whole
bunch of import libs out there in the wild.

Cheers,
Peter




Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 2

2009-01-03 Thread Charles Wilson
Peter Rosin wrote:
 I'm primarily trying to determine what impact this has on my
 MSVC branch...
 
 Den 2009-01-03 02:39 skrev Charles Wilson:
 *snip*
 +*cygwin* | *mingw* | *cegcc* )
 
 We should strive to have fewer of these in ltmain.m4sh, not more.

Yep. But the problem is, there are really two BIG categories of
platforms: those that support ELF-semantics for shared libraries, and
those that support PE-DLL semantics. The differences between, say, HP
and Linux are in this regard much less significant than the differences
between win32 (cygwin, mingw, msvc, even wince) and any *nixoid. And
vice verse: cygwin and msvc are much more similar *with regards to the
construction of shared libraries* than they are different (even though
the implib/static lib formats are non-interchangeable).

I'm not sure it would be an improvement, exactly, but we could have a
libtool variable $LT_HOST_SUPPORTS_PE_DLL (or a function that takes
$host), and replace many of these
   case $host in
 *cygwin* | *mingw* | *cegcc* ) ... ;;
 everything else ) ... ;;
   esac
occurences with 'if host_supports_pe_dll ; then ... ; else ... ; fi'
Still ugly, but it means you only have to fix the case $host pattern
in one place.

 +  func_warn Using fallback code to determine dllname for $1;
 consider updating binutils to version 2.20 (2.19.50.20081115), or newer.
 
 I fail to find that version of binutils in Cygwin setup, so I guess the
 fallback code will get exercised.

Not exactly.  Background: the --identify option is supported only in
CVS. There has not yet been an official binutils release containing that
code. Furthermore, cygwin's official binutils, while based on a CVS
snapshot, is from 20080624.  So, no, if you want it you have to compile
it yourself.  Same for the mingw-provided binutils.

But, func_win32_dllname_for_implib is only ever called if we were unable
to find the .la file for the implib.  Normally, you'd do something like:

  -dlpreopen foo.la

and then, the libfile_$(transliterated implib name) stuff is used, not
func_win32_dllname_for_implib.  You only use that function if somebody did:

  -dlpreopen /path/to/foo.dll.a

(maybe because they had no .la file, or it was not installed by the
system packaging, or whatever).

BTW, I'm assuming that func_win32_import_lib_p() works for msvc, because
you've got the correct $file_magic_command for your toolchain
(whatever correct means)...

 The comments say that gcc adds the
 dll name in .idata$7, and the fallback code makes use of this fact. How
 stable is that?

For gcc dll's, very. Hasn't changed in over a decade. pe-ppc (as opposed
to pe-i386) uses .idata$6; a dlltool compiled for that toolchain
correctly inspects .idata$6. That's why this manual parsing stuff is
just a fallback; it'd be better to use a binary tool. But, for msvc, I
haven't even thought about trying to parse dumpbin output.

Can we (libtool?) provide binary utility programs to go along with msvc
to solve some of these issues?  Furthermore, can we even assume that
binutils progs like objdump and dlltool are available? Or file, for that
matter?

 What happens if you generate an import library late (if
 you only have the dll) with something that is not gcc? 

First, I'm assuming that you're not talking about mixing between
toolchains (e.g. if $CC is gcc, then the libs were generated by gcc/ld.
if $CC is cl, then the libs were generated by cl.exe.)

So, you're talking about completely non-gcc-based toolchains AND you're
dealing with an import library for which you have no .la file. In that
case you're stuck using func_win32_dllname_for_implib.  Now, dlltool
--identify (if you even HAVE dlltool) doesn't work with MSVC lib files
(I just checked). I could see different functions defined for each
toolchain, and then a libtool variable $pedll_dllname_for_implib_cmd
that points to the correct implementation for your toolchain. (mingw and
cygwin and probably cegcc would point to my implementation, whatever
it is renamed to).

But remember, this is all a corner case: dlpreopen of a shared library,
when you don't have an .la file. Otherwise, it ought to just work(tm).

 What if you have
 an import library created by e.g. MSVC? Looking at a few import libs in
 the Platform SDK suggests that MSVC uses .idata$6 instead.

Sortof. I see a LOT of .idata$6 entries. Most of them contain symbol
names at offset 2, and offset 0,1 contain a little-endian count. But the
one that has the DLL name starts at offset 0.  How are you supposed to
tell that 'MS' is actually the first two characters of a DLL name, and
not an indication that symbol 'VCR80.dll' is entry number 0x534d in the
symbol list?

Unless the first member of the archive is ALWAYS the one that will
specify the DLL, and you look in the .idata$LAST of that member.

I think it would be better NOT to try to generalize this, and instead go
the $LT_pedll_dllname_for_implib_cmd route above. If your msvc port can
use objdump, then the following works (and 

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static take 2

2009-01-02 Thread Charles Wilson
Charles Wilson wrote:

 bootstrapped on cygwin, tested the
   demo-{conf|shared|static} + demo-make + demo-exec
 test cases with success. Full test suite in progress.

And...4.5 hours later, test suite results on cygwin (1.7.0-37, but that
shouldn't matter.  The good news is, cygwin-1.7 now handles missing DLL
errors without a GUI popup dialog. Un-attended testsuite execution on
Vista has been restored!)

old:
===
All 113 tests passed
(11 tests were not run)
===
SKIP: tests/cdemo-undef.test
SKIP: tests/tagdemo-undef.test
SKIP: tests/fcdemo-* == 9 tests


new:
76 tests behaved as expected.
5 tests were skipped.
 23: Java convenience archives skipped (convenience.at:230)
 27: shlibpath_overrides_runpath   skipped (shlibpath.at:54)
 41: GCJ inferred tag  skipped (infer-tag.at:84)
 52: ltdl API  skipped (ltdl-api.at:31)
 81: darwin fat compileskipped (darwin.at:42)

OK to push? Comments? Revisions?

--
Chuck





Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static

2008-11-24 Thread Ralf Wildenhues
Hello Brian,

* Brian Dessent wrote on Thu, Nov 13, 2008 at 11:16:24PM CET:
 Ralf Wildenhues wrote:
 
  I'm actually not sure whether _GLOBAL__F[ID]_.* can appear on w32.
  Do you know?  They should happen with C++ code using constructors
  and destructors IIRC.
 
 Yes they do occur, although not matching that regexp.  For one, they
 will have two leading underscores before the G, as with all symbols
 compared to their linux counterparts (i.e. __USER_LABEL_PREFIX__ is _
 on Cygwin/MinGW.)  For another I would have expected the regexp to match
 [FID] not F[ID] as there seems to generally be only one character in
 that position, whose purpose is illuminated by this comment in
 gcc/tree.c:
[...]
 This implies that 'FI' is not valid, or at least not recognised by the
 demangler as significant.

Well, the bug report that prompted this addition:
http://lists.gnu.org/archive/html/libtool-patches/2008-01/msg00033.html
http://lists.gnu.org/archive/html/bug-libtool/2008-01/msg9.html
needed F[ID] rather than [FID].  GCC 4.0.0 is used there on AIX.

Did GCC change since then, or is this system-dependent?

Thanks,
Ralf




Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static

2008-11-24 Thread Brian Dessent
Ralf Wildenhues wrote:

 Did GCC change since then, or is this system-dependent?

Interesting.  I'd be curious to see if powerpc-ibm-aix5.3.0.0-c++filt
recognises the FI/FD encoding, and if so then it would be reasonable to
conclude that this is in fact system-dependent or otherwise an internal
implementation detail.  Nevertheless it seems to me like the regexp
ought accept but not require the leading F since the testcase of a
simple ctor results in a symbol _GLOBAL__I_.* on Linux as well.

Brian




Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static

2008-11-24 Thread Charles Wilson
Brian Dessent wrote:
 Ralf Wildenhues wrote:
 
 Did GCC change since then, or is this system-dependent?
 
 Interesting.  I'd be curious to see if powerpc-ibm-aix5.3.0.0-c++filt
 recognises the FI/FD encoding, and if so then it would be reasonable to
 conclude that this is in fact system-dependent or otherwise an internal
 implementation detail.  Nevertheless it seems to me like the regexp
 ought accept but not require the leading F since the testcase of a
 simple ctor results in a symbol _GLOBAL__I_.* on Linux as well.

My revised patch is still in-progress, but I've changed the libtool.m4
[cygwin|mingw] part to look like this;

--
Chuck
diff --git a/libltdl/m4/libtool.m4 b/libltdl/m4/libtool.m4
index 35d7d5c..d45013d 100644
--- a/libltdl/m4/libtool.m4
+++ b/libltdl/m4/libtool.m4
@@ -4089,6 +4089,7 @@ m4_require([_LT_TAG_COMPILER])dnl
 AC_MSG_CHECKING([whether the $compiler linker ($LD) supports shared libraries])
 m4_if([$1], [CXX], [
   _LT_TAGVAR(export_symbols_cmds, $1)='$NM $libobjs $convenience | 
$global_symbol_pipe | $SED '\''s/.* //'\'' | sort | uniq  $export_symbols'
+  _LT_TAGVAR(exclude_expsyms, $1)=['_GLOBAL_OFFSET_TABLE_|_GLOBAL__F[ID]_.*']
   case $host_os in
   aix[[4-9]]*)
 # If we're using GNU nm, then we don't want the -C option.
@@ -4103,13 +4104,13 @@ m4_if([$1], [CXX], [
 _LT_TAGVAR(export_symbols_cmds, $1)=$ltdll_cmds
   ;;
   cygwin* | mingw* | cegcc*)
-_LT_TAGVAR(export_symbols_cmds, $1)='$NM $libobjs $convenience | 
$global_symbol_pipe | $SED -e '\''/^[[BCDGRS]][[ ]]/s/.*[[ ]]\([[^ ]]*\)/\1 
DATA/;/^.*[[ ]]__nm__/s/^.*[[ ]]__nm__\([[^ ]]*\)[[ ]][[^ ]]*/\1 DATA/;/^I[[ 
]]/d;/^[[AITW]][[ ]]/s/.* //'\'' | sort | uniq  $export_symbols'
+_LT_TAGVAR(export_symbols_cmds, $1)='$NM $libobjs $convenience | 
$global_symbol_pipe | $SED -e '\''/^[[BCDGRS]][[ ]]/s/.*[[ ]]\([[^ ]]*\)/\1 
DATA/;s/^.*[[ ]]__nm__\([[^ ]]*\)[[ ]][[^ ]]*/\1 DATA/;/^I[[ ]]/d;/^[[AITW]][[ 
]]/s/.* //'\'' | sort | uniq  $export_symbols'
+_LT_TAGVAR(exclude_expsyms, 
$1)=['[_]+GLOBAL_OFFSET_TABLE_|[_]+GLOBAL__[FID]_.*|[_]+head_[A-Za-z0-9_]+_dll|[A-Za-z0-9_]+_dll_iname']
   ;;
   *)
 _LT_TAGVAR(export_symbols_cmds, $1)='$NM $libobjs $convenience | 
$global_symbol_pipe | $SED '\''s/.* //'\'' | sort | uniq  $export_symbols'
   ;;
   esac
-  _LT_TAGVAR(exclude_expsyms, $1)=['_GLOBAL_OFFSET_TABLE_|_GLOBAL__F[ID]_.*']
 ], [
   runpath_var=
   _LT_TAGVAR(allow_undefined_flag, $1)=
@@ -4248,7 +4249,8 @@ _LT_EOF
   _LT_TAGVAR(allow_undefined_flag, $1)=unsupported
   _LT_TAGVAR(always_export_symbols, $1)=no
   _LT_TAGVAR(enable_shared_with_static_runtimes, $1)=yes
-  _LT_TAGVAR(export_symbols_cmds, $1)='$NM $libobjs $convenience | 
$global_symbol_pipe | $SED -e '\''/^[[BCDGRS]][[ ]]/s/.*[[ ]]\([[^ ]]*\)/\1 
DATA/'\'' | $SED -e '\''/^[[AITW]][[ ]]/s/.*[[ ]]//'\'' | sort | uniq  
$export_symbols'
+  _LT_TAGVAR(export_symbols_cmds, $1)='$NM $libobjs $convenience | 
$global_symbol_pipe | $SED -e '\''/^[[BCDGRS]][[ ]]/s/.*[[ ]]\([[^ ]]*\)/\1 
DATA/;s/^.*[[ ]]__nm__\([[^ ]]*\)[[ ]][[^ ]]*/\1 DATA/;/^I[[ ]]/d;/^[[AITW]][[ 
]]/s/.* //'\'' | sort | uniq  $export_symbols'
+  _LT_TAGVAR(exclude_expsyms, 
$1)=['[_]+GLOBAL_OFFSET_TABLE_|[_]+GLOBAL__[FID]_.*|[_]+head_[A-Za-z0-9_]+_dll|[A-Za-z0-9_]+_dll_iname']
 
   if $LD --help 21 | $GREP 'auto-import'  /dev/null; then
 _LT_TAGVAR(archive_cmds, $1)='$CC -shared $libobjs $deplibs 
$compiler_flags -o $output_objdir/$soname ${wl}--enable-auto-image-base 
-Xlinker --out-implib -Xlinker $lib'


Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static

2008-11-15 Thread Charles Wilson
Charles Wilson wrote:
 Of course, first I need to revise the dlltool patch and get it accepted;
 there have been some comments on the binutils list.

Done. Yay!
http://sourceware.org/ml/binutils/2008-11/msg00180.html

 Well, that, and it fixes a test that currently fails.
 Which one, and can you post output for failure as well as success with
 the patch, please?
 
 demo-exec after demo-shared, in the old test suite.
 
 I'll post the output(s) tonight or tomorrow.

Attached. The fixed output is from the original, unmodified patch that
started this thread.

--
Chuck



dlpreopen-failure.log.bz2
Description: Binary data


dlpreopen-fixed.log.bz2
Description: Binary data


Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static

2008-11-13 Thread Ralf Wildenhues
Hi Charles,

* Charles Wilson wrote on Thu, Nov 13, 2008 at 06:09:20AM CET:
 Ralf Wildenhues wrote:
  
  Well, --verbose is documented to be a reversal of --silent, and
  documented to be the default.  The fact that opt_verbose is never set is
  a limitation.  If fixed, that should better happen in a separate patch.
 
 Well, if --verbose is really the negation of --silent, then (unless the
 functionality is extended as you describe) the effect of --verbose
 should be to only unset opt_silent (which it does), and there should not
 exist any 'opt_verbose' variable.

Yes, I agree there is some inconsistency currently.

 In this case, to avoid backwards compatibility issues, the new I want
 really talkative output, but not --debug option should be something
 other than '--verbose' -- because that already has a specific meaning:
 negate--silent.
 
 --chatty?  (I know, it's idiomatic and that's bad. It's more a
 tongue-in-cheek suggestion anyway).
 
 Regardless, I think the current (2? 3?) usages of opt_verbose should be
 changed to !opt_silent.

20 uses of func_verbose, actually.  A bit much to undo, methinks.
OK, how about this.  It is a slight backward incompatibility, but
not a large one:
- --verbose undoes --silent *and* enables verbose output (that one with
  func_verbose),
- --no-silent *only* undoes --silent,

It should still be bearable for the user, in the sense that if you
use --verbose rather than --no-silent, it's not a big problem.
And we don't have to think about what
  --verbose --verbose --silent

causes, we can just make the last one win.

If you agree, then let's proceed this way.  I don't mind who writes the
patch.

  B) func_win32_libid() gives some confusing errors to users
 when (a) using recursive make, and (b) MAKEFLAGS does not
 contain $OBJDUMP. Add a diagnostic error message, rather
 than allowing $SED to die a horrible death.
[...]
 Actually, this may no longer be necessary given the _LT_DECL_OBJDUMP
 changes (I /said/ this was an old patch).  Here's the thread:
 http://cygwin.com/ml/cygwin/2008-09/msg00552.html

Ah, ok, thanks.

  When configuring with --disable-static, dlpreopen is very confused.
  First, libtool tries to extract the symbols -- using $NM and
  $global_symbols_pipe -- from the DLL. That works...poorly:
[...]
  OK, I see that this is problematic.  What I don't understand yet is:
  is there a way to extract only the interesting symbols from the DLL?
  I don't yet understand why we have to move to the import library.
 
 Because it's extremely tricky. You have to use objdump (not nm), and
 then search for the exported symbols which is non-trivial because you
 need a stateful parser -- maybe an awk program...

Hmm, yes, agreed.  Thanks for all your detailed explanations.  I'm glad
to not have had to analyze this myself.

 The ugliest part of my patch is the fallback code for deducing the
 dllname from an import library.  But that's *pretty* compared to mucking
 with objdump output.

OK.  :-)

 Alternatively, libtool used to embed an impgen program which could be
 ressurected to generate the symbol lists we need (rather than a .def
 file, as it used to do).  A better solution would be to push that
 functionality into dlltool (Hmm. I need to generate a symbol list or
 def file for a DLL. but 'dlltool --output-def=my.def my.dll' doesn't do
 what you expect).  Even so, the core functionality of impgen would need
 some improvements (especially so as to indicate DATA items, which it
 doesn't do at present).
 http://loreley.ath.cx/cygwin/impgen/impgen.c
 
 But then, you're back to requiring a very recent binutils, or providing
 a fallback -- see objdump ugliness above.

ACK.

  Once I understand that, I can better judge the rest of the patch.
 
 Well, one reason I sat on this for so long was the 'fallback' mechanism
 for deducing the dll name from the import library was just so...hideous.
 And it wasn't a fallback -- it was the only mechanism since I hadn't yet
 enhanced dlltool.

Do you steer dlltool development, BTW?

 The only reason to allow it is because (hopefully) that ugly fallback
 code can get flagged with a warning, and maybe in a year or so get
 removed.

Sounds like a good idea.

 Well, that, and it fixes a test that currently fails.

Which one, and can you post output for failure as well as success with
the patch, please?

  for example mentioning in the mail
  whether you considered cygwin or cegcc or so would be helpful for
  review).
 
 cygwin and mingw yes. I know nothing about cegcc.

OK, that's what I figured.

In summary, it'd be great if you could redo the patch(es) along the
comments in the previous message (but read below first).
Couple more nits inline:

 --- a/libltdl/config/ltmain.m4sh
 +++ b/libltdl/config/ltmain.m4sh

 @@ -1991,10 +1992,36 @@ extern \C\ {
 func_verbose extracting global C symbols from \`$dlprefile'
 func_basename $dlprefile
 name=$func_basename_result
 -   $opt_dry_run || {
 - eval 

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static

2008-11-13 Thread Brian Dessent
Ralf Wildenhues wrote:

 I'm actually not sure whether _GLOBAL__F[ID]_.* can appear on w32.
 Do you know?  They should happen with C++ code using constructors
 and destructors IIRC.

Yes they do occur, although not matching that regexp.  For one, they
will have two leading underscores before the G, as with all symbols
compared to their linux counterparts (i.e. __USER_LABEL_PREFIX__ is _
on Cygwin/MinGW.)  For another I would have expected the regexp to match
[FID] not F[ID] as there seems to generally be only one character in
that position, whose purpose is illuminated by this comment in
gcc/tree.c:

/* Generate a name for a special-purpose function function.
   The generated name may need to be unique across the whole link.
   TYPE is some string to identify the purpose of this function to the
   linker or collect2; it must start with an uppercase letter,
   one of:
   I - for constructors
   D - for destructors
   N - for C++ anonymous namespaces
   F - for DWARF unwind frame information.  */

A testcase:

$ echo struct foo { int x; foo() : x(42) {}; }; static foo bar; \
  | g++ -x c++ -S - -o - -O2 
.file   
.section.ctors,w
.align 4
.long   __GLOBAL__I__77970994_840EDDA1
.lcomm _bar,16
.text
.align 2
.p2align 4,,15
.def__GLOBAL__I__77970994_840EDDA1; .scl3;  .type  
32; .endef
__GLOBAL__I__77970994_840EDDA1:
pushl   %ebp
movl$42, %eax
movl%esp, %ebp
popl%ebp
movl%eax, _bar
ret

Also:

$ c++filt __GLOBAL__I__foobar
global constructors keyed to _foobar

$ c++filt __GLOBAL__D__foobar
global destructors keyed to _foobar

$ c++filt __GLOBAL__FI__foobar
__GLOBAL__FI__foobar

This implies that 'FI' is not valid, or at least not recognised by the
demangler as significant.

Brian




Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static

2008-11-13 Thread libtool

On Thu, 13 Nov 2008 22:09:07 +0100, Ralf Wildenhues said:
 OK, how about this.  It is a slight backward incompatibility, but
 not a large one:
 - --verbose undoes --silent *and* enables verbose output (that one with
   func_verbose),
 - --no-silent *only* undoes --silent,
 
 It should still be bearable for the user, in the sense that if you
 use --verbose rather than --no-silent, it's not a big problem.
 And we don't have to think about what
   --verbose --verbose --silent
 
 causes, we can just make the last one win.
 
 If you agree, then let's proceed this way.  I don't mind who writes the
 patch.

That sounds good to me. The help output would need a little re-wording:

#   --quiet, --silentdon't print informational messages
#   -v, --verboseprint informational messages (default)
#   --no-silent  ???

I'll let you do that. g

   B) func_win32_libid() gives some confusing errors to users
  when (a) using recursive make, and (b) MAKEFLAGS does not
  contain $OBJDUMP. Add a diagnostic error message, rather
  than allowing $SED to die a horrible death.
 [...]
  Actually, this may no longer be necessary given the _LT_DECL_OBJDUMP
  changes (I /said/ this was an old patch).  Here's the thread:
  http://cygwin.com/ml/cygwin/2008-09/msg00552.html
 
 Ah, ok, thanks.

I'll remove any of these bits from the revised patch(es).

  Well, one reason I sat on this for so long was the 'fallback' mechanism
  for deducing the dll name from the import library was just so...hideous.
  And it wasn't a fallback -- it was the only mechanism since I hadn't yet
  enhanced dlltool.
 
 Do you steer dlltool development, BTW?

No. I've contributed a few patches over the years to dlltool and
binutils, but that's it.

  The only reason to allow it is because (hopefully) that ugly fallback
  code can get flagged with a warning, and maybe in a year or so get
  removed.
 
 Sounds like a good idea.

Of course, first I need to revise the dlltool patch and get it accepted;
there have been some comments on the binutils list.

  Well, that, and it fixes a test that currently fails.
 
 Which one, and can you post output for failure as well as success with
 the patch, please?

demo-exec after demo-shared, in the old test suite.

I'll post the output(s) tonight or tomorrow.

 Hmm.  I reviewed this whole function, and only when done I asked myself
 this, more radical question: we go great lengths here to find out a
 name.  Iff we have a *.la file to go with the implib, can't we just
 *know* the name?  I mean, we produced that thing, it has the expected
 name, no?  That's what the *.la file was designed for: to not have to
 look into the binary files for information.
 
 Or is this purely for import libraries not created with libtool (and
 people who throw away *.la files)?

The information (e.g. library to dlpreopen) is passed in $dlprefiles.
But, if that filename is .la:  
func_mode_link():
...
dlfiles|dlprefiles)
  if test $preload = no; then
# Add the symbol object into the linking commands.
func_append compile_command  @SYMFILE@
func_append finalize_command  @SYMFILE@
preload=yes
  fi
  case $arg in
  *.la | *.lo) ;;  # We handle these cases below.
  ...

...much later...
  *.la)
# A libtool-controlled library.

if test $prev = dlfiles; then
  # This library was specified with -dlopen.
  dlfiles=$dlfiles $arg
  prev=
elif test $prev = dlprefiles; then
  # The library was specified with -dlpreopen.
  dlprefiles=$dlprefiles $arg
  prev=
else
  deplibs=$deplibs $arg
fi
continue
;;

So far, so good.  But then we eventually source the .la file, and end up
here (this is, in fact, what's happening in the demo-shared case):

   # This library was specified with -dlpreopen.
if test $pass = dlpreopen; then
  if test -z $libdir  test $linkmode = prog; then
func_fatal_error only libraries may -dlpreopen a
convenience library: \`$lib'
  fi
  # Prefer using a static library (so that no silly _DYNAMIC
  symbols
  # are required to link).
  if test -n $old_library; then
newdlprefiles=$newdlprefiles $dir/$old_library
# Keep a list of preopened convenience libraries to check
# that they are being used correctly in the link pass.
test -z $libdir  \
dlpreconveniencelibs=$dlpreconveniencelibs
$dir/$old_library
  # Otherwise, use the dlname, so that lt_dlopen finds it.
  elif test -n $dlname; then
newdlprefiles=$newdlprefiles $dir/$dlname
  else
newdlprefiles=$newdlprefiles $dir/$linklib
  fi
fi # $pass = dlpreopen

We've stored the DLL name as just ONE of the entries in $newdlprefiles.

Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static

2008-11-13 Thread Ralf Wildenhues
* [EMAIL PROTECTED] wrote on Fri, Nov 14, 2008 at 12:28:36AM CET:
 
 The point is, we perhaps STARTED with the .la file, but the whole point
 of the dlpreopen $pass is to replace each .la file in $dlprefiles with
 the name of the object from which the symbols should be extracted, to
 build the symbol table. So, pick one: either the DLL, or the import
 library (there is no static lib, the failure mode in question occurs
 when --disable-static).
 
 If you pick DLL -- then it's real hard to get the symbols (objdump
 ugliness, plus figuring out which ones are DATA).
 
 If you pick implib -- then it's real hard to get the correct DLL name
 (but not nearly as hard as extracting the correct symbols from the dll).
 
 But the name of the .la file is no longer available.

But that's a problem that can be solved.

# turn $1 into a string suitable for a shell variable name
func_tr_sh ()
{
 ... # typically forks, except maybe with bash ${var/subst/repl}
}

# when treating $dlprefile, save the corresponding .la file name:
func_tr_sh $dlprefile
eval libfile_$tr_sh_result=\$corresponding_dotla_file

# later, when searching for the .la file, test libfile$tr_sh_result
# for contents

What do you think?

Cheers,
Ralf




Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static

2008-11-13 Thread Charles Wilson
Ralf Wildenhues wrote:
 The point is, we perhaps STARTED with the .la file, but the whole point
 of the dlpreopen $pass is to replace each .la file in $dlprefiles with
 the name of the object from which the symbols should be extracted, to
 build the symbol table. So, pick one: either the DLL, or the import
 library (there is no static lib, the failure mode in question occurs
 when --disable-static).

 If you pick DLL -- then it's real hard to get the symbols (objdump
 ugliness, plus figuring out which ones are DATA).

 If you pick implib -- then it's real hard to get the correct DLL name
 (but not nearly as hard as extracting the correct symbols from the dll).

 But the name of the .la file is no longer available.
 
 But that's a problem that can be solved.
 
 # turn $1 into a string suitable for a shell variable name
 func_tr_sh ()
 {
  ... # typically forks, except maybe with bash ${var/subst/repl}
 }
 
 # when treating $dlprefile, save the corresponding .la file name:
 func_tr_sh $dlprefile
 eval libfile_$tr_sh_result=\$corresponding_dotla_file
 
 # later, when searching for the .la file, test libfile$tr_sh_result
 # for contents
 
 What do you think?

That would work. But it only gets rid of the grotty find the name of
the DLL given the implib problem -- which is not a small thing, of course.

But that presupposes that my change to the dlpreopen $pass, where on
cygwin|mingw we replace the la file in the $dlprefiles list with the
implib, stands. Did I convince you we needed this bit:

- elif test -n $dlname; then
-   newdlprefiles=$newdlprefiles $dir/$dlname
+ # Except on mingw|cygwin, where we must use the import library,
+ # so lt_dlopen is handled in another way
  else
-   newdlprefiles=$newdlprefiles $dir/$linklib
+   case $host in
+ *cygwin* | *mingw* )
+   newdlprefiles=$newdlprefiles $dir/$linklib
+;;
+ * )
+   if test -n $dlname; then
+ newdlprefiles=$newdlprefiles $dir/$dlname
+   else
+ newdlprefiles=$newdlprefiles $dir/$linklib
+   fi
+;;
+   esac

If so, then I guess the other code section would look like

 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* )
+# 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_$tr_sh_result
+  $opt_dry_run || {
+if test -n $curr_lafile  func_lalib_p $curr_lafile; then
+  # geez. does this need to happen in a subshell, to
+  # avoid clobbering our current variable values?
+  source $curr_lafile
+  if test -n $dlname ; then
+func_basename $dlname
+dlbasename=$func_basename_result
+eval '$ECHO : $dlbasename  $nlist'
+  else
+func_warning Could not compute DLL name from $name
+eval '$ECHO : $name   $nlist'
+  fi
+else
+  func_warning Could not determing .la name from $name
+  eval '$ECHO : $name   $nlist'
+fi
+eval $NM $dlprefile 2/dev/null | $global_symbol_pipe |
+  $SED -e '/I __imp/d' -e 's/I __nm_/D /;s/_nm__//' 
'$nlist'
+  }
+else # not an import lib
+  $opt_dry_run || {
+  eval '$ECHO : $name   $nlist'
+  eval $NM $dlprefile 2/dev/null | $global_symbol_pipe 
'$nlist'
+  }
+fi
+;;
+  *)
etc.

Is that the idea?

--
Chuck




Re: [PATCH] [cygwin|mingw] fix dlpreopen with --disable-static

2008-11-12 Thread Charles Wilson
Ralf Wildenhues wrote:
 Hello Charles,
 
 thanks for the patch!  Quoting a bit out of order:
 
 * Charles Wilson wrote on Sat, May 31, 2008 at 07:01:45PM CEST:
 * libltdl/config/ltmain.m4sh (func_enable_tag): allow
 --verbose to set opt_verbose
 [...]
 A) libtool --verbose does not actually set opt_verbose. In fact,
nothing ever sets opt_verbose true.  Should all uses of opt_verbose
be replaced by !opt_silent, or should (as I have done in this patch)
--verbose set both opt_silent false, and opt_verbose true?
 
 Well, --verbose is documented to be a reversal of --silent, and
 documented to be the default.  The fact that opt_verbose is never set is
 a limitation.  If fixed, that should better happen in a separate patch.

Well, if --verbose is really the negation of --silent, then (unless the
functionality is extended as you describe) the effect of --verbose
should be to only unset opt_silent (which it does), and there should not
exist any 'opt_verbose' variable.

 Several possibilities:
 - a new switch for being very verbose.
 - allowing
  --verbose
  --silent --verbose --verbose
 
 to output verbose output.  That way one can still undo the effects of
 --silent without verbose output; however, now the user needs to know
 whether --silent was passed earlier on the command line (this is a
 backward incompatibility).  Not nice (imagine --silent being passed in
 AM_LIBTOOLFLAGS).

In this case, to avoid backwards compatibility issues, the new I want
really talkative output, but not --debug option should be something
other than '--verbose' -- because that already has a specific meaning:
negate--silent.

--chatty?  (I know, it's idiomatic and that's bad. It's more a
tongue-in-cheek suggestion anyway).

Regardless, I think the current (2? 3?) usages of opt_verbose should be
changed to !opt_silent.

 B) func_win32_libid() gives some confusing errors to users
when (a) using recursive make, and (b) MAKEFLAGS does not
contain $OBJDUMP. Add a diagnostic error message, rather
than allowing $SED to die a horrible death.
 
 Why should external variables ever matter here?  ,,,
 Anyway, if this should be needed, then it should be a separate change as
 well ...

Actually, this may no longer be necessary given the _LT_DECL_OBJDUMP
changes (I /said/ this was an old patch).  Here's the thread:
http://cygwin.com/ml/cygwin/2008-09/msg00552.html

 
 Now to the meat of the patch:
 
 When configuring with --disable-static, dlpreopen is very confused.
 First, libtool tries to extract the symbols -- using $NM and
 $global_symbols_pipe -- from the DLL. That works...poorly:

 Here's a snippet from helldl.exeS.c when --disable-static:
 ==
 /* External symbol declarations for the compiler. */
 extern int _CTOR_LIST__();
 extern int _DTOR_LIST__();
 extern char _RUNTIME_PSEUDO_RELOC_LIST_END__;
 extern char _RUNTIME_PSEUDO_RELOC_LIST__;
 extern int __CTOR_LIST__();
 extern int __DTOR_LIST__();
 ...
 extern int printf();
 extern int puts();
 extern int realloc();
 
 OK, I see that this is problematic.  What I don't understand yet is:
 is there a way to extract only the interesting symbols from the DLL?
 I don't yet understand why we have to move to the import library.

Because it's extremely tricky. You have to use objdump (not nm), and
then search for the exported symbols which is non-trivial because you
need a stateful parser -- maybe an awk program...

objdump -p cyghello-2.dll | awk '/^Export Address Table/{foundEdata=1}
/[Ordinal/Name Pointer] Table/{foundTable=1}
{if (foundTable==1  foundEdata==1) print $0 }'

is a start. But, the objdump output doesn't indicate whether a symbol is
a function or data. To figure that out (from objdump) you need to parse
the reloc section of the report...or cross-reference to the nm output,
which indicates function or data, but doesn't tell you which DLL the
symbol actually came from: $arg, or one of $arg's dependencies.

The reason nm is not acceptable, is it detects all symbols imported and
exported by the DLL (assuming the DLL has not had its symbol information
stripped, in which case nm reports nothing).  How can we construct a
regex that accepts _foo, but rejects _free?

cyghello-2.dll:6c641020 T _foo
cyghello-2.dll:6c6413c0 T _free

The ugliest part of my patch is the fallback code for deducing the
dllname from an import library.  But that's *pretty* compared to mucking
with objdump output.

Alternatively, libtool used to embed an impgen program which could be
ressurected to generate the symbol lists we need (rather than a .def
file, as it used to do).  A better solution would be to push that
functionality into dlltool (Hmm. I need to generate a symbol list or
def file for a DLL. but 'dlltool --output-def=my.def my.dll' doesn't do
what you expect).  Even so, the core functionality of impgen would need
some improvements (especially so as to indicate DATA items, which it
doesn't do at present).
http://loreley.ath.cx/cygwin/impgen/impgen.c

But then, you're back to