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