clean after distclean gobbles source files

clean after distclean gobbles source files

[Steven Sistare]

Run 'make distclean', and GNUmakefile is removed.
But, GNUmakefile is where we cd to build/.
Run 'make distclean' or 'make clean' again, and Makefile applies
the clean actions, such as this one, at the top level of the tree:

    find . \( -name '*.so' -o -name '*.dll' -o \
          -name '*.[oda]' -o -name '*.gcno' \) -type f \
        ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-aarch64.a \
        ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-arm.a \
        -exec rm {} +

For example, it removes the .d source files in 'meson/test cases/d/*/*.d'.
The damage could be worse in the future if more suffixes are cleaned.

I don't have a suggested fix.  Recursion and the GNUmakefile bootstrap
make it non-trivial.

- Steve

Re: clean after distclean gobbles source files

[Thomas Huth]

On 07/04/2023 17.44, Steven Sistare wrote:
> Run 'make distclean', and GNUmakefile is removed.
> But, GNUmakefile is where we cd to build/.
> Run 'make distclean' or 'make clean' again, and Makefile applies
> the clean actions, such as this one, at the top level of the tree:
> 
>      find . \( -name '*.so' -o -name '*.dll' -o \
>            -name '*.[oda]' -o -name '*.gcno' \) -type f \
>          ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-aarch64.a \
>          ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-arm.a \
>          -exec rm {} +
> 
> For example, it removes the .d source files in 'meson/test cases/d/*/*.d'.
> The damage could be worse in the future if more suffixes are cleaned.
> 
> I don't have a suggested fix.  Recursion and the GNUmakefile bootstrap
> make it non-trivial.

That's somewhat ugly, indeed.

We could maybe disallow make [dist]clean if running in-tree? Something like that:

diff a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -26,7 +26,7 @@ quiet-command-run = $(if $(V),,$(if $2,printf "  %-7s %s\n" $2 $3 && ))$1
  quiet-@ = $(if $(V),,@)
  quiet-command = $(quiet-@)$(call quiet-command-run,$1,$2,$3)
  
-UNCHECKED_GOALS := %clean TAGS cscope ctags dist \
+UNCHECKED_GOALS := TAGS cscope ctags dist \
      help check-help print-% \
      docker docker-% vm-help vm-test vm-build-%
  
@@ -201,7 +201,7 @@ recurse-distclean: $(addsuffix /distclean, $(ROMS))
  
  ######################################################################
  
-clean: recurse-clean
+clean: config-host.mak recurse-clean
         -$(quiet-@)test -f build.ninja && $(NINJA) $(NINJAFLAGS) -t clean || :
         -$(quiet-@)test -f build.ninja && $(NINJA) $(NINJAFLAGS) clean-ctlist || :
         find . \( -name '*.so' -o -name '*.dll' -o \


... or if we still want to allow that, maybe just make an exception for the *.d files:

diff --git a/Makefile b/Makefile
index e421f8a1f4..0cb2a7aa98 100644
--- a/Makefile
+++ b/Makefile
@@ -208,6 +208,7 @@ clean: recurse-clean
                   -name '*.[oda]' -o -name '*.gcno' \) -type f \
                 ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-aarch64.a \
                 ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-arm.a \
