diff mbox series

[WIP,01/30] build: replace uses of $(abspath ) with existing variables

Message ID 2173c7fa03e24291f2f59423d77a3cb175317688.1709508290.git.ehem+linux@m5p.com (mailing list archive)
State New
Headers show
Series Adding trailing slash to $(*tree) | expand

Commit Message

Elliott Mitchell March 1, 2024, 8:51 p.m. UTC 
As $(abs_objtree) and $(abs_srctree) already exist, $(abspath )
shouldn't be used with $(objtree) or $(srctree).

Fixes: 0e1aa629f1ce ("kbuild: Do not clean resolve_btfids if the output does not exist")
Signed-off-by: Elliott Mitchell <ehem+linux@m5p.com>
---
I believe this is pretty much all fixes, but 0e1aa629f1ce was the most
recent one.
---
 Documentation/Makefile     |  8 ++++----
 Makefile                   |  8 ++++----
 rust/Makefile              | 14 +++++++-------
 samples/bpf/Makefile       |  2 +-
 samples/hid/Makefile       |  2 +-
 scripts/gdb/linux/Makefile |  2 +-
 6 files changed, 18 insertions(+), 18 deletions(-)

Comments

Nicolas Schier March 4, 2024, 9:50 a.m. UTC | #1 
On Fri, Mar 01, 2024 at 12:51:01PM -0800, Elliott Mitchell wrote:
> As $(abs_objtree) and $(abs_srctree) already exist, $(abspath )
> shouldn't be used with $(objtree) or $(srctree).
> 
> Fixes: 0e1aa629f1ce ("kbuild: Do not clean resolve_btfids if the output does not exist")
> Signed-off-by: Elliott Mitchell <ehem+linux@m5p.com>
> ---
> I believe this is pretty much all fixes, but 0e1aa629f1ce was the most
> recent one.

If you add a 'Fixes' trailer, please just include changes that fix that
commit.  You're patch does much more than just modifying the changes
from commit 0e1aa629f1ce.

Kind regards,
Nicolas
Nicolas Schier March 4, 2024, 10:01 a.m. UTC | #2 
On Mon, Mar 04, 2024 at 10:50:49AM +0100, Nicolas Schier wrote:
> On Fri, Mar 01, 2024 at 12:51:01PM -0800, Elliott Mitchell wrote:
> > As $(abs_objtree) and $(abs_srctree) already exist, $(abspath )
> > shouldn't be used with $(objtree) or $(srctree).
> > 
> > Fixes: 0e1aa629f1ce ("kbuild: Do not clean resolve_btfids if the output does not exist")
> > Signed-off-by: Elliott Mitchell <ehem+linux@m5p.com>
> > ---
> > I believe this is pretty much all fixes, but 0e1aa629f1ce was the most
> > recent one.
> 
> If you add a 'Fixes' trailer, please just include changes that fix that
> commit.  You're patch does much more than just modifying the changes
> from commit 0e1aa629f1ce.

oh and please do not re-use the same commit subject line for different
changes.  This will be quite confusing when reading the git history.

Kind regards,
Nicolas
Elliott Mitchell March 4, 2024, 7:45 p.m. UTC | #3 
On Mon, Mar 04, 2024 at 10:50:49AM +0100, Nicolas Schier wrote:
> On Fri, Mar 01, 2024 at 12:51:01PM -0800, Elliott Mitchell wrote:
> > As $(abs_objtree) and $(abs_srctree) already exist, $(abspath )
> > shouldn't be used with $(objtree) or $(srctree).
> > 
> > Fixes: 0e1aa629f1ce ("kbuild: Do not clean resolve_btfids if the output does not exist")
> > Signed-off-by: Elliott Mitchell <ehem+linux@m5p.com>
> > ---
> > I believe this is pretty much all fixes, but 0e1aa629f1ce was the most
> > recent one.
> 
> If you add a 'Fixes' trailer, please just include changes that fix that
> commit.  You're patch does much more than just modifying the changes
> from commit 0e1aa629f1ce.

Since all of these should have been used $(abs_srctree) or $(abs_objtree)
when committed, all of these are fixes.  Issue is most of them moved at
least once, so tracking down the list is annoying.

You consider ignoring this is fixing around 5-13 commits reasonable?


