[Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Eric Farman]

How often does one really do a "make clean" ?  Rather infrequently,
as I only stumbled on this today.

Perhaps I have missed the RM variable somewhere, as I see similar syntax
in some of the tests/tcg/ Makefiles, but I don't see it being set here.
My configure statement isn't terribly interesting, just enabling debug
for an s390x target, and as such there's no RM variable in its output.
I'll trust that Thomas will chime in with where it should have been.
In the meantime, this does the trick for me.

I didn't rebuild the image file after this change, because it didn't seem
necessary.  Let me know if that was a bad assumption, and I can submit one.

Eric Farman (1):
  pc-bios/s390-ccw: Use rm command during make clean

 pc-bios/s390-ccw/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.11.2

[Qemu-devel] [PATCH 1/1] pc-bios/s390-ccw: Use rm command during make clean

[Eric Farman]

This reverts what I think was an accidental change that replaced
the "rm -f" command with an undefined variable RM, and causes the
"make clean" command to fail for a s390 target:

make[1]: Entering directory '/usr/src/qemu/build/pc-bios/s390-ccw'
rm -f *.timestamp
*.o *.d *.img *.elf *~ *.a
/bin/sh: *.o: command not found
Makefile:39: recipe for target 'clean' failed
make[1]: *** [clean] Error 127
make[1]: Leaving directory '/usr/src/qemu/build/pc-bios/s390-ccw'
Makefile:489: recipe for target 'clean' failed
make: *** [clean] Error 1

Fixes: 3e4415a751d2 ("pc-bios/s390-ccw: Add core files for the network
                      bootloading program")
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com>
---
 pc-bios/s390-ccw/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index cbae74522a..6d0c2ee691 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -36,4 +36,4 @@ s390-netboot.img:
 endif
 
 clean:
-	$(RM) *.o *.d *.img *.elf *~ *.a
+	rm -f *.o *.d *.img *.elf *~ *.a
-- 
2.11.2

Re: [Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Thomas Huth]

On 14.08.2017 22:44, Eric Farman wrote:
> How often does one really do a "make clean" ?  Rather infrequently,
> as I only stumbled on this today.
> 
> Perhaps I have missed the RM variable somewhere, as I see similar syntax
> in some of the tests/tcg/ Makefiles, but I don't see it being set here.
> My configure statement isn't terribly interesting, just enabling debug
> for an s390x target, and as such there's no RM variable in its output.
> I'll trust that Thomas will chime in with where it should have been.
> In the meantime, this does the trick for me.

RM is one of the variables that should be pre-initialized by Make, and
AFAIK should be used to increase portability (well, it's likely not
important for QEMU since we require a posix-shell like built environment
anyway).

According to the info page of Make, chapter "10.3 Variables Used by
Implicit Rules":

`RM'
     Command to remove a file; default `rm -f'.

I've also checked it again and "make clean" works fine here (using GNU
Make 3.82). Which version of Make (and Linux distro) are you using?

Anyway, maybe I also simply missed something, so I'm certainly also fine
with the patch to revert it to "rm -f".

 Thomas

Re: [Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Cornelia Huck]

On Tue, 15 Aug 2017 07:02:10 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 14.08.2017 22:44, Eric Farman wrote:
> > How often does one really do a "make clean" ?  Rather infrequently,
> > as I only stumbled on this today.
> > 
> > Perhaps I have missed the RM variable somewhere, as I see similar syntax
> > in some of the tests/tcg/ Makefiles, but I don't see it being set here.
> > My configure statement isn't terribly interesting, just enabling debug
> > for an s390x target, and as such there's no RM variable in its output.
> > I'll trust that Thomas will chime in with where it should have been.
> > In the meantime, this does the trick for me.  
> 
> RM is one of the variables that should be pre-initialized by Make, and
> AFAIK should be used to increase portability (well, it's likely not
> important for QEMU since we require a posix-shell like built environment
> anyway).
> 
> According to the info page of Make, chapter "10.3 Variables Used by
> Implicit Rules":
> 
> `RM'
>      Command to remove a file; default `rm -f'.
> 
> I've also checked it again and "make clean" works fine here (using GNU
> Make 3.82). Which version of Make (and Linux distro) are you using?

Interesting. It fails for me with GNU Make 3.82 on my RHEL guest as
well.

> Anyway, maybe I also simply missed something, so I'm certainly also fine
> with the patch to revert it to "rm -f".

Given that other bios makefiles use rm -f as well, let's just change
back until we figure out what's wrong.

Re: [Qemu-devel] [PATCH 1/1] pc-bios/s390-ccw: Use rm command during make clean

[Cornelia Huck]

On Mon, 14 Aug 2017 22:44:50 +0200
Eric Farman <farman@linux.vnet.ibm.com> wrote:

