[PATCH] perf tools: Move feature cleanup under tools/build

[PATCH] perf tools: Move feature cleanup under tools/build

[Jiri Olsa]

Arnaldo reported issue for following build command:

  $ rm -rf /tmp/krava; mkdir /tmp/krava; make O=/tmp/krava clean
    CLEAN    config
  /bin/sh: line 0: cd: /tmp/krava/feature/: No such file or directory
  ../../scripts/Makefile.include:17: *** output directory "/tmp/krava/feature/" does not exist.  Stop.
  make[1]: *** [Makefile.perf:1010: config-clean] Error 2
  make: *** [Makefile:90: clean] Error 2

The problem is that now that we include scripts/Makefile.include
in feature's Makefile (which is fine and needed), we need to ensure
the OUTPUT directory exists, before executing (out of tree) clean
command.

Removing the feature's cleanup from perf Makefile and fixing
feature's cleanup under build Makefile, so it now checks that
there's existing OUTPUT directory before calling the clean.

Cc: Sedat Dilek <sedat.dilek@gmail.com>
Fixes: 211a741cd3e1 ("tools: Factor Clang, LLC and LLVM utils definitions")
Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/build/Makefile     |  8 +++++++-
 tools/perf/Makefile.perf | 10 +---------
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/tools/build/Makefile b/tools/build/Makefile
index bae48e6fa995..5ed41b96fcde 100644
--- a/tools/build/Makefile
+++ b/tools/build/Makefile
@@ -30,12 +30,18 @@ build     := -f $(srctree)/tools/build/Makefile.build dir=. obj
 
 all: $(OUTPUT)fixdep
 
+# Make sure there's anything to clean,
+# feature contains check for existing OUTPUT
+TMP_O := $(if $(OUTPUT),$(OUTPUT)/feature,./)
+
 clean:
 	$(call QUIET_CLEAN, fixdep)
 	$(Q)find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
 	$(Q)rm -f $(OUTPUT)fixdep
 	$(call QUIET_CLEAN, feature-detect)
-	$(Q)$(MAKE) -C feature/ clean >/dev/null
+ifneq ($(wildcard $(TMP_O)),)
+	$(Q)$(MAKE) -C feature OUTPUT=$(TMP_O) clean >/dev/null
+endif
 
 $(OUTPUT)fixdep-in.o: FORCE
 	$(Q)$(MAKE) $(build)=fixdep
diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 5345ac70cd83..536f6f90af92 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -1001,14 +1001,6 @@ $(INSTALL_DOC_TARGETS):
 
 ### Cleaning rules
 
-#
-# This is here, not in Makefile.config, because Makefile.config does
-# not get included for the clean target:
-#
-config-clean:
-	$(call QUIET_CLEAN, config)
-	$(Q)$(MAKE) -C $(srctree)/tools/build/feature/ $(if $(OUTPUT),OUTPUT=$(OUTPUT)feature/,) clean >/dev/null
-
 python-clean:
 	$(python-clean)
 
@@ -1048,7 +1040,7 @@ endif # BUILD_BPF_SKEL
 bpf-skel-clean:
 	$(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
 
-clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBPERF)-clean config-clean fixdep-clean python-clean bpf-skel-clean
+clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean
 	$(call QUIET_CLEAN, core-objs)  $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-with-kcore $(LANG_BINDINGS)
 	$(Q)find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
 	$(Q)$(RM) $(OUTPUT).config-detected
-- 
2.29.2

Re: [PATCH] perf tools: Move feature cleanup under tools/build

[Sedat Dilek]