On Mon, Mar 04, 2024 at 11:01:24AM +0100, Nicolas Schier wrote:
> oh and please do not re-use the same commit subject line for different
> changes.  This will be quite confusing when reading the git history.

Several of these were originally a single commit.  Many of them are
doing exactly the same job, just to a distinct filename pattern.  Issue
is #5 is already so large squashing things together would likely make
reviewing harder.
Nicolas Schier March 5, 2024, 10:51 a.m. UTC | #4 
On Mon, Mar 04, 2024 at 11:45:45AM -0800, Elliott Mitchell wrote:
> On Mon, Mar 04, 2024 at 10:50:49AM +0100, Nicolas Schier wrote:
> > On Fri, Mar 01, 2024 at 12:51:01PM -0800, Elliott Mitchell wrote:
> > > As $(abs_objtree) and $(abs_srctree) already exist, $(abspath )
> > > shouldn't be used with $(objtree) or $(srctree).
> > > 
> > > Fixes: 0e1aa629f1ce ("kbuild: Do not clean resolve_btfids if the output does not exist")
> > > Signed-off-by: Elliott Mitchell <ehem+linux@m5p.com>
> > > ---
> > > I believe this is pretty much all fixes, but 0e1aa629f1ce was the most
> > > recent one.
> > 
> > If you add a 'Fixes' trailer, please just include changes that fix that
> > commit.  You're patch does much more than just modifying the changes
> > from commit 0e1aa629f1ce.
> 
> Since all of these should have been used $(abs_srctree) or $(abs_objtree)
> when committed, all of these are fixes.  Issue is most of them moved at
> least once, so tracking down the list is annoying.
> 
> You consider ignoring this is fixing around 5-13 commits reasonable?

I think, fixing inconsistencies is a good thing, but I'm afraid it's
not always important enough to mark such changes as 'Fixes', as 'Fixes'
is especially used for auto-selecting patches for stable trees.

Kind regards,
Nicolas
diff mbox series

Patch

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 3885bbe260eb..85d8deace4bf 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -82,19 +82,19 @@  loop_cmd = $(echo-cmd) $(cmd_$(1)) || exit;
 quiet_cmd_sphinx = SPHINX  $@ --> file://$(abspath $(BUILDDIR)/$3/$4)
       cmd_sphinx = $(MAKE) BUILDDIR=$(abspath $(BUILDDIR)) $(build)=Documentation/userspace-api/media $2 && \
 	PYTHONDONTWRITEBYTECODE=1 \
-	BUILDDIR=$(abspath $(BUILDDIR)) SPHINX_CONF=$(abspath $(srctree)/$(src)/$5/$(SPHINX_CONF)) \
+	BUILDDIR=$(abspath $(BUILDDIR)) SPHINX_CONF=$(abs_srctree)/$(src)/$5/$(SPHINX_CONF) \
 	$(PYTHON3) $(srctree)/scripts/jobserver-exec \
 	$(CONFIG_SHELL) $(srctree)/Documentation/sphinx/parallel-wrapper.sh \
 	$(SPHINXBUILD) \
 	-b $2 \
-	-c $(abspath $(srctree)/$(src)) \
+	-c $(abs_srctree)/$(src) \
 	-d $(abspath $(BUILDDIR)/.doctrees/$3) \
 	-D version=$(KERNELVERSION) -D release=$(KERNELRELEASE) \
 	$(ALLSPHINXOPTS) \
-	$(abspath $(srctree)/$(src)/$5) \
+	$(abs_srctree)/$(src)/$5 \
 	$(abspath $(BUILDDIR)/$3/$4) && \
 	if [ "x$(DOCS_CSS)" != "x" ]; then \
-		cp $(if $(patsubst /%,,$(DOCS_CSS)),$(abspath $(srctree)/$(DOCS_CSS)),$(DOCS_CSS)) $(BUILDDIR)/$3/_static/; \
+		cp $(if $(patsubst /%,,$(DOCS_CSS)),$(abs_srctree)/$(DOCS_CSS),$(DOCS_CSS)) $(BUILDDIR)/$3/_static/; \
 	fi
 
 YNL_INDEX:=$(srctree)/Documentation/networking/netlink_spec/index.rst
