| Message ID | 20240819160309.2218114-9-vegard.nossum@oracle.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | output a valid shell script when running 'make -n' | expand |
On Mon, Aug 19, 2024 at 06:03:05PM +0200, Vegard Nossum wrote: > Make link-vmlinux.sh print the commands it wants to run instead of > actually running them when $dry_run is defined. Do we need link-vmlinux.sh to show it's commands? Couldn't we just show the call of link-vmlinux.sh instead (and replace/hide the '+' prefix if 'dry_run' is set)? Why not? (I guess, you found good reasons. Can you please add them to the commit message?) > > This is needed in order for make -n/--dry-run to output a complete > build script. > > Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> > --- > scripts/Makefile.vmlinux | 15 ++++++++++++-- > scripts/link-vmlinux.sh | 44 ++++++++++++++++++++++++++-------------- > 2 files changed, 42 insertions(+), 17 deletions(-) > > diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux > index 10d80e07f945c..1fe0faab52889 100644 > --- a/scripts/Makefile.vmlinux > +++ b/scripts/Makefile.vmlinux > @@ -1,5 +1,12 @@ > # SPDX-License-Identifier: GPL-2.0-only > > +# there doesn't seem to be a straightforward way for 'make -n' to output > +# one command while actually executing another, so we resort to this hack > +# where we set an environment variable differently depending on whether > +# we are executing from 'make' or executing from the 'make -n' log. > +LINK_VMLINUX=scripts/link-vmlinux.sh > +export LINK_VMLINUX > + > PHONY := __default > __default: vmlinux > > @@ -29,7 +36,8 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) > > # Final link of vmlinux with optional arch pass after final link > cmd_link_vmlinux = \ > - $< "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)"; \ > + "$${LINK_VMLINUX}" \ > + "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)"; \ > $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) > > # We don't know how to build these > @@ -37,7 +45,10 @@ PHONY += vmlinux.o > PHONY += $(KBUILD_LDS) > > targets += vmlinux > -vmlinux: scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE > +vmlinux: $(LINK_VMLINUX) vmlinux.o $(KBUILD_LDS) FORCE > +ifdef dry_run > + @LINK_VMLINUX=/bin/true > +endif > +$(call if_changed_dep,link_vmlinux) > > # Add FORCE to the prequisites of a target to force it to be always rebuilt. > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh > index f7b2503cdba95..fd32e48a8a455 100755 > --- a/scripts/link-vmlinux.sh > +++ b/scripts/link-vmlinux.sh > @@ -32,6 +32,17 @@ LD="$1" > KBUILD_LDFLAGS="$2" > LDFLAGS_vmlinux="$3" > > +# If $dry_run is set, just echo the command instead of executing it. > +run() > +{ > + if [ ! -v dry_run ] > + then > + eval "$1" > + else > + echo "$1" > + fi > +} > + > is_enabled() { > grep -q "^$1=y" include/config/auto.conf > } > @@ -40,7 +51,10 @@ is_enabled() { > # Will be supressed by "make -s" > info() > { > - printf " %-7s %s\n" "${1}" "${2}" > + if [ ! -v dry_run ] > + then > + printf " %-7s %s\n" "${1}" "${2}" > + fi > } > > # Link of vmlinux Do you have some thoughts about the if is_enabled ... lines? Do we need to defer the evaluation of include/config/auto.conf, too? > @@ -97,10 +111,10 @@ vmlinux_link() > ldflags="${ldflags} ${wl}-Map=${output}.map" > fi > > - ${ld} ${ldflags} -o ${output} \ > + run "${ld} ${ldflags} -o ${output} \ > ${wl}--whole-archive ${objs} ${wl}--no-whole-archive \ > ${wl}--start-group ${libs} ${wl}--end-group \ > - ${kallsymso} ${btf_vmlinux_bin_o} ${ldlibs} > + ${kallsymso} ${btf_vmlinux_bin_o} ${ldlibs}" > } > > # generate .BTF typeinfo from DWARF debuginfo > @@ -129,8 +143,8 @@ gen_btf() > # deletes all symbols including __start_BTF and __stop_BTF, which will > # be redefined in the linker script. Add 2>/dev/null to suppress GNU > # objcopy warnings: "empty loadable segment detected at ..." > - ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \ > - --strip-all ${1} "${btf_data}" 2>/dev/null > + run "${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \ > + --strip-all ${1} "${btf_data}" 2>/dev/null" ^^^^^^^^^^^^^ Backslashes to keep ${btf_data} quoted are missing. > # Change e_type to ET_REL so that it can be used to link final vmlinux. > # GNU ld 2.35+ and lld do not allow an ET_EXEC input. > if is_enabled CONFIG_CPU_BIG_ENDIAN; then printf "${et_rel}" | dd of="${btf_data}" conv=notrunc bs=1 seek=16 status=none 'dd' should probably be called via 'run'. > @@ -161,11 +175,11 @@ kallsyms() > fi > > info KSYMS "${2}.S" > - scripts/kallsyms ${kallsymopt} "${1}" > "${2}.S" > + run "scripts/kallsyms ${kallsymopt} \"${1}\" > \"${2}.S\"" > > info AS "${2}.o" > - ${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS} \ > - ${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL} -c -o "${2}.o" "${2}.S" > + run "${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS} \ > + ${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL} -c -o \"${2}.o\" \"${2}.S\"" > > kallsymso=${2}.o > } > @@ -184,12 +198,12 @@ sysmap_and_kallsyms() > mksysmap() > { > info NM ${2} > - ${NM} -n "${1}" | sed -f "${srctree}/scripts/mksysmap" > "${2}" > + run "${NM} -n \"${1}\" | sed -f \"${srctree}/scripts/mksysmap\" > \"${2}\"" > } > > sorttable() > { > - ${objtree}/scripts/sorttable ${1} > + run "${objtree}/scripts/sorttable ${1}" > } > > cleanup() I know, 'link-vmlinux.sh clean' will not by called by 'make --dry-run clean' (as there is no '+' prefix in the call in Makefile), but for consistency, I'd like to see also cleanup() be prepared for handling dry_run correctly. Can you please move patch 11 (resetting of KBUILD_VERBOSE if '--dry-run' or '--silent' is given) before this one? Does '${MAKE} ...' also need to be called via 'run'? > @@ -270,13 +284,13 @@ if is_enabled CONFIG_KALLSYMS; then > strip_debug=1 > > sysmap_and_kallsyms .tmp_vmlinux1 > - size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso}) > + [ -v dry_run ] || size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso}) > > vmlinux_link .tmp_vmlinux2 > sysmap_and_kallsyms .tmp_vmlinux2 > - size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso}) > + [ -v dry_run ] || size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso}) > > - if [ $size1 -ne $size2 ] || [ -n "${KALLSYMS_EXTRA_PASS}" ]; then > + if [ ! -v dry_run ] || [ $size1 -ne $size2 ] || [ -n "${KALLSYMS_EXTRA_PASS}" ]; then > vmlinux_link .tmp_vmlinux3 > sysmap_and_kallsyms .tmp_vmlinux3 > fi > @@ -289,7 +303,7 @@ vmlinux_link vmlinux > # fill in BTF IDs > if is_enabled CONFIG_DEBUG_INFO_BTF && is_enabled CONFIG_BPF; then > info BTFIDS vmlinux > - ${RESOLVE_BTFIDS} vmlinux > + run ${RESOLVE_BTFIDS} vmlinux Quotes are missing, as run only evaluated its first argument. > fi > > mksysmap vmlinux System.map > @@ -303,7 +317,7 @@ if is_enabled CONFIG_BUILDTIME_TABLE_SORT; then I am sceptical about if ! sorttable vmlinux and similar. With dry_run being set, sorttable shows the call of 'sorttable' and always returns success. In effect, the resulting output shell build script will miss such error detection. I think it would be really good, if we could skip patching link-vmlinux.sh but show its call instead. Kind regards, Nicolas > fi > > # step a (see comment above) > -if is_enabled CONFIG_KALLSYMS; then > +if [ ! -v dry_run ] && is_enabled CONFIG_KALLSYMS; then > if ! cmp -s System.map "${kallsyms_sysmap}"; then > echo >&2 Inconsistent kallsyms data > echo >&2 'Try "make KALLSYMS_EXTRA_PASS=1" as a workaround' > -- > 2.34.1 >
diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux index 10d80e07f945c..1fe0faab52889 100644 --- a/scripts/Makefile.vmlinux +++ b/scripts/Makefile.vmlinux @@ -1,5 +1,12 @@ # SPDX-License-Identifier: GPL-2.0-only +# there doesn't seem to be a straightforward way for 'make -n' to output +# one command while actually executing another, so we resort to this hack +# where we set an environment variable differently depending on whether +# we are executing from 'make' or executing from the 'make -n' log. +LINK_VMLINUX=scripts/link-vmlinux.sh +export LINK_VMLINUX + PHONY := __default __default: vmlinux @@ -29,7 +36,8 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) # Final link of vmlinux with optional arch pass after final link cmd_link_vmlinux = \ - $< "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)"; \ + "$${LINK_VMLINUX}" \ + "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)"; \ $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) # We don't know how to build these @@ -37,7 +45,10 @@ PHONY += vmlinux.o PHONY += $(KBUILD_LDS) targets += vmlinux -vmlinux: scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE +vmlinux: $(LINK_VMLINUX) vmlinux.o $(KBUILD_LDS) FORCE +ifdef dry_run + @LINK_VMLINUX=/bin/true +endif +$(call if_changed_dep,link_vmlinux) # Add FORCE to the prequisites of a target to force it to be always rebuilt. diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh index f7b2503cdba95..fd32e48a8a455 100755 --- a/scripts/link-vmlinux.sh +++ b/scripts/link-vmlinux.sh @@ -32,6 +32,17 @@ LD="$1" KBUILD_LDFLAGS="$2" LDFLAGS_vmlinux="$3" +# If $dry_run is set, just echo the command instead of executing it. +run() +{ + if [ ! -v dry_run ] + then + eval "$1" + else + echo "$1" + fi +} + is_enabled() { grep -q "^$1=y" include/config/auto.conf } @@ -40,7 +51,10 @@ is_enabled() { # Will be supressed by "make -s" info() { - printf " %-7s %s\n" "${1}" "${2}" + if [ ! -v dry_run ] + then + printf " %-7s %s\n" "${1}" "${2}" + fi } # Link of vmlinux @@ -97,10 +111,10 @@ vmlinux_link() ldflags="${ldflags} ${wl}-Map=${output}.map" fi - ${ld} ${ldflags} -o ${output} \ + run "${ld} ${ldflags} -o ${output} \ ${wl}--whole-archive ${objs} ${wl}--no-whole-archive \ ${wl}--start-group ${libs} ${wl}--end-group \ - ${kallsymso} ${btf_vmlinux_bin_o} ${ldlibs} + ${kallsymso} ${btf_vmlinux_bin_o} ${ldlibs}" } # generate .BTF typeinfo from DWARF debuginfo @@ -129,8 +143,8 @@ gen_btf() # deletes all symbols including __start_BTF and __stop_BTF, which will # be redefined in the linker script. Add 2>/dev/null to suppress GNU # objcopy warnings: "empty loadable segment detected at ..." - ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \ - --strip-all ${1} "${btf_data}" 2>/dev/null + run "${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \ + --strip-all ${1} "${btf_data}" 2>/dev/null" # Change e_type to ET_REL so that it can be used to link final vmlinux. # GNU ld 2.35+ and lld do not allow an ET_EXEC input. if is_enabled CONFIG_CPU_BIG_ENDIAN; then @@ -161,11 +175,11 @@ kallsyms() fi info KSYMS "${2}.S" - scripts/kallsyms ${kallsymopt} "${1}" > "${2}.S" + run "scripts/kallsyms ${kallsymopt} \"${1}\" > \"${2}.S\"" info AS "${2}.o" - ${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS} \ - ${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL} -c -o "${2}.o" "${2}.S" + run "${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS} \ + ${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL} -c -o \"${2}.o\" \"${2}.S\"" kallsymso=${2}.o } @@ -184,12 +198,12 @@ sysmap_and_kallsyms() mksysmap() { info NM ${2} - ${NM} -n "${1}" | sed -f "${srctree}/scripts/mksysmap" > "${2}" + run "${NM} -n \"${1}\" | sed -f \"${srctree}/scripts/mksysmap\" > \"${2}\"" } sorttable() { - ${objtree}/scripts/sorttable ${1} + run "${objtree}/scripts/sorttable ${1}" } cleanup() @@ -270,13 +284,13 @@ if is_enabled CONFIG_KALLSYMS; then strip_debug=1 sysmap_and_kallsyms .tmp_vmlinux1 - size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso}) + [ -v dry_run ] || size1=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso}) vmlinux_link .tmp_vmlinux2 sysmap_and_kallsyms .tmp_vmlinux2 - size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso}) + [ -v dry_run ] || size2=$(${CONFIG_SHELL} "${srctree}/scripts/file-size.sh" ${kallsymso}) - if [ $size1 -ne $size2 ] || [ -n "${KALLSYMS_EXTRA_PASS}" ]; then + if [ ! -v dry_run ] || [ $size1 -ne $size2 ] || [ -n "${KALLSYMS_EXTRA_PASS}" ]; then vmlinux_link .tmp_vmlinux3 sysmap_and_kallsyms .tmp_vmlinux3 fi @@ -289,7 +303,7 @@ vmlinux_link vmlinux # fill in BTF IDs if is_enabled CONFIG_DEBUG_INFO_BTF && is_enabled CONFIG_BPF; then info BTFIDS vmlinux - ${RESOLVE_BTFIDS} vmlinux + run ${RESOLVE_BTFIDS} vmlinux fi mksysmap vmlinux System.map @@ -303,7 +317,7 @@ if is_enabled CONFIG_BUILDTIME_TABLE_SORT; then fi # step a (see comment above) -if is_enabled CONFIG_KALLSYMS; then +if [ ! -v dry_run ] && is_enabled CONFIG_KALLSYMS; then if ! cmp -s System.map "${kallsyms_sysmap}"; then echo >&2 Inconsistent kallsyms data echo >&2 'Try "make KALLSYMS_EXTRA_PASS=1" as a workaround'
Make link-vmlinux.sh print the commands it wants to run instead of actually running them when $dry_run is defined. This is needed in order for make -n/--dry-run to output a complete build script. Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com> --- scripts/Makefile.vmlinux | 15 ++++++++++++-- scripts/link-vmlinux.sh | 44 ++++++++++++++++++++++++++-------------- 2 files changed, 42 insertions(+), 17 deletions(-)