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

[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 2>&1 -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, insert the new content, copy the rest of
the lt output" method.

Since that's really slow...I figured only windows $hosts should pay that
penalty (even if the penalty only occurs during LT_OUTPUT).

> (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.)

Ack -- that's why my version was named _LT_PROG_FUNCTION_REPLACE (the
other was that I needed a different internal implementation of that
macro, as described above).

But...according to Ralf's message, this discussion is OBE. I'll revert
back to a revised version of the 'take 7' patch.

--
Chuck