diff --git a/Makefile b/Makefile
index 6cdb5717bfe0..a7696f13bece 100644
--- a/Makefile
+++ b/Makefile
@@ -1337,7 +1337,7 @@  endif
 
 PHONY += resolve_btfids_clean
 
-resolve_btfids_O = $(abspath $(objtree))/tools/bpf/resolve_btfids
+resolve_btfids_O = $(abs_objtree)/tools/bpf/resolve_btfids
 
 # tools/bpf/resolve_btfids directory might not exist
 # in output directory, skip its clean in that case
@@ -1353,11 +1353,11 @@  endif
 
 tools/: FORCE
 	$(Q)mkdir -p $(objtree)/tools
-	$(Q)$(MAKE) LDFLAGS= MAKEFLAGS="$(tools_silent) $(filter --j% -j,$(MAKEFLAGS))" O=$(abspath $(objtree)) subdir=tools -C $(srctree)/tools/
+	$(Q)$(MAKE) LDFLAGS= MAKEFLAGS="$(tools_silent) $(filter --j% -j,$(MAKEFLAGS))" O=$(abs_objtree) subdir=tools -C $(srctree)/tools/
 
 tools/%: FORCE
 	$(Q)mkdir -p $(objtree)/tools
-	$(Q)$(MAKE) LDFLAGS= MAKEFLAGS="$(tools_silent) $(filter --j% -j,$(MAKEFLAGS))" O=$(abspath $(objtree)) subdir=tools -C $(srctree)/tools/ $*
+	$(Q)$(MAKE) LDFLAGS= MAKEFLAGS="$(tools_silent) $(filter --j% -j,$(MAKEFLAGS))" O=$(abs_objtree) subdir=tools -C $(srctree)/tools/ $*
 
 # ---------------------------------------------------------------------------
 # Kernel selftest
@@ -1757,7 +1757,7 @@  all: misc-check
 PHONY += scripts_gdb
 scripts_gdb: prepare0
 	$(Q)$(MAKE) $(build)=scripts/gdb
-	$(Q)ln -fsn $(abspath $(srctree)/scripts/gdb/vmlinux-gdb.py)
+	$(Q)ln -fsn $(abs_srctree)/scripts/gdb/vmlinux-gdb.py
 
 ifdef CONFIG_GDB_SCRIPTS
 all: scripts_gdb
diff --git a/rust/Makefile b/rust/Makefile
index 9d2a16cc91cb..0a7c278abf0b 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -73,7 +73,7 @@  alloc-cfgs = \
 
 quiet_cmd_rustdoc = RUSTDOC $(if $(rustdoc_host),H, ) $<
       cmd_rustdoc = \
-	OBJTREE=$(abspath $(objtree)) \
+	OBJTREE=$(abs_objtree) \
 	$(RUSTDOC) $(if $(rustdoc_host),$(rust_common_flags),$(rust_flags)) \
 		$(rustc_target_flags) -L$(objtree)/$(obj) \
 		--output $(rustdoc_output) \
@@ -136,7 +136,7 @@  rustdoc-kernel: $(src)/kernel/lib.rs rustdoc-core rustdoc-macros \
 
 quiet_cmd_rustc_test_library = RUSTC TL $<
       cmd_rustc_test_library = \
-	OBJTREE=$(abspath $(objtree)) \
+	OBJTREE=$(abs_objtree) \
 	$(RUSTC) $(rust_common_flags) \
 		@$(objtree)/include/generated/rustc_cfg $(rustc_target_flags) \
 		--crate-type $(if $(rustc_test_library_proc),proc-macro,rlib) \
@@ -161,7 +161,7 @@  rusttestlib-uapi: $(src)/uapi/lib.rs rusttest-prepare FORCE
 
 quiet_cmd_rustdoc_test = RUSTDOC T $<
       cmd_rustdoc_test = \
-	OBJTREE=$(abspath $(objtree)) \
+	OBJTREE=$(abs_objtree) \
 	$(RUSTDOC) --test $(rust_common_flags) \
 		@$(objtree)/include/generated/rustc_cfg \
 		$(rustc_target_flags) $(rustdoc_test_target_flags) \
@@ -173,7 +173,7 @@  quiet_cmd_rustdoc_test_kernel = RUSTDOC TK $<
       cmd_rustdoc_test_kernel = \
 	rm -rf $(objtree)/$(obj)/test/doctests/kernel; \
 	mkdir -p $(objtree)/$(obj)/test/doctests/kernel; \