> This reverts what I think was an accidental change that replaced
> the "rm -f" command with an undefined variable RM, and causes the
> "make clean" command to fail for a s390 target:
> 
> make[1]: Entering directory '/usr/src/qemu/build/pc-bios/s390-ccw'
> rm -f *.timestamp
> *.o *.d *.img *.elf *~ *.a
> /bin/sh: *.o: command not found
> Makefile:39: recipe for target 'clean' failed
> make[1]: *** [clean] Error 127
> make[1]: Leaving directory '/usr/src/qemu/build/pc-bios/s390-ccw'
> Makefile:489: recipe for target 'clean' failed
> make: *** [clean] Error 1
> 
> Fixes: 3e4415a751d2 ("pc-bios/s390-ccw: Add core files for the network
>                       bootloading program")
> Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com>
> ---
>  pc-bios/s390-ccw/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
> index cbae74522a..6d0c2ee691 100644
> --- a/pc-bios/s390-ccw/Makefile
> +++ b/pc-bios/s390-ccw/Makefile
> @@ -36,4 +36,4 @@ s390-netboot.img:
>  endif
>  
>  clean:
> -	$(RM) *.o *.d *.img *.elf *~ *.a
> +	rm -f *.o *.d *.img *.elf *~ *.a

Thanks, applied with a tweaked commit message:

pc-bios/s390-ccw: Use rm command during make clean
    
This reverts a change that replaced the "rm -f" command with the
undefined variable RM (expected to be set by make), and causes the
"make clean" command to fail for a s390 target:
(...)

Re: [Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Thomas Huth]

On 15.08.2017 09:03, Cornelia Huck wrote:
> On Tue, 15 Aug 2017 07:02:10 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> On 14.08.2017 22:44, Eric Farman wrote:
>>> How often does one really do a "make clean" ?  Rather infrequently,
>>> as I only stumbled on this today.
>>>
>>> Perhaps I have missed the RM variable somewhere, as I see similar syntax
>>> in some of the tests/tcg/ Makefiles, but I don't see it being set here.
>>> My configure statement isn't terribly interesting, just enabling debug
>>> for an s390x target, and as such there's no RM variable in its output.
>>> I'll trust that Thomas will chime in with where it should have been.
>>> In the meantime, this does the trick for me.  
>>
>> RM is one of the variables that should be pre-initialized by Make, and
>> AFAIK should be used to increase portability (well, it's likely not
>> important for QEMU since we require a posix-shell like built environment
>> anyway).
>>
>> According to the info page of Make, chapter "10.3 Variables Used by
>> Implicit Rules":
>>
>> `RM'
>>      Command to remove a file; default `rm -f'.
>>
>> I've also checked it again and "make clean" works fine here (using GNU
>> Make 3.82). Which version of Make (and Linux distro) are you using?
> 
> Interesting. It fails for me with GNU Make 3.82 on my RHEL guest as
> well.
> 
>> Anyway, maybe I also simply missed something, so I'm certainly also fine
>> with the patch to revert it to "rm -f".
> 
> Given that other bios makefiles use rm -f as well, let's just change
> back until we figure out what's wrong.

I just discovered that it fails for me as well when I do "make clean"
from the top directory. So far I was only doing "make clean" after doing
a "cd pc-bios/s390-ccw" first, and that works fine. Weird. Something
seems to unset the RM variable in our build system, but I fail to find
the spot where this happens...

 Thomas

Re: [Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Thomas Huth]

On 15.08.2017 10:01, Thomas Huth wrote:
> On 15.08.2017 09:03, Cornelia Huck wrote:
>> On Tue, 15 Aug 2017 07:02:10 +0200
>> Thomas Huth <thuth@redhat.com> wrote:
>>
>>> On 14.08.2017 22:44, Eric Farman wrote:
>>>> How often does one really do a "make clean" ?  Rather infrequently,
>>>> as I only stumbled on this today.
>>>>
>>>> Perhaps I have missed the RM variable somewhere, as I see similar syntax
>>>> in some of the tests/tcg/ Makefiles, but I don't see it being set here.
>>>> My configure statement isn't terribly interesting, just enabling debug
>>>> for an s390x target, and as such there's no RM variable in its output.
>>>> I'll trust that Thomas will chime in with where it should have been.
>>>> In the meantime, this does the trick for me.  
>>>
>>> RM is one of the variables that should be pre-initialized by Make, and
>>> AFAIK should be used to increase portability (well, it's likely not
>>> important for QEMU since we require a posix-shell like built environment
>>> anyway).
>>>
>>> According to the info page of Make, chapter "10.3 Variables Used by
>>> Implicit Rules":
>>>
>>> `RM'
>>>      Command to remove a file; default `rm -f'.
>>>
>>> I've also checked it again and "make clean" works fine here (using GNU
>>> Make 3.82). Which version of Make (and Linux distro) are you using?
>>
>> Interesting. It fails for me with GNU Make 3.82 on my RHEL guest as
>> well.
>>
>>> Anyway, maybe I also simply missed something, so I'm certainly also fine
>>> with the patch to revert it to "rm -f".
>>
>> Given that other bios makefiles use rm -f as well, let's just change
>> back until we figure out what's wrong.
> 
> I just discovered that it fails for me as well when I do "make clean"
> from the top directory. So far I was only doing "make clean" after doing
> a "cd pc-bios/s390-ccw" first, and that works fine. Weird. Something
> seems to unset the RM variable in our build system, but I fail to find
> the spot where this happens...

