|
From: | Philippe Mathieu-Daudé |
Subject: | Re: [Qemu-devel] [PATCH] configure: fix qemu-ga missing '.exe' extension on windows |
Date: | Wed, 26 Jul 2017 00:28:34 -0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 07/25/2017 11:56 PM, Michael Roth wrote:
Quoting Michael Roth (2017-07-25 21:53:41)Quoting Philippe Mathieu-Daudé (2017-07-25 20:45:31)
[...]
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.
I see, I didn't think about running git blame on this line, sorry.
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
Now I understand your fix. And looking at the Makefile structure it seems to be written with only UNIX system in mind (well, Windows world was not written with UNIX philosophy and GNU Make has some limitations out of it). I can't think of a non-ugly short way to resolve Sameeh's issue. So I'll drop the patch I purposed for 2.10.
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,On w32 I mean, sorry.
side question: should we use $(RM) with something like: ifeq ($(OS),Windows_NT) RM = cmd //C del //Q //F else RM = rf -f endif
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.
[Prev in Thread] | Current Thread | [Next in Thread] |