Re: [Qemu-devel] [PATCH] configure: fix qemu-ga missing '.exe' extension on windows

[Michael Roth]

Quoting Philippe Mathieu-Daudé (2017-07-25 20:45:31)
> Hi Michael,
> 
> On 07/25/2017 08:03 PM, Michael Roth wrote:
> > Quoting Philippe Mathieu-Daudé (2017-07-24 20:15:30)
> >> Reported-by: Sameeh Jubran <address@hidden>
> >> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> >> ---
> >> original report from Sameeh Jubran:
> >> http://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg00906.html
> > 
> > AFAICT the issue discussed in the context of that patch is simply that
> > the qemu-ga.exe file isn't deleted as part of `make clean`, which does
> > seem to be an issue.
> > 
> > But qemu.ga.exe is created just fine as part of `make qemu-ga`, at least
> > as of:
> > 
> >    fafcaf1d build: qemu-ga: add 'qemu-ga' build target for w32
> > 
> 
> Yes, on w32 if "make ASDF" invokes the linker, it will create ASDF.exe.
> 
> > The 'qemu-ga' target is actually an intermediate target which, on w32,
> > creates the MSI package (if configured) as well as qemu-ga.exe.
> > 
> >>
> >>   Makefile  | 2 +-
> >>   configure | 2 +-
> >>   2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Makefile b/Makefile
> >> index ef721480eb..5f18243d05 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -490,7 +490,7 @@ clean:
> >>          rm -f qemu-options.def
> >>          rm -f *.msi
> >>          find . \( -name '*.so' -o -name '*.dll' -o -name '*.mo' -o -name 
> >> '*.[oda]' \) -type f -exec rm {} +
> >> -       rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) qemu-ga TAGS 
> >> cscope.* *.pod *~ */*~
> >> +       rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) TAGS cscope.* 
> >> *.pod *~ */*~
> 
> On win32, this patch add qemu-ga.exe instead of qemu-ga in $TOOLS, so 
> when the 'clean' target is executed it removes qemu-ga.exe (Sameeh 

Ok, that makes more sense, but it's not what the commit msg implies.

> Jubran report). Before this patch the $TOOLS looks like:
> "qemu-img.exe qemu-io.exe qemu-nbd.exe ivshmem-client.exe 
> ivshmem-server.exe qemu-ga fsdev/virtfs-proxy-helper.exe"

That change was done explicitly via fafcaf1d so that `make qemu-ga` and
`make` without --disable-tools specified to configure will generate the
MSI package when the user configures it. So, unlike the other $TOOLS
targets, the qemu-ga "distributables" encompass more than just the .exe
on w32, so we use the "qemu-ga" target instead of "qemu-ga.exe" directly.

Reverting that change to coax `make clean` into cleaning up qemu-ga.exe
means that `make` no longer builds the qemu-ga-*.msi when the user
configures it, which is a regression.

> The only executables which doesn't match %.exe is qemu-ga, so the 
> 'clean' target remove all .exe _but_ qemu-ga.exe.

As with Sameeh's original patch, the qemu-ga target would already
require special handling to deal with qemu-ga-*.msi file. We should
similarly just cleanup qemu-ga.exe as a special case instead of
modifying $TOOLS, since that brings about unecessary side-effects
described above.

As a workaround to the issue you/Peter pointed out with Sameeh's patch
(nuking the entire source tree for posix builds where $EXESUF == ""), I
proposed we just do:

  make clean:
    ...
  ifneq($EXESUF,)
    rm -f *$(EXESUF)
  endif

But given your clarification here, I understand that $TOOLS already
takes care of everything except qemu-ga.exe, so I think you've already
mentioned the most straightforward fix in the other thread:

- rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod 
*~ */*~
+ rm -f $(filter-out %.tlb,$(TOOLS)) $(HELPERS-y) qemu-ga${EXESUF} TAGS 
cscope.* *.pod *~ */*~

It's a bit ugly since `rm -f qemu-ga.exe` would silently fail on posix,
but `rm -f $TOOLS qemu-ga ...` already silently fails since it already gets
removed via $TOOLS.

Alternatively, you can explicitly check for qemu-ga.exe and remove it if
it exists. I would consider either acceptable, but not this patch as it
currently stands.

> 
> Now if by "this is not an issue" you mean it is not a bug, I agree this 
> can wait 2.11.

I just mean the issue as described in the commit msg.

For the `make clean` stuff, if it's simple enough it might be acceptable for
rc1 if you can get it out this week. Otherwise we can wait for 2.11.

> 
> Regards,
> 
> Phil.
> 
> >>          rm -f fsdev/*.pod
> >>          rm -f qemu-img-cmds.h
> >>          rm -f ui/shader/*-vert.h ui/shader/*-frag.h
> >> diff --git a/configure b/configure
> >> index 48d4d7a2a7..f8b1d014d7 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -5073,7 +5073,7 @@ fi
> >>
> >>   if [ "$guest_agent" != "no" ]; then
> >>     if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" -o 
> >> "$mingw32" = "yes" ] ; then
> >> -      tools="qemu-ga $tools"
> >> +      tools="qemu-ga\$(EXESUF) $tools"
> >>         guest_agent=yes
> >>     elif [ "$guest_agent" != yes ]; then
> >>         guest_agent=no
> >> -- 
> >> 2.13.3
> >>
> >>
> > 
>