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 <sjub...@redhat.com> > >> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> > >> --- > >> 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 > >> > >> > > >