Ok, just found it: It's this line in rules.mak:

MAKEFLAGS += -rR

The parameter -R disables the built-in variables, so RM can indeed not
work here. Sorry, I wasn't aware of that setting yet, so your patch is
indeed the right fix here (or we should maybe define RM in rules.mak, too).

 Thomas

Re: [Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Cornelia Huck]

On Tue, 15 Aug 2017 10:26:18 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 15.08.2017 10:01, Thomas Huth wrote:
> > On 15.08.2017 09:03, Cornelia Huck wrote:  
> >> On Tue, 15 Aug 2017 07:02:10 +0200
> >> Thomas Huth <thuth@redhat.com> wrote:
> >>  
> >>> On 14.08.2017 22:44, Eric Farman wrote:  
> >>>> How often does one really do a "make clean" ?  Rather infrequently,
> >>>> as I only stumbled on this today.
> >>>>
> >>>> Perhaps I have missed the RM variable somewhere, as I see similar syntax
> >>>> in some of the tests/tcg/ Makefiles, but I don't see it being set here.
> >>>> My configure statement isn't terribly interesting, just enabling debug
> >>>> for an s390x target, and as such there's no RM variable in its output.
> >>>> I'll trust that Thomas will chime in with where it should have been.
> >>>> In the meantime, this does the trick for me.    
> >>>
> >>> RM is one of the variables that should be pre-initialized by Make, and
> >>> AFAIK should be used to increase portability (well, it's likely not
> >>> important for QEMU since we require a posix-shell like built environment
> >>> anyway).
> >>>
> >>> According to the info page of Make, chapter "10.3 Variables Used by
> >>> Implicit Rules":
> >>>
> >>> `RM'
> >>>      Command to remove a file; default `rm -f'.
> >>>
> >>> I've also checked it again and "make clean" works fine here (using GNU
> >>> Make 3.82). Which version of Make (and Linux distro) are you using?  
> >>
> >> Interesting. It fails for me with GNU Make 3.82 on my RHEL guest as
> >> well.
> >>  
> >>> Anyway, maybe I also simply missed something, so I'm certainly also fine
> >>> with the patch to revert it to "rm -f".  
> >>
> >> Given that other bios makefiles use rm -f as well, let's just change
> >> back until we figure out what's wrong.  
> > 
> > I just discovered that it fails for me as well when I do "make clean"
> > from the top directory. So far I was only doing "make clean" after doing
> > a "cd pc-bios/s390-ccw" first, and that works fine. Weird. Something
> > seems to unset the RM variable in our build system, but I fail to find
> > the spot where this happens...  
> 
> Ok, just found it: It's this line in rules.mak:
> 
> MAKEFLAGS += -rR
> 
> The parameter -R disables the built-in variables, so RM can indeed not
> work here. Sorry, I wasn't aware of that setting yet, so your patch is
> indeed the right fix here (or we should maybe define RM in rules.mak, too).

The other users are all in tests/tcg/.

Would there be value in using the built-in variables generally? (Well,
value that outweighs having to go through the qemu build system...)

Re: [Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Thomas Huth]

On 15.08.2017 10:32, Cornelia Huck wrote:
> On Tue, 15 Aug 2017 10:26:18 +0200
> Thomas Huth <thuth@redhat.com> wrote:
[...]
>> The parameter -R disables the built-in variables, so RM can indeed not
>> work here. Sorry, I wasn't aware of that setting yet, so your patch is
>> indeed the right fix here (or we should maybe define RM in rules.mak, too).
> 
> The other users are all in tests/tcg/.
> 
> Would there be value in using the built-in variables generally? (Well,
> value that outweighs having to go through the qemu build system...)

There are some non-posix shells where "rm -f" does not work (e.g.
COMMAND.COM on Windows), and this is where $(RM) is really useful. But
since we require a posix-style built environment anyway, it's not that
important for the QEMU build system. OTOH, $(RM) is a standard Make
variable, so other people might step into this trap as well - so it's
maybe better to define it in rules.mak to avoid future confusion?

 Thomas

