Re: Repost: [PATCH] [cygwin|mingw] Create UAC manifest files.

[Ralf Wildenhues]

Hello,

* Charles Wilson wrote on Sun, Feb 21, 2010 at 07:31:15AM CET:
>       [cygwin|mingw] Create UAC manifest files.
> 
>       * libltdl/config/ltmain.m4sh (func_emit_exe_manifest): New
>       function.
>       (func_mode_link) [cygwin|mingw]: Create manifest files for
>       wrapper and target exe when target name matches heuristic that
>       triggers UAC problems for newer win32 OSs. Clean up
>       $cwrapper.manifest on error. Ensure manifest files have
>       executable permission.
>       (func_mode_uninstall): Clean up manifest files.
>       Various reports by Eric Blake, Kai Tietz, and Cesar Strauss.

I have the following issues/questions with this patch:

0) I still haven't fully understood the semantics here.  Are the
manifest files only needed at link editor time or at runtime?  If the
latter, then I don't see how the manifest file ever gets to be installed
at `make install' time, nor uninstalled at `make uninstall' time?
If the former, then why doesn't a --mode=link remove the manifest file
before exiting (non-interrupted)?

1) --mode=clean removes .manifest files even if --mode=link did not
create them, namely when the name of the program does not match
*instal*|*patch*|*setup*|*update*.  Neither this nor manifest creation
and usage, nor (0) are covered in the libtool testsuite.  But need to
be.

2) automake/lib/am/progs.am does not use --mode=clean for cleaning
programs which means the manifest files for cwrappers will be leftover.
I expect this to cause a distcheck failure.  So Automake needs to be
fixed.  I'm willing to change Automake to use 'libtool --mode=clean' for
when new-enough libtool is used, but the comment

## FIXME: In the future (i.e., when it works) it would be nice to delegate
## this task to `libtool --mode=clean'.

still requires some history digging for cases where this doesn't work as
expected.  These cases should be investigated and covered by the libtool
testsuite.

3) This patch will overwrite a .manifest file created in the build dir,
thus may break a package that create them through some other means.
Note that here, I'm not speaking about the manifest below .libs, and I
understand that that is necessary as well, and that if the user creates
one, it is probably a bug that we don't use that for the to-be-installed
program; not sure if it's always harmless to then-reuse that for the
cwrapper as well.

IIUC then this particular regression could be worked around inside a
package by using an embedded manifest, since that will override an
external .manifest file.  Right?

Of course libtool shouldn't remove a user-created manifest file as in
(1) at all.  Not sure how to fix that if (0) is "runtime".

4) How many code instances does this patch help *in practice*?  I
suppose you guys know since you've used this for a lot of packages,
but couldn't find data in the cited email threads.  I'd like to gauge
the importance of this being fixed in libtool against the regressions
this patch may cause, and the cost of waiting (or not waiting) for an
Automake fix to propagate.

The alternative to this patch would be workarounds in said packages to
add explicit code to create embedded manifests (that then apply to the
to-be-installed program).  We'd still need a manifest for the cwrapper.
Hmm, that means waiting for (2) is moot, unless we embed a manifest into
the cwrapper.  Ugh.

5) I'd prefer a nod from Peter R. as to whether this is fixable easily:

> The only comment NOT addressed by this version is: Peter Rosin doesn't
> want this code activated for msvc, but I'm not sure this patch should be
> modified to guard against a compiler, whose support (in Peter's branch)
> has not itself yet been merged into master.  And I don't know what
> symbol to use; maybe Peter's branch has a special variable that should
> be tested?
> 
> Anyway, IF this patch should be modified to "protect" msvc, I'd sure
> like somebody to tell me how that should be done:
> 
>               # note: this script will not be executed, so do not chmod.
>               if test "x$build" = "x$host" ; then
> +               # Create the UAC manifests first if necessary (but the
> +               # manifest files must have executable permission
> regardless).
> 
> like this, maybe:
>               if [ $CC != msvc ]; then
> 
> +               case $output_name in
> +                 *instal*|*patch*|*setup*|*update*)
> 
> Or tell me that such protection should be deferred until Peter's stuff
> is merged?

Well, my guess is that this should be addressed at the time master is
next merged to pr-msvc-support, either in the merge or in a followup
commit.  To prevent even more friction caused from my incredibly slow
progress on libtool, releases, and merging of said branch, I'd try to
help with that merge if that helps, and would like to apologize again
to Peter.

6)
> +  case $host in
> +  i?86-*-* )   echo '     processorArchitecture="x86"' ;;
> +  ia64-*-* )   echo '     processorArchitecture="ia64"' ;;
> +  x86_64-*-* ) echo '     processorArchitecture="amd64"' ;;
> +  *)           echo '     processorArchitecture="*"' ;;
> +  esac

Please use an intermediate variable to save on code size here and save
one cat instance, i.e., move the case statement up.

Thanks,
Ralf