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