Re: clean after distclean gobbles source files

[Thomas Huth <thuth@redhat.com>]

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