On Sat, Jan 11, 2025 at 10:13 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
>
> On Sat, Jan 11, 2025 at 3:33 AM Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > On Fri, 10 Jan 2025 at 10:11, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> > >
> > > I suppose this is a consequence of 8962b6b475bddc ("kbuild: print
> > > short log in addition to the whole command with V=1") which introduced
> > > an $(if ) where the else branch is simply ':' and so it always
> > > succeeds.
> >
> > Hmm. Odd. I don't see why that part of the commit exists, and you're
> > right, that seems like a bad idea.
> >
> > And removing that odd $(if..) and making it just do
> >
> > cmd = @set -e; $($(quiet)log_print) $(delete-on-interrupt) $(cmd_$(1))
> >
> > doesn't seem to have any obvious negative effects.
>
> When cmd_foo is not defined, $(call if_changed,foo) will fail with this change,
> but $(call cmd,foo) will succeed regardless.
>
> In Makefile, the variable expansion works like the latter.
> When X is not defined, $(X) is expanded to an empty string successfully.
> This is useful.
>
>
> > I'm probably missing some obvious reason why that $(if..) was added,
> > it's been that way for two years now.
>
> We do not need to guard both a definition and its callsite.
>
> For example, the current code in arch/powerpc/Makefile.postlink:
>
> vmlinux: FORCE
> @true
> ifdef CONFIG_PPC64
> $(call cmd,head_check)
> endif
> ifdef CONFIG_RELOCATABLE
> $(call if_changed,relocs_check)
> endif
> ifdef CONFIG_FUNCTION_TRACER
> ifndef CONFIG_PPC64_ELF_ABI_V1
> $(call cmd,ftrace_check)
> endif
> endif
>
>
> ... can be simplified into:
>
> vmlinux: FORCE
> $(call cmd,head_check)
> $(call if_changed,relocs_check)
> $(call cmd,ftrace_check)
>
arch/riscv/Makefile.postlink is a better example.
CONFIG_RELOCATABLE for the callsite is redundant.
--
Best Regards
Masahiro Yamada
