[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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:
     rm -f *$(EXESUF)

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
    RM = rf -f

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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]