Re: [Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Peter Maydell]

On 14 August 2017 at 21:44, Eric Farman <farman@linux.vnet.ibm.com> wrote:
> How often does one really do a "make clean" ?  Rather infrequently,
> as I only stumbled on this today.

FWIW, one of my standard pre-merge build tests does a
"make clean" and then a make (mostly as a check that we
do build from clean as well as incrementally). Not sure
why it didn't catch this...

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Eric Farman]

On 08/15/2017 04:26 AM, Thomas Huth wrote:
> On 15.08.2017 10:01, Thomas Huth wrote:
>> On 15.08.2017 09:03, Cornelia Huck wrote:
>>> On Tue, 15 Aug 2017 07:02:10 +0200
>>> Thomas Huth <thuth@redhat.com> wrote:
>>>
>>>> On 14.08.2017 22:44, Eric Farman wrote:
>>>>> How often does one really do a "make clean" ?  Rather infrequently,
>>>>> as I only stumbled on this today.
>>>>>
>>>>> Perhaps I have missed the RM variable somewhere, as I see similar syntax
>>>>> in some of the tests/tcg/ Makefiles, but I don't see it being set here.
>>>>> My configure statement isn't terribly interesting, just enabling debug
>>>>> for an s390x target, and as such there's no RM variable in its output.
>>>>> I'll trust that Thomas will chime in with where it should have been.
>>>>> In the meantime, this does the trick for me.
>>>>
>>>> RM is one of the variables that should be pre-initialized by Make, and
>>>> AFAIK should be used to increase portability (well, it's likely not
>>>> important for QEMU since we require a posix-shell like built environment
>>>> anyway).
>>>>
>>>> According to the info page of Make, chapter "10.3 Variables Used by
>>>> Implicit Rules":
>>>>
>>>> `RM'
>>>>       Command to remove a file; default `rm -f'.
>>>>
>>>> I've also checked it again and "make clean" works fine here (using GNU
>>>> Make 3.82). Which version of Make (and Linux distro) are you using?

Not that it matters now, but make 4.1 on F24

>>>
>>> Interesting. It fails for me with GNU Make 3.82 on my RHEL guest as
>>> well.
>>>
>>>> Anyway, maybe I also simply missed something, so I'm certainly also fine
>>>> with the patch to revert it to "rm -f".
>>>
>>> Given that other bios makefiles use rm -f as well, let's just change
>>> back until we figure out what's wrong.
>>
>> I just discovered that it fails for me as well when I do "make clean"
>> from the top directory. So far I was only doing "make clean" after doing
>> a "cd pc-bios/s390-ccw" first, and that works fine. Weird. Something
>> seems to unset the RM variable in our build system, but I fail to find
>> the spot where this happens...
> 
> Ok, just found it: It's this line in rules.mak:
> 
> MAKEFLAGS += -rR
> 
> The parameter -R disables the built-in variables, so RM can indeed not
> work here. Sorry, I wasn't aware of that setting yet, so your patch is
> indeed the right fix here (or we should maybe define RM in rules.mak, too).

Excellent find, Thomas!  I was not aware of that either.  Thanks to both 
you and Cornelia for this.

  - Eric

Re: [Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Cornelia Huck]

On Tue, 15 Aug 2017 11:00:51 +0100
Peter Maydell <peter.maydell@linaro.org> wrote:

> On 14 August 2017 at 21:44, Eric Farman <farman@linux.vnet.ibm.com> wrote:
> > How often does one really do a "make clean" ?  Rather infrequently,
> > as I only stumbled on this today.  
> 
> FWIW, one of my standard pre-merge build tests does a
> "make clean" and then a make (mostly as a check that we
> do build from clean as well as incrementally). Not sure
> why it didn't catch this...

Probably because you don't build on s390x... I'll add it to my
workflow, though.

Re: [Qemu-devel] [PATCH for-2.10 0/1] Fix "make clean" for s390 target

[Peter Maydell]

On 15 August 2017 at 12:07, Cornelia Huck <cohuck@redhat.com> wrote:
> On Tue, 15 Aug 2017 11:00:51 +0100
> Peter Maydell <peter.maydell@linaro.org> wrote:
>
>> On 14 August 2017 at 21:44, Eric Farman <farman@linux.vnet.ibm.com> wrote:
>> > How often does one really do a "make clean" ?  Rather infrequently,
>> > as I only stumbled on this today.
>>
>> FWIW, one of my standard pre-merge build tests does a
>> "make clean" and then a make (mostly as a check that we
>> do build from clean as well as incrementally). Not sure
>> why it didn't catch this...
>
> Probably because you don't build on s390x...

I do, but not with a 'make clean' step :-)

thanks
-- PMM