+               ! -path './meson/test cases/d/*/*.d' \
                 -exec rm {} +
         rm -f TAGS cscope.* *~ */*~
  

What do you think?

  Thomas

Re: clean after distclean gobbles source files

[Steven Sistare]

On 4/13/2023 7:41 AM, Thomas Huth wrote:
> On 07/04/2023 17.44, Steven Sistare wrote:
>> Run 'make distclean', and GNUmakefile is removed.
>> But, GNUmakefile is where we cd to build/.
>> Run 'make distclean' or 'make clean' again, and Makefile applies
>> the clean actions, such as this one, at the top level of the tree:
>>
>>      find . \( -name '*.so' -o -name '*.dll' -o \
>>            -name '*.[oda]' -o -name '*.gcno' \) -type f \
>>          ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-aarch64.a \
>>          ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-arm.a \
>>          -exec rm {} +
>>
>> For example, it removes the .d source files in 'meson/test cases/d/*/*.d'.
>> The damage could be worse in the future if more suffixes are cleaned.
>>
>> I don't have a suggested fix.  Recursion and the GNUmakefile bootstrap
>> make it non-trivial.
> 
> That's somewhat ugly, indeed.
> 
> We could maybe disallow make [dist]clean if running in-tree? Something like that:
> 
> diff a/Makefile b/Makefile
> --- a/Makefile
> +++ b/Makefile
> @@ -26,7 +26,7 @@ quiet-command-run = $(if $(V),,$(if $2,printf "  %-7s %s\n" $2 $3 && ))$1
>  quiet-@ = $(if $(V),,@)
>  quiet-command = $(quiet-@)$(call quiet-command-run,$1,$2,$3)
>  
> -UNCHECKED_GOALS := %clean TAGS cscope ctags dist \
> +UNCHECKED_GOALS := TAGS cscope ctags dist \
>      help check-help print-% \
>      docker docker-% vm-help vm-test vm-build-%
>  
> @@ -201,7 +201,7 @@ recurse-distclean: $(addsuffix /distclean, $(ROMS))
>  
>  ######################################################################
>  
> -clean: recurse-clean
> +clean: config-host.mak recurse-clean
>         -$(quiet-@)test -f build.ninja && $(NINJA) $(NINJAFLAGS) -t clean || :
>         -$(quiet-@)test -f build.ninja && $(NINJA) $(NINJAFLAGS) clean-ctlist || :
>         find . \( -name '*.so' -o -name '*.dll' -o \
> 
> 
> ... or if we still want to allow that, maybe just make an exception for the *.d files:
> 
> diff --git a/Makefile b/Makefile
> index e421f8a1f4..0cb2a7aa98 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -208,6 +208,7 @@ clean: recurse-clean
>                   -name '*.[oda]' -o -name '*.gcno' \) -type f \
>                 ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-aarch64.a \
>                 ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-arm.a \
> +               ! -path './meson/test cases/d/*/*.d' \
>                 -exec rm {} +
>         rm -f TAGS cscope.* *~ */*~
>  
> 
> What do you think?

Actually, all make targets are broken if we do not cd to build first.

This should do the trick.  If you agree, I will submit a patch.

diff --git a/Makefile b/Makefile
index a48103c..3d03101 100644
--- a/Makefile
+++ b/Makefile
@@ -4,6 +4,10 @@ ifneq ($(words $(subst :, ,$(CURDIR))), 1)
   $(error main directory cannot contain spaces nor colons)
 endif

+ifneq ($(notdir $(CURDIR)),build)
+$(error To build in tree, run configure first.)
+endif
+
 # Always point to the root of the build tree (needs GNU make).
 BUILD_DIR=$(CURDIR)

- Steve

Re: clean after distclean gobbles source files

[Thomas Huth]

On 14/04/2023 17.30, Steven Sistare wrote:
> On 4/13/2023 7:41 AM, Thomas Huth wrote:
>> On 07/04/2023 17.44, Steven Sistare wrote:
>>> Run 'make distclean', and GNUmakefile is removed.
>>> But, GNUmakefile is where we cd to build/.
>>> Run 'make distclean' or 'make clean' again, and Makefile applies
>>> the clean actions, such as this one, at the top level of the tree:
>>>
>>>       find . \( -name '*.so' -o -name '*.dll' -o \
>>>             -name '*.[oda]' -o -name '*.gcno' \) -type f \
>>>           ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-aarch64.a \
>>>           ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-arm.a \
>>>           -exec rm {} +
>>>
>>> For example, it removes the .d source files in 'meson/test cases/d/*/*.d'.
>>> The damage could be worse in the future if more suffixes are cleaned.
>>>
>>> I don't have a suggested fix.  Recursion and the GNUmakefile bootstrap
>>> make it non-trivial.
>>
>> That's somewhat ugly, indeed.
>>
>> We could maybe disallow make [dist]clean if running in-tree? Something like that:
>>
>> diff a/Makefile b/Makefile
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -26,7 +26,7 @@ quiet-command-run = $(if $(V),,$(if $2,printf "  %-7s %s\n" $2 $3 && ))$1
>>   quiet-@ = $(if $(V),,@)
>>   quiet-command = $(quiet-@)$(call quiet-command-run,$1,$2,$3)
>>   
>> -UNCHECKED_GOALS := %clean TAGS cscope ctags dist \
>> +UNCHECKED_GOALS := TAGS cscope ctags dist \
>>       help check-help print-% \
>>       docker docker-% vm-help vm-test vm-build-%
>>   
>> @@ -201,7 +201,7 @@ recurse-distclean: $(addsuffix /distclean, $(ROMS))
>>   
>>   ######################################################################
>>   
>> -clean: recurse-clean
>> +clean: config-host.mak recurse-clean
>>          -$(quiet-@)test -f build.ninja && $(NINJA) $(NINJAFLAGS) -t clean || :
>>          -$(quiet-@)test -f build.ninja && $(NINJA) $(NINJAFLAGS) clean-ctlist || :
>>          find . \( -name '*.so' -o -name '*.dll' -o \
>>
>>
>> ... or if we still want to allow that, maybe just make an exception for the *.d files:
>>
>> diff --git a/Makefile b/Makefile
>> index e421f8a1f4..0cb2a7aa98 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -208,6 +208,7 @@ clean: recurse-clean
>>                    -name '*.[oda]' -o -name '*.gcno' \) -type f \
>>                  ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-aarch64.a \
>>                  ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-arm.a \
>> +               ! -path './meson/test cases/d/*/*.d' \
>>                  -exec rm {} +
>>          rm -f TAGS cscope.* *~ */*~
>>   
>>
>> What do you think?
> 
> Actually, all make targets are broken if we do not cd to build first.

I think some of them work from the source directory, too... e.g. "make help" 
or "make vm-build-XXX" or "make dist" ... not sure how important this 
possibility is ... I guess "make dist" is still a thing? Michael?

> This should do the trick.  If you agree, I will submit a patch.
> 
> diff --git a/Makefile b/Makefile
> index a48103c..3d03101 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -4,6 +4,10 @@ ifneq ($(words $(subst :, ,$(CURDIR))), 1)
>     $(error main directory cannot contain spaces nor colons)
>   endif
> 
> +ifneq ($(notdir $(CURDIR)),build)
> +$(error To build in tree, run configure first.)
> +endif

If we decide to go down that road, I think you should remove the existing 
"Please call configure before running make" UNCHECKED_GOALS logic in that 
file, too.

  Thomas

Re: clean after distclean gobbles source files

[Steven Sistare]

On 4/14/2023 11:41 AM, Thomas Huth wrote:
> On 14/04/2023 17.30, Steven Sistare wrote:
>> On 4/13/2023 7:41 AM, Thomas Huth wrote:
>>> On 07/04/2023 17.44, Steven Sistare wrote:
>>>> Run 'make distclean', and GNUmakefile is removed.
>>>> But, GNUmakefile is where we cd to build/.
>>>> Run 'make distclean' or 'make clean' again, and Makefile applies
>>>> the clean actions, such as this one, at the top level of the tree:
>>>>
>>>>       find . \( -name '*.so' -o -name '*.dll' -o \
>>>>             -name '*.[oda]' -o -name '*.gcno' \) -type f \
>>>>           ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-aarch64.a \
>>>>           ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-arm.a \
>>>>           -exec rm {} +
>>>>
>>>> For example, it removes the .d source files in 'meson/test cases/d/*/*.d'.
>>>> The damage could be worse in the future if more suffixes are cleaned.
>>>>
>>>> I don't have a suggested fix.  Recursion and the GNUmakefile bootstrap
>>>> make it non-trivial.
>>>
>>> That's somewhat ugly, indeed.
>>>
>>> We could maybe disallow make [dist]clean if running in-tree? Something like that:
>>>
>>> diff a/Makefile b/Makefile
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -26,7 +26,7 @@ quiet-command-run = $(if $(V),,$(if $2,printf "  %-7s %s\n" $2 $3 && ))$1
>>>   quiet-@ = $(if $(V),,@)
>>>   quiet-command = $(quiet-@)$(call quiet-command-run,$1,$2,$3)
>>>   -UNCHECKED_GOALS := %clean TAGS cscope ctags dist \
>>> +UNCHECKED_GOALS := TAGS cscope ctags dist \
>>>       help check-help print-% \
>>>       docker docker-% vm-help vm-test vm-build-%
>>>   @@ -201,7 +201,7 @@ recurse-distclean: $(addsuffix /distclean, $(ROMS))
>>>     ######################################################################
>>>   -clean: recurse-clean
>>> +clean: config-host.mak recurse-clean
>>>          -$(quiet-@)test -f build.ninja && $(NINJA) $(NINJAFLAGS) -t clean || :
>>>          -$(quiet-@)test -f build.ninja && $(NINJA) $(NINJAFLAGS) clean-ctlist || :
>>>          find . \( -name '*.so' -o -name '*.dll' -o \
>>>
>>>
>>> ... or if we still want to allow that, maybe just make an exception for the *.d files:
>>>
>>> diff --git a/Makefile b/Makefile
>>> index e421f8a1f4..0cb2a7aa98 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -208,6 +208,7 @@ clean: recurse-clean
>>>                    -name '*.[oda]' -o -name '*.gcno' \) -type f \
>>>                  ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-aarch64.a \
>>>                  ! -path ./roms/edk2/ArmPkg/Library/GccLto/liblto-arm.a \
>>> +               ! -path './meson/test cases/d/*/*.d' \
>>>                  -exec rm {} +
>>>          rm -f TAGS cscope.* *~ */*~
>>>  
>>> What do you think?
>>
>> Actually, all make targets are broken if we do not cd to build first.
> 
> I think some of them work from the source directory, too... e.g. "make help" or "make vm-build-XXX" or "make dist" ... not sure how important this possibility is ... I guess "make dist" is still a thing? Michael?
> 
>> This should do the trick.  If you agree, I will submit a patch.
>>
>> diff --git a/Makefile b/Makefile
>> index a48103c..3d03101 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -4,6 +4,10 @@ ifneq ($(words $(subst :, ,$(CURDIR))), 1)
>>     $(error main directory cannot contain spaces nor colons)
>>   endif
>>
>> +ifneq ($(notdir $(CURDIR)),build)
>> +$(error To build in tree, run configure first.)
>> +endif
> 
> If we decide to go down that road, I think you should remove the existing "Please call configure before running make" UNCHECKED_GOALS logic in that file, too.

OK, UNCHECKED_GOALS is supposed to handle this situation.  I will post a patch that does so.

- Steve