[PATCH] perf build: fix in-tree build

[PATCH] perf build: fix in-tree build

[Luca Ceresoli]

Building perf in-tree is broken after commit 890a1961c812 ("perf tools:
Create source symlink in perf object dir") which added a 'source' symlink
in the output dir pointing to the source dir.

With in-tree builds, the added 'SOURCE = ...' line is executed multiple
times (I observed 2 during the build plus 2 during installation). This is a
minor inefficiency, in theory not harmful because symlink creation is
assumed to be idempotent. But it is not.

Considering with in-tree builds:

  srctree=/absolute/path/to/linux
   OUTPUT=/absolute/path/to/linux/tools/perf

here's what happens:

 1. ln -sf $(srctree)/tools/perf $(OUTPUT)/source
    -> creates /absolute/path/to/linux/tools/perf/source
       link to /absolute/path/to/linux/tools/perf
    => OK, that's what was intended
 2. ln -sf $(srctree)/tools/perf $(OUTPUT)/source   # same command as 1
    -> creates /absolute/path/to/linux/tools/perf/perf
       link to /absolute/path/to/linux/tools/perf
    => Not what was intended, not idempotent
 3. Now the build _should_ create the 'perf' executable, but it fails

The reason is the tricky 'ln' command line. At the first invocation 'ln'
uses the 1st form:

       ln [OPTION]... [-T] TARGET LINK_NAME

and creates a link to TARGET *called LINK_NAME*.

At the second invocation $(OUTPUT)/source exists, so 'ln' uses the 3rd
form:

       ln [OPTION]... TARGET... DIRECTORY

and creates a link to TARGET *called TARGET* inside DIRECTORY.

Fix by adding --no-dereference to "treat LINK_NAME as a normal file if it
is a symbolic link to a directory", as the manpage says.

Link: https://lore.kernel.org/all/20241125182506.38af9907@booty/
Fixes: 890a1961c812 ("perf tools: Create source symlink in perf object dir")
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 tools/perf/Makefile.perf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index d74241a151313bd09101aabb5d765a5a0a6efc84..bbd799a0fd544db220f29d1e250a819a765d04f3 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -164,7 +164,7 @@ ifneq ($(OUTPUT),)
 VPATH += $(OUTPUT)
 export VPATH
 # create symlink to the original source
-SOURCE := $(shell ln -sf $(srctree)/tools/perf $(OUTPUT)/source)
+SOURCE := $(shell ln -sf --no-dereference $(srctree)/tools/perf $(OUTPUT)/source)
 endif
 
 ifeq ($(V),1)

---
base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
change-id: 20250124-perf-fix-intree-build-fbd97f560254

Best regards,
-- 
Luca Ceresoli <luca.ceresoli@bootlin.com>

Re: [PATCH] perf build: fix in-tree build

[Namhyung Kim]

Hello,

On Fri, Jan 24, 2025 at 02:06:08PM +0100, Luca Ceresoli wrote:
> Building perf in-tree is broken after commit 890a1961c812 ("perf tools:
> Create source symlink in perf object dir") which added a 'source' symlink
> in the output dir pointing to the source dir.

I cannot reproduce it - both `make -C tools/perf` and `cd tools/perf; make`
work well for me.  What do you mean by in-tree build exactly?  Can you
please share your command line and the error messages?

Thanks,
Namhyung

> 
> With in-tree builds, the added 'SOURCE = ...' line is executed multiple
> times (I observed 2 during the build plus 2 during installation). This is a
> minor inefficiency, in theory not harmful because symlink creation is
> assumed to be idempotent. But it is not.
> 
> Considering with in-tree builds:
> 
>   srctree=/absolute/path/to/linux
>    OUTPUT=/absolute/path/to/linux/tools/perf
> 
> here's what happens:
> 
>  1. ln -sf $(srctree)/tools/perf $(OUTPUT)/source
>     -> creates /absolute/path/to/linux/tools/perf/source
>        link to /absolute/path/to/linux/tools/perf
>     => OK, that's what was intended
>  2. ln -sf $(srctree)/tools/perf $(OUTPUT)/source   # same command as 1
>     -> creates /absolute/path/to/linux/tools/perf/perf
>        link to /absolute/path/to/linux/tools/perf
>     => Not what was intended, not idempotent
>  3. Now the build _should_ create the 'perf' executable, but it fails
> 
> The reason is the tricky 'ln' command line. At the first invocation 'ln'
> uses the 1st form:
> 
>        ln [OPTION]... [-T] TARGET LINK_NAME
> 
> and creates a link to TARGET *called LINK_NAME*.
> 
> At the second invocation $(OUTPUT)/source exists, so 'ln' uses the 3rd
> form:
> 
>        ln [OPTION]... TARGET... DIRECTORY
> 
> and creates a link to TARGET *called TARGET* inside DIRECTORY.
> 
> Fix by adding --no-dereference to "treat LINK_NAME as a normal file if it
> is a symbolic link to a directory", as the manpage says.
> 
> Link: https://lore.kernel.org/all/20241125182506.38af9907@booty/
> Fixes: 890a1961c812 ("perf tools: Create source symlink in perf object dir")
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  tools/perf/Makefile.perf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index d74241a151313bd09101aabb5d765a5a0a6efc84..bbd799a0fd544db220f29d1e250a819a765d04f3 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -164,7 +164,7 @@ ifneq ($(OUTPUT),)
>  VPATH += $(OUTPUT)
>  export VPATH
>  # create symlink to the original source
> -SOURCE := $(shell ln -sf $(srctree)/tools/perf $(OUTPUT)/source)
> +SOURCE := $(shell ln -sf --no-dereference $(srctree)/tools/perf $(OUTPUT)/source)
>  endif
>  
>  ifeq ($(V),1)
> 
> ---
> base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
> change-id: 20250124-perf-fix-intree-build-fbd97f560254
> 
> Best regards,
> -- 
> Luca Ceresoli <luca.ceresoli@bootlin.com>
> 

Re: [PATCH] perf build: fix in-tree build

[Luca Ceresoli]

Hello Namhyung Kim, 

thanks for having a look!

On Sun, 26 Jan 2025 13:00:10 -0800
Namhyung Kim <namhyung@kernel.org> wrote:

> Hello,
> 
> On Fri, Jan 24, 2025 at 02:06:08PM +0100, Luca Ceresoli wrote:
> > Building perf in-tree is broken after commit 890a1961c812 ("perf tools:
> > Create source symlink in perf object dir") which added a 'source' symlink
> > in the output dir pointing to the source dir.  
> 
> I cannot reproduce it - both `make -C tools/perf` and `cd tools/perf; make`
> work well for me.  What do you mean by in-tree build exactly?  Can you
> please share your command line and the error messages?

I have narrowed down my reproducer script from the initial report to a
fairly minimal one, and here it is:

------------------------8<------------------------
#!/bin/sh

set -eu

OUT_DIR=${1:-$(pwd)/tools/perf}
DESTDIR=/tmp/aaa

RET=0

echo "Kernel version: $(git describe)"
echo "OUT_DIR = ${OUT_DIR}"
echo "DESTDIR = ${DESTDIR}"
echo

git clean -xdfq
rm -fr ${DESTDIR}/

# Only for out of tree builds: clear the build dir
if ! echo ${OUT_DIR} | grep -q "tools/perf"; then
    rm -fr ${OUT_DIR}
    mkdir -p ${OUT_DIR}
fi

LINUX_MAKE_FLAGS="\
    -C tools/perf \
    O=${OUT_DIR} \
    DESTDIR=${DESTDIR} \
"

make ${LINUX_MAKE_FLAGS}
make ${LINUX_MAKE_FLAGS} install

echo
    
if [ -h ${OUT_DIR}/perf ]; then
    echo "*** ERROR: ${OUT_DIR}/perf is a symlink, should be an exectuable file: ***" >&2
    ls -l ${OUT_DIR}/perf
    RET=255
fi

if ! find ${DESTDIR}/ -name perf -not -type d | xargs file | grep 'ELF.*executable'; then
    echo "*** ERROR: perf executable not preseint in install dir ${DESTDIR} ***" >&2
    RET=255
fi

exit $RET
------------------------8<------------------------

Just put it outside of your kernel dir (or it will be removed by the
'git clean' command) and run it in your kernel source dir. E.g. right
now I'm doing:

cd <kernel dir>
git checkout v6.13
../build-perf

You should see it failing as:

*** ERROR: /.../linux/tools/perf/perf is a symlink, should be an exectuable file: ***
lrwxrwxrwx 1 user user 39 Jan 27 14:10 /.../linux/tools/perf/perf -> /.../linux/tools/perf
*** ERROR: perf executable not preseint in install dir /tmp/aaa ***

Note that in the broken case the kernel build continues and returns 0,
but there is no perf exectuable.

PS:

And after having come up with the above script, I also found that the
build succeeds with the following change:

------------------------8<------------------------
@@ -24,11 +24,10 @@
 LINUX_MAKE_FLAGS="\
     -C tools/perf \
     O=${OUT_DIR} \
-    DESTDIR=${DESTDIR} \
 "
 
 make ${LINUX_MAKE_FLAGS}
-make ${LINUX_MAKE_FLAGS} install
+make ${LINUX_MAKE_FLAGS} DESTDIR=${DESTDIR} install
------------------------8<------------------------

In other words:

 * This fails:
     make ${LINUX_MAKE_FLAGS} DESTDIR=${DESTDIR}
     make ${LINUX_MAKE_FLAGS} DESTDIR=${DESTDIR} install
 * This succeeds:
     make ${LINUX_MAKE_FLAGS}
     make ${LINUX_MAKE_FLAGS} DESTDIR=${DESTDIR} install

The only difference is the presence of 'DESTDIR' in the compile command.

My understanding and expectation is that DESTDIR has no effect on the
make process except in the 'install' target. This is clearly not
happening here.

I have not yet found an obvious reason why 'DESTDIR' is special in the
compilation stage, after a quick look.

Best regards,
Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] perf build: fix in-tree build

[Namhyung Kim]

On Mon, Jan 27, 2025 at 02:48:49PM +0100, Luca Ceresoli wrote:
> Hello Namhyung Kim, 
> 
> thanks for having a look!
> 
> On Sun, 26 Jan 2025 13:00:10 -0800
> Namhyung Kim <namhyung@kernel.org> wrote:
> 
> > Hello,
> > 
> > On Fri, Jan 24, 2025 at 02:06:08PM +0100, Luca Ceresoli wrote:
> > > Building perf in-tree is broken after commit 890a1961c812 ("perf tools:
> > > Create source symlink in perf object dir") which added a 'source' symlink
> > > in the output dir pointing to the source dir.  
> > 
> > I cannot reproduce it - both `make -C tools/perf` and `cd tools/perf; make`
> > work well for me.  What do you mean by in-tree build exactly?  Can you
> > please share your command line and the error messages?
> 
> I have narrowed down my reproducer script from the initial report to a
> fairly minimal one, and here it is:
> 
> ------------------------8<------------------------
> #!/bin/sh
> 
> set -eu
> 
> OUT_DIR=${1:-$(pwd)/tools/perf}
> DESTDIR=/tmp/aaa
> 
> RET=0
> 
> echo "Kernel version: $(git describe)"
> echo "OUT_DIR = ${OUT_DIR}"
> echo "DESTDIR = ${DESTDIR}"
> echo
> 
> git clean -xdfq
> rm -fr ${DESTDIR}/
> 
> # Only for out of tree builds: clear the build dir
> if ! echo ${OUT_DIR} | grep -q "tools/perf"; then
>     rm -fr ${OUT_DIR}
>     mkdir -p ${OUT_DIR}
> fi
> 
> LINUX_MAKE_FLAGS="\
>     -C tools/perf \
>     O=${OUT_DIR} \
>     DESTDIR=${DESTDIR} \
> "
> 
> make ${LINUX_MAKE_FLAGS}
> make ${LINUX_MAKE_FLAGS} install
> 
> echo
>     
> if [ -h ${OUT_DIR}/perf ]; then
>     echo "*** ERROR: ${OUT_DIR}/perf is a symlink, should be an exectuable file: ***" >&2
>     ls -l ${OUT_DIR}/perf
>     RET=255
> fi
> 
> if ! find ${DESTDIR}/ -name perf -not -type d | xargs file | grep 'ELF.*executable'; then
>     echo "*** ERROR: perf executable not preseint in install dir ${DESTDIR} ***" >&2
>     RET=255
> fi
> 
> exit $RET
> ------------------------8<------------------------
> 
> Just put it outside of your kernel dir (or it will be removed by the
> 'git clean' command) and run it in your kernel source dir. E.g. right
> now I'm doing:
> 
> cd <kernel dir>
> git checkout v6.13
> ../build-perf
> 
> You should see it failing as:
> 
> *** ERROR: /.../linux/tools/perf/perf is a symlink, should be an exectuable file: ***
> lrwxrwxrwx 1 user user 39 Jan 27 14:10 /.../linux/tools/perf/perf -> /.../linux/tools/perf
> *** ERROR: perf executable not preseint in install dir /tmp/aaa ***
> 
> Note that in the broken case the kernel build continues and returns 0,
> but there is no perf exectuable.

Thanks for sharing this.  I've reproduced it and found it had a
symlink to a directory rather than an executable.

> 
> PS:
> 
> And after having come up with the above script, I also found that the
> build succeeds with the following change:
> 
> ------------------------8<------------------------
> @@ -24,11 +24,10 @@
>  LINUX_MAKE_FLAGS="\
>      -C tools/perf \
>      O=${OUT_DIR} \
> -    DESTDIR=${DESTDIR} \
>  "
>  
>  make ${LINUX_MAKE_FLAGS}
> -make ${LINUX_MAKE_FLAGS} install
> +make ${LINUX_MAKE_FLAGS} DESTDIR=${DESTDIR} install
> ------------------------8<------------------------
> 
> In other words:
> 
>  * This fails:
>      make ${LINUX_MAKE_FLAGS} DESTDIR=${DESTDIR}
>      make ${LINUX_MAKE_FLAGS} DESTDIR=${DESTDIR} install
>  * This succeeds:
>      make ${LINUX_MAKE_FLAGS}
>      make ${LINUX_MAKE_FLAGS} DESTDIR=${DESTDIR} install
> 
> The only difference is the presence of 'DESTDIR' in the compile command.
> 
> My understanding and expectation is that DESTDIR has no effect on the
> make process except in the 'install' target. This is clearly not
> happening here.
> 
> I have not yet found an obvious reason why 'DESTDIR' is special in the
> compilation stage, after a quick look.

Interesting, maybe some install commands were called internally.  I'll
take a look.

Thanks,
Namhyung

Re: [PATCH] perf build: fix in-tree build

[Charlie Jenkins]

On Tue, Jan 28, 2025 at 10:31:14AM -0800, Namhyung Kim wrote:
> On Mon, Jan 27, 2025 at 02:48:49PM +0100, Luca Ceresoli wrote:
> > Hello Namhyung Kim, 
> > 
> > thanks for having a look!
> > 
> > On Sun, 26 Jan 2025 13:00:10 -0800
> > Namhyung Kim <namhyung@kernel.org> wrote:
> > 
> > > Hello,
> > > 
> > > On Fri, Jan 24, 2025 at 02:06:08PM +0100, Luca Ceresoli wrote:
> > > > Building perf in-tree is broken after commit 890a1961c812 ("perf tools:
> > > > Create source symlink in perf object dir") which added a 'source' symlink
> > > > in the output dir pointing to the source dir.  
> > > 
> > > I cannot reproduce it - both `make -C tools/perf` and `cd tools/perf; make`
> > > work well for me.  What do you mean by in-tree build exactly?  Can you
> > > please share your command line and the error messages?
> > 
> > I have narrowed down my reproducer script from the initial report to a
> > fairly minimal one, and here it is:
> > 
> > ------------------------8<------------------------
> > #!/bin/sh
> > 
> > set -eu
> > 
> > OUT_DIR=${1:-$(pwd)/tools/perf}
> > DESTDIR=/tmp/aaa
> > 
> > RET=0
> > 
> > echo "Kernel version: $(git describe)"
> > echo "OUT_DIR = ${OUT_DIR}"
> > echo "DESTDIR = ${DESTDIR}"
> > echo
> > 
> > git clean -xdfq
> > rm -fr ${DESTDIR}/
> > 
> > # Only for out of tree builds: clear the build dir
> > if ! echo ${OUT_DIR} | grep -q "tools/perf"; then
> >     rm -fr ${OUT_DIR}
> >     mkdir -p ${OUT_DIR}
> > fi
> > 
> > LINUX_MAKE_FLAGS="\
> >     -C tools/perf \
> >     O=${OUT_DIR} \
> >     DESTDIR=${DESTDIR} \
> > "
> > 
> > make ${LINUX_MAKE_FLAGS}
> > make ${LINUX_MAKE_FLAGS} install
> > 
> > echo
> >     
> > if [ -h ${OUT_DIR}/perf ]; then
> >     echo "*** ERROR: ${OUT_DIR}/perf is a symlink, should be an exectuable file: ***" >&2
> >     ls -l ${OUT_DIR}/perf
> >     RET=255
> > fi
> > 
> > if ! find ${DESTDIR}/ -name perf -not -type d | xargs file | grep 'ELF.*executable'; then
> >     echo "*** ERROR: perf executable not preseint in install dir ${DESTDIR} ***" >&2
> >     RET=255
> > fi
> > 
> > exit $RET
> > ------------------------8<------------------------
> > 
> > Just put it outside of your kernel dir (or it will be removed by the
> > 'git clean' command) and run it in your kernel source dir. E.g. right
> > now I'm doing:
> > 
> > cd <kernel dir>
> > git checkout v6.13
> > ../build-perf
> > 
> > You should see it failing as:
> > 
> > *** ERROR: /.../linux/tools/perf/perf is a symlink, should be an exectuable file: ***
> > lrwxrwxrwx 1 user user 39 Jan 27 14:10 /.../linux/tools/perf/perf -> /.../linux/tools/perf
> > *** ERROR: perf executable not preseint in install dir /tmp/aaa ***
> > 
> > Note that in the broken case the kernel build continues and returns 0,
> > but there is no perf exectuable.
> 
> Thanks for sharing this.  I've reproduced it and found it had a
> symlink to a directory rather than an executable.
> 
> > 
> > PS:
> > 
> > And after having come up with the above script, I also found that the
> > build succeeds with the following change:
> > 
> > ------------------------8<------------------------
> > @@ -24,11 +24,10 @@
> >  LINUX_MAKE_FLAGS="\
> >      -C tools/perf \
> >      O=${OUT_DIR} \
> > -    DESTDIR=${DESTDIR} \
> >  "
> >  
> >  make ${LINUX_MAKE_FLAGS}
> > -make ${LINUX_MAKE_FLAGS} install
> > +make ${LINUX_MAKE_FLAGS} DESTDIR=${DESTDIR} install
> > ------------------------8<------------------------
> > 
> > In other words:
> > 
> >  * This fails:
> >      make ${LINUX_MAKE_FLAGS} DESTDIR=${DESTDIR}
> >      make ${LINUX_MAKE_FLAGS} DESTDIR=${DESTDIR} install
> >  * This succeeds:
> >      make ${LINUX_MAKE_FLAGS}
> >      make ${LINUX_MAKE_FLAGS} DESTDIR=${DESTDIR} install
> > 
> > The only difference is the presence of 'DESTDIR' in the compile command.
> > 
> > My understanding and expectation is that DESTDIR has no effect on the
> > make process except in the 'install' target. This is clearly not
> > happening here.
> > 
> > I have not yet found an obvious reason why 'DESTDIR' is special in the
> > compilation stage, after a quick look.
> 
> Interesting, maybe some install commands were called internally.  I'll
> take a look.
> 
> Thanks,
> Namhyung
> 

I just stumbled across this as well. It is breaking Buildroot. Buildroot
has this in the Makefile that triggers this bug:

	$(TARGET_MAKE_ENV) $(MAKE1) $(PERF_MAKE_FLAGS) \
		-C $(LINUX_DIR)/tools/perf O=$(LINUX_DIR)/tools/perf/ install

Having the output equal the srcdir causes this overwritting. I sent
pretty much the same patch here [1] before double-checking that somebody
hadn't posted this fix already.

- Charlie

[1] https://lore.kernel.org/lkml/Z8Ixn3J2hw8TLdRj@ghost/

Re: [PATCH] perf build: fix in-tree build

[Charlie Jenkins]

On Fri, Jan 24, 2025 at 02:06:08PM +0100, Luca Ceresoli wrote:
> Building perf in-tree is broken after commit 890a1961c812 ("perf tools:
> Create source symlink in perf object dir") which added a 'source' symlink
> in the output dir pointing to the source dir.
> 
> With in-tree builds, the added 'SOURCE = ...' line is executed multiple
> times (I observed 2 during the build plus 2 during installation). This is a
> minor inefficiency, in theory not harmful because symlink creation is
> assumed to be idempotent. But it is not.
> 
> Considering with in-tree builds:
> 
>   srctree=/absolute/path/to/linux
>    OUTPUT=/absolute/path/to/linux/tools/perf
> 
> here's what happens:
> 
>  1. ln -sf $(srctree)/tools/perf $(OUTPUT)/source
>     -> creates /absolute/path/to/linux/tools/perf/source
>        link to /absolute/path/to/linux/tools/perf
>     => OK, that's what was intended
>  2. ln -sf $(srctree)/tools/perf $(OUTPUT)/source   # same command as 1
>     -> creates /absolute/path/to/linux/tools/perf/perf
>        link to /absolute/path/to/linux/tools/perf
>     => Not what was intended, not idempotent
>  3. Now the build _should_ create the 'perf' executable, but it fails
> 
> The reason is the tricky 'ln' command line. At the first invocation 'ln'
> uses the 1st form:
> 
>        ln [OPTION]... [-T] TARGET LINK_NAME
> 
> and creates a link to TARGET *called LINK_NAME*.
> 
> At the second invocation $(OUTPUT)/source exists, so 'ln' uses the 3rd
> form:
> 
>        ln [OPTION]... TARGET... DIRECTORY
> 
> and creates a link to TARGET *called TARGET* inside DIRECTORY.
> 
> Fix by adding --no-dereference to "treat LINK_NAME as a normal file if it
> is a symbolic link to a directory", as the manpage says.
> 
> Link: https://lore.kernel.org/all/20241125182506.38af9907@booty/
> Fixes: 890a1961c812 ("perf tools: Create source symlink in perf object dir")
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  tools/perf/Makefile.perf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index d74241a151313bd09101aabb5d765a5a0a6efc84..bbd799a0fd544db220f29d1e250a819a765d04f3 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -164,7 +164,7 @@ ifneq ($(OUTPUT),)
>  VPATH += $(OUTPUT)
>  export VPATH
>  # create symlink to the original source
> -SOURCE := $(shell ln -sf $(srctree)/tools/perf $(OUTPUT)/source)
> +SOURCE := $(shell ln -sf --no-dereference $(srctree)/tools/perf $(OUTPUT)/source)

The kernel Makefile has:
	$(Q)ln -fsn $(srcroot) source

So for parity the --no-dereference could become `n`, but it doesn't
really matter.

>  endif
>  
>  ifeq ($(V),1)
> 
> ---
> base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
> change-id: 20250124-perf-fix-intree-build-fbd97f560254
> 
> Best regards,
> -- 
> Luca Ceresoli <luca.ceresoli@bootlin.com>
> 

Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
Tested-by: Charlie Jenkins <charlie@rivosinc.com>

Re: [PATCH] perf build: fix in-tree build

[Namhyung Kim]

Hello,

On Fri, Feb 28, 2025 at 02:08:24PM -0800, Charlie Jenkins wrote:
> On Fri, Jan 24, 2025 at 02:06:08PM +0100, Luca Ceresoli wrote:
> > Building perf in-tree is broken after commit 890a1961c812 ("perf tools:
> > Create source symlink in perf object dir") which added a 'source' symlink
> > in the output dir pointing to the source dir.
> > 
> > With in-tree builds, the added 'SOURCE = ...' line is executed multiple
> > times (I observed 2 during the build plus 2 during installation). This is a
> > minor inefficiency, in theory not harmful because symlink creation is
> > assumed to be idempotent. But it is not.
> > 
> > Considering with in-tree builds:
> > 
> >   srctree=/absolute/path/to/linux
> >    OUTPUT=/absolute/path/to/linux/tools/perf
> > 
> > here's what happens:
> > 
> >  1. ln -sf $(srctree)/tools/perf $(OUTPUT)/source
> >     -> creates /absolute/path/to/linux/tools/perf/source
> >        link to /absolute/path/to/linux/tools/perf
> >     => OK, that's what was intended
> >  2. ln -sf $(srctree)/tools/perf $(OUTPUT)/source   # same command as 1
> >     -> creates /absolute/path/to/linux/tools/perf/perf
> >        link to /absolute/path/to/linux/tools/perf
> >     => Not what was intended, not idempotent
> >  3. Now the build _should_ create the 'perf' executable, but it fails
> > 
> > The reason is the tricky 'ln' command line. At the first invocation 'ln'
> > uses the 1st form:
> > 
> >        ln [OPTION]... [-T] TARGET LINK_NAME
> > 
> > and creates a link to TARGET *called LINK_NAME*.
> > 
> > At the second invocation $(OUTPUT)/source exists, so 'ln' uses the 3rd
> > form:
> > 
> >        ln [OPTION]... TARGET... DIRECTORY
> > 
> > and creates a link to TARGET *called TARGET* inside DIRECTORY.
> > 
> > Fix by adding --no-dereference to "treat LINK_NAME as a normal file if it
> > is a symbolic link to a directory", as the manpage says.
> > 
> > Link: https://lore.kernel.org/all/20241125182506.38af9907@booty/
> > Fixes: 890a1961c812 ("perf tools: Create source symlink in perf object dir")
> > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > ---
> >  tools/perf/Makefile.perf | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> > index d74241a151313bd09101aabb5d765a5a0a6efc84..bbd799a0fd544db220f29d1e250a819a765d04f3 100644
> > --- a/tools/perf/Makefile.perf
> > +++ b/tools/perf/Makefile.perf
> > @@ -164,7 +164,7 @@ ifneq ($(OUTPUT),)
> >  VPATH += $(OUTPUT)
> >  export VPATH
> >  # create symlink to the original source
> > -SOURCE := $(shell ln -sf $(srctree)/tools/perf $(OUTPUT)/source)
> > +SOURCE := $(shell ln -sf --no-dereference $(srctree)/tools/perf $(OUTPUT)/source)
> 
> The kernel Makefile has:
> 	$(Q)ln -fsn $(srcroot) source
> 
> So for parity the --no-dereference could become `n`, but it doesn't
> really matter.

I can make the change.

> 
> >  endif
> >  
> >  ifeq ($(V),1)
> > 
> > ---
> > base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
> > change-id: 20250124-perf-fix-intree-build-fbd97f560254
> > 
> > Best regards,
> > -- 
> > Luca Ceresoli <luca.ceresoli@bootlin.com>
> > 
> 
> Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
> Tested-by: Charlie Jenkins <charlie@rivosinc.com>
 
Thanks for your review!
Namhyung

Re: [PATCH] perf build: fix in-tree build

[Namhyung Kim]

On Fri, Jan 24, 2025 at 02:06:08PM +0100, Luca Ceresoli wrote:
> Building perf in-tree is broken after commit 890a1961c812 ("perf tools:
> Create source symlink in perf object dir") which added a 'source' symlink
> in the output dir pointing to the source dir.
> 
> With in-tree builds, the added 'SOURCE = ...' line is executed multiple
> times (I observed 2 during the build plus 2 during installation). This is a
> minor inefficiency, in theory not harmful because symlink creation is
> assumed to be idempotent. But it is not.
> 
> Considering with in-tree builds:
> 
>   srctree=/absolute/path/to/linux
>    OUTPUT=/absolute/path/to/linux/tools/perf
> 
[...]

Applied to perf-tools-next, thanks!

Best Regards,
Namhyung