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

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

[Philippe Mathieu-Daudé]

Reported-by: Sameeh Jubran <sjubran@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
original report from Sameeh Jubran:
http://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg00906.html

 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 *~ */*~
 	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

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

[Daniel P. Berrange]

On Mon, Jul 24, 2017 at 10:15:30PM -0300, Philippe Mathieu-Daudé wrote:
> Reported-by: Sameeh Jubran <sjubran@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> original report from Sameeh Jubran:
> http://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg00906.html
> 
>  Makefile  | 2 +-
>  configure | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Michael Roth]

Quoting Philippe Mathieu-Daudé (2017-07-24 20:15:30)
> Reported-by: Sameeh Jubran <sjubran@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@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

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 *~ */*~
>         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
> 
> 

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

[Philippe Mathieu-Daudé]

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 <sjubran@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@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 
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"
The only executables which doesn't match %.exe is qemu-ga, so the 
'clean' target remove all .exe _but_ qemu-ga.exe.

Now if by "this is not an issue" you mean it is not a bug, I agree this 
can wait 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
>>
>>
> 

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 <sjubran@redhat.com>
> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@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
> >>
> >>
> > 
> 

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

[Michael Roth]

Quoting Michael Roth (2017-07-25 21:53:41)
> 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 <sjubran@redhat.com>
> > >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@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,

On w32 I mean, sorry.

> 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
> > >>
> > >>
> > > 
> > 

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

[Michael Roth]

Quoting Michael Roth (2017-07-25 21:56:14)
> Quoting Michael Roth (2017-07-25 21:53:41)
> > 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 <sjubran@redhat.com>
> > > >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@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,
> 
> On w32 I mean, sorry.

Ugh, no, on posix. Let's just try that again:

It's a bit ugly since `rm -f ... qemu-ga${EXESUF}` would silently fail on posix,
but `rm -f $TOOLS qemu-ga ...` already silently fails since qemu-ga already gets
removed via $TOOLS, so it's not any worse.

> 
> > 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
> > > >>
> > > >>
> > > > 
> > > 

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

[Philippe Mathieu-Daudé]

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.

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1212 bytes --]

On 07/25/2017 10:28 PM, Philippe Mathieu-Daudé wrote:

>>> - 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 that's the whole point of 'rm -f' - to silently ignore files that
didn't exist.

>>
>> 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
> 

Absolutely not.  We can at least assume a mingw-like environment that
has all the usual tools like rm.

>>>
>>> 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.

But that's what 'rm -f' already does - I don't see the point in adding a
racy check for existence prior to deletion, when deletion already
handles the race.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

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

[Philippe Mathieu-Daudé]

As noticed by Michael Roth the ./configure entry for qemu-ga is missing 
the $(EXESUF) on purpose (see fafcaf1d).

Patch dropped for 2.10

On 07/24/2017 10:15 PM, Philippe Mathieu-Daudé wrote:
> Reported-by: Sameeh Jubran <sjubran@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> original report from Sameeh Jubran:
> http://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg00906.html
> 
>   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 *~ */*~
>   	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
>