-	OBJTREE=$(abspath $(objtree)) \
+	OBJTREE=$(abs_objtree) \
 	$(RUSTDOC) --test $(rust_flags) \
 		@$(objtree)/include/generated/rustc_cfg \
 		-L$(objtree)/$(obj) --extern alloc --extern kernel \
@@ -195,7 +195,7 @@  quiet_cmd_rustdoc_test_kernel = RUSTDOC TK $<
 # so for the moment we skip `-Cpanic=abort`.
 quiet_cmd_rustc_test = RUSTC T  $<
       cmd_rustc_test = \
-	OBJTREE=$(abspath $(objtree)) \
+	OBJTREE=$(abs_objtree) \
 	$(RUSTC) --test $(rust_common_flags) \
 		@$(objtree)/include/generated/rustc_cfg \
 		$(rustc_target_flags) --out-dir $(objtree)/$(obj)/test \
@@ -242,7 +242,7 @@  quiet_cmd_rustsysroot = RUSTSYSROOT
 	cp -r $(srctree)/$(src)/alloc/* \
 		$(objtree)/$(obj)/test/sysroot/lib/rustlib/src/rust/library/alloc/src; \
 	echo '\#!/bin/sh' > $(objtree)/$(obj)/test/rustc_sysroot; \
-	echo "$(RUSTC) --sysroot=$(abspath $(objtree)/$(obj)/test/sysroot) \"\$$@\"" \
+	echo "$(RUSTC) --sysroot=$(abs_objtree)/$(obj)/test/sysroot \"\$$@\"" \
 		>> $(objtree)/$(obj)/test/rustc_sysroot; \
 	chmod u+x $(objtree)/$(obj)/test/rustc_sysroot; \
 	$(CARGO) -q new $(objtree)/$(obj)/test/dummy; \
@@ -400,7 +400,7 @@  $(obj)/libmacros.so: $(src)/macros/lib.rs $(obj)/core.o FORCE
 
 quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L $@
       cmd_rustc_library = \
-	OBJTREE=$(abspath $(objtree)) \
+	OBJTREE=$(abs_objtree) \
 	$(if $(skip_clippy),$(RUSTC),$(RUSTC_OR_CLIPPY)) \
 		$(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \
 		--emit=dep-info=$(depfile) --emit=obj=$@ \
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 933f6c3fe6b0..8b53249aa73a 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -1,6 +1,6 @@ 
 # SPDX-License-Identifier: GPL-2.0
 
-BPF_SAMPLES_PATH ?= $(abspath $(srctree)/$(src))
+BPF_SAMPLES_PATH ?= $(abs_srctree)/$(src)
 TOOLS_PATH := $(BPF_SAMPLES_PATH)/../../tools
 
 pound := \#
diff --git a/samples/hid/Makefile b/samples/hid/Makefile
index 9f7fe29dd749..8af8a52d0ab7 100644
--- a/samples/hid/Makefile
+++ b/samples/hid/Makefile
@@ -1,6 +1,6 @@ 
 # SPDX-License-Identifier: GPL-2.0
 
-HID_SAMPLES_PATH ?= $(abspath $(srctree)/$(src))
+HID_SAMPLES_PATH ?= $(abs_srctree)/$(src)
 TOOLS_PATH := $(HID_SAMPLES_PATH)/../../tools
 
 pound := \#
diff --git a/scripts/gdb/linux/Makefile b/scripts/gdb/linux/Makefile
index 48941faa6ea6..52b5c961a6b1 100644
--- a/scripts/gdb/linux/Makefile
+++ b/scripts/gdb/linux/Makefile
@@ -5,7 +5,7 @@  ifdef building_out_of_srctree
 symlinks := $(patsubst $(srctree)/$(src)/%,%,$(wildcard $(srctree)/$(src)/*.py))
 
 quiet_cmd_symlink = SYMLINK $@
-      cmd_symlink = ln -fsn $(patsubst $(obj)/%,$(abspath $(srctree))/$(src)/%,$@) $@
+      cmd_symlink = ln -fsn $(patsubst $(obj)/%,$(abs_srctree)/$(src)/%,$@) $@
 
 always-y += $(symlinks)
 $(addprefix $(obj)/, $(symlinks)): FORCE