[-- Attachment #1: Type: text/plain, Size: 4875 bytes --]

On Wed, Feb 24, 2021 at 4:38 PM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Arnaldo reported issue for following build command:
>
>   $ rm -rf /tmp/krava; mkdir /tmp/krava; make O=/tmp/krava clean
>     CLEAN    config
>   /bin/sh: line 0: cd: /tmp/krava/feature/: No such file or directory
>   ../../scripts/Makefile.include:17: *** output directory "/tmp/krava/feature/" does not exist.  Stop.
>   make[1]: *** [Makefile.perf:1010: config-clean] Error 2
>   make: *** [Makefile:90: clean] Error 2
>
> The problem is that now that we include scripts/Makefile.include
> in feature's Makefile (which is fine and needed), we need to ensure
> the OUTPUT directory exists, before executing (out of tree) clean
> command.
>
> Removing the feature's cleanup from perf Makefile and fixing
> feature's cleanup under build Makefile, so it now checks that
> there's existing OUTPUT directory before calling the clean.
>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>

For my new friends from linux-bpf and linux-perf...

I tested your patch on top of Linux v5.11-10201-gc03c21ba6f4e.

git checkout c03c21ba6f4e -b for-5.12/perf-tools_feature_cleanup-jolsa
git branch --show-current

link="https://lore.kernel.org/r/20210224150831.409639-1-jolsa@kernel.org"
b4 am $link -o - | git am

LLVM_TOOLCHAIN_PATH="/opt/llvm-toolchain/bin"
if [ -d ${LLVM_TOOLCHAIN_PATH} ]; then
   export PATH="${LLVM_TOOLCHAIN_PATH}:${PATH}"
fi
echo $PATH

MAKE="make V=1"
MAKE_OPTS="HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang
LD=ld.lld LLVM=1 LLVM_IAS=1"
MAKE_OPTS="$MAKE_OPTS PAHOLE=/opt/pahole/bin/pahole"
echo $MAKE $MAKE_OPTS

LC_ALL=C $MAKE -j1 $MAKE_OPTS -C tools/ clean 2>&1 | tee
../make-log_tools-clean_before-perf-build.txt

LC_ALL=C PYTHON=python3 $MAKE -j4 $MAKE_OPTS -C tools/perf/ 2>&1 | tee
../make-log_tools-perf.txt

grep gcc ../make-log_tools-perf.txt
[ empty ]

LC_ALL=C $MAKE -j1 $MAKE_OPTS -C tools/ clean 2>&1 | tee
../make-log_tools-clean_after-perf-build.txt

$ clang --version
dileks clang version 13.0.0 (https://github.com/llvm/llvm-project.git
c465429f286f50e52a8d2b3b39f38344f3381cce)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /opt/llvm-toolchain/bin

Logs etc. see attached tarball.

Feel free to add:

Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM/Clang v13-git

- Sedat -

> Fixes: 211a741cd3e1 ("tools: Factor Clang, LLC and LLVM utils definitions")
> Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/build/Makefile     |  8 +++++++-
>  tools/perf/Makefile.perf | 10 +---------
>  2 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/tools/build/Makefile b/tools/build/Makefile
> index bae48e6fa995..5ed41b96fcde 100644
> --- a/tools/build/Makefile
> +++ b/tools/build/Makefile
> @@ -30,12 +30,18 @@ build     := -f $(srctree)/tools/build/Makefile.build dir=. obj
>
>  all: $(OUTPUT)fixdep
>
> +# Make sure there's anything to clean,
> +# feature contains check for existing OUTPUT
> +TMP_O := $(if $(OUTPUT),$(OUTPUT)/feature,./)
> +
>  clean:
>         $(call QUIET_CLEAN, fixdep)
>         $(Q)find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
>         $(Q)rm -f $(OUTPUT)fixdep
>         $(call QUIET_CLEAN, feature-detect)
> -       $(Q)$(MAKE) -C feature/ clean >/dev/null
> +ifneq ($(wildcard $(TMP_O)),)
> +       $(Q)$(MAKE) -C feature OUTPUT=$(TMP_O) clean >/dev/null
> +endif
>
>  $(OUTPUT)fixdep-in.o: FORCE
>         $(Q)$(MAKE) $(build)=fixdep
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 5345ac70cd83..536f6f90af92 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -1001,14 +1001,6 @@ $(INSTALL_DOC_TARGETS):
>
>  ### Cleaning rules
>
> -#
> -# This is here, not in Makefile.config, because Makefile.config does
> -# not get included for the clean target:
> -#
> -config-clean:
> -       $(call QUIET_CLEAN, config)
> -       $(Q)$(MAKE) -C $(srctree)/tools/build/feature/ $(if $(OUTPUT),OUTPUT=$(OUTPUT)feature/,) clean >/dev/null
> -
>  python-clean:
>         $(python-clean)
>
> @@ -1048,7 +1040,7 @@ endif # BUILD_BPF_SKEL
>  bpf-skel-clean:
>         $(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
>
> -clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBPERF)-clean config-clean fixdep-clean python-clean bpf-skel-clean
> +clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean
>         $(call QUIET_CLEAN, core-objs)  $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-with-kcore $(LANG_BINDINGS)
>         $(Q)find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
>         $(Q)$(RM) $(OUTPUT).config-detected
> --
> 2.29.2
>

[-- Attachment #2: for-linux-bpf.tar.zst --]
[-- Type: application/zstd, Size: 25587 bytes --]

[-- Attachment #3: for-linux-bpf.tar.zst.sha256 --]
[-- Type: application/octet-stream, Size: 88 bytes --]

c5e3d6668101cb8ec4375da3031c04fce6527b1d0c68fa1e217dfba20c386b57  for-linux-bpf.tar.zst

Re: [PATCH] perf tools: Move feature cleanup under tools/build

[Jiri Olsa]

On Thu, Feb 25, 2021 at 11:10:00AM +0100, Sedat Dilek wrote:
> On Wed, Feb 24, 2021 at 4:38 PM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Arnaldo reported issue for following build command:
> >
> >   $ rm -rf /tmp/krava; mkdir /tmp/krava; make O=/tmp/krava clean
> >     CLEAN    config
> >   /bin/sh: line 0: cd: /tmp/krava/feature/: No such file or directory
> >   ../../scripts/Makefile.include:17: *** output directory "/tmp/krava/feature/" does not exist.  Stop.
> >   make[1]: *** [Makefile.perf:1010: config-clean] Error 2
> >   make: *** [Makefile:90: clean] Error 2
> >
> > The problem is that now that we include scripts/Makefile.include
> > in feature's Makefile (which is fine and needed), we need to ensure
> > the OUTPUT directory exists, before executing (out of tree) clean
> > command.
> >
> > Removing the feature's cleanup from perf Makefile and fixing
> > feature's cleanup under build Makefile, so it now checks that
> > there's existing OUTPUT directory before calling the clean.
> >
> > Cc: Sedat Dilek <sedat.dilek@gmail.com>
> 
> For my new friends from linux-bpf and linux-perf...
> 
> I tested your patch on top of Linux v5.11-10201-gc03c21ba6f4e.
> 
> git checkout c03c21ba6f4e -b for-5.12/perf-tools_feature_cleanup-jolsa
> git branch --show-current
> 
> link="https://lore.kernel.org/r/20210224150831.409639-1-jolsa@kernel.org"
> b4 am $link -o - | git am
> 
> LLVM_TOOLCHAIN_PATH="/opt/llvm-toolchain/bin"
> if [ -d ${LLVM_TOOLCHAIN_PATH} ]; then
>    export PATH="${LLVM_TOOLCHAIN_PATH}:${PATH}"
> fi
> echo $PATH
> 
> MAKE="make V=1"
> MAKE_OPTS="HOSTCC=clang HOSTCXX=clang++ HOSTLD=ld.lld CC=clang
> LD=ld.lld LLVM=1 LLVM_IAS=1"
> MAKE_OPTS="$MAKE_OPTS PAHOLE=/opt/pahole/bin/pahole"
> echo $MAKE $MAKE_OPTS
> 
> LC_ALL=C $MAKE -j1 $MAKE_OPTS -C tools/ clean 2>&1 | tee
> ../make-log_tools-clean_before-perf-build.txt
> 
> LC_ALL=C PYTHON=python3 $MAKE -j4 $MAKE_OPTS -C tools/perf/ 2>&1 | tee
> ../make-log_tools-perf.txt
> 
> grep gcc ../make-log_tools-perf.txt
> [ empty ]
> 
> LC_ALL=C $MAKE -j1 $MAKE_OPTS -C tools/ clean 2>&1 | tee
> ../make-log_tools-clean_after-perf-build.txt
> 
> $ clang --version
> dileks clang version 13.0.0 (https://github.com/llvm/llvm-project.git
> c465429f286f50e52a8d2b3b39f38344f3381cce)
> Target: x86_64-unknown-linux-gnu
> Thread model: posix
> InstalledDir: /opt/llvm-toolchain/bin
> 
> Logs etc. see attached tarball.
> 
> Feel free to add:
> 
> Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM/Clang v13-git

great, thanks a lot

jirka

Re: [PATCH] perf tools: Move feature cleanup under tools/build

[Arnaldo Carvalho de Melo]

Em Wed, Feb 24, 2021 at 04:08:31PM +0100, Jiri Olsa escreveu:
> Arnaldo reported issue for following build command:
> 
>   $ rm -rf /tmp/krava; mkdir /tmp/krava; make O=/tmp/krava clean
>     CLEAN    config
>   /bin/sh: line 0: cd: /tmp/krava/feature/: No such file or directory
>   ../../scripts/Makefile.include:17: *** output directory "/tmp/krava/feature/" does not exist.  Stop.
>   make[1]: *** [Makefile.perf:1010: config-clean] Error 2
>   make: *** [Makefile:90: clean] Error 2
> 
> The problem is that now that we include scripts/Makefile.include
> in feature's Makefile (which is fine and needed), we need to ensure
> the OUTPUT directory exists, before executing (out of tree) clean
> command.

Thanks, tested and applied.

- Arnaldo
 
> Removing the feature's cleanup from perf Makefile and fixing
> feature's cleanup under build Makefile, so it now checks that
> there's existing OUTPUT directory before calling the clean.
> 
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Fixes: 211a741cd3e1 ("tools: Factor Clang, LLC and LLVM utils definitions")
> Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/build/Makefile     |  8 +++++++-
>  tools/perf/Makefile.perf | 10 +---------
>  2 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/build/Makefile b/tools/build/Makefile
> index bae48e6fa995..5ed41b96fcde 100644
> --- a/tools/build/Makefile
> +++ b/tools/build/Makefile
> @@ -30,12 +30,18 @@ build     := -f $(srctree)/tools/build/Makefile.build dir=. obj
>  
>  all: $(OUTPUT)fixdep
>  
> +# Make sure there's anything to clean,
> +# feature contains check for existing OUTPUT
> +TMP_O := $(if $(OUTPUT),$(OUTPUT)/feature,./)
> +
>  clean:
>  	$(call QUIET_CLEAN, fixdep)
>  	$(Q)find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
>  	$(Q)rm -f $(OUTPUT)fixdep
>  	$(call QUIET_CLEAN, feature-detect)
> -	$(Q)$(MAKE) -C feature/ clean >/dev/null
> +ifneq ($(wildcard $(TMP_O)),)
> +	$(Q)$(MAKE) -C feature OUTPUT=$(TMP_O) clean >/dev/null
> +endif
>  
>  $(OUTPUT)fixdep-in.o: FORCE
>  	$(Q)$(MAKE) $(build)=fixdep
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 5345ac70cd83..536f6f90af92 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -1001,14 +1001,6 @@ $(INSTALL_DOC_TARGETS):
>  
>  ### Cleaning rules
>  
> -#
> -# This is here, not in Makefile.config, because Makefile.config does
> -# not get included for the clean target:
> -#
> -config-clean:
> -	$(call QUIET_CLEAN, config)
> -	$(Q)$(MAKE) -C $(srctree)/tools/build/feature/ $(if $(OUTPUT),OUTPUT=$(OUTPUT)feature/,) clean >/dev/null
> -
>  python-clean:
>  	$(python-clean)
>  
> @@ -1048,7 +1040,7 @@ endif # BUILD_BPF_SKEL
>  bpf-skel-clean:
>  	$(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS)
>  
> -clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBPERF)-clean config-clean fixdep-clean python-clean bpf-skel-clean
> +clean:: $(LIBTRACEEVENT)-clean $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean $(LIBPERF)-clean fixdep-clean python-clean bpf-skel-clean
>  	$(call QUIET_CLEAN, core-objs)  $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive $(OUTPUT)perf-with-kcore $(LANG_BINDINGS)
>  	$(Q)find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
>  	$(Q)$(RM) $(OUTPUT).config-detected
> -- 
> 2.29.2
> 

-- 

- Arnaldo