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

[Namhyung Kim <namhyung@kernel.org>]

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