Kbuild.include: addtree: Remove quotes before matching path
Subsystems (2)
PATCH 1/1 Kbuild.include: addtree: Remove quotes before matching path
systemtap currently fails to build modules when the kernel source and object trees are separate.
systemtap adds something like -I"/usr/share/systemtap/runtime" to EXTRA_CFLAGS, and addtree should not adjust this as it's specifying an absolute directory. But since make has no understanding of shell quoting, it does anyway.
For a long time this didn't matter, because addtree would still emit the original -I option after the adjusted one. However, commit db547ef19064 ("Kbuild: don't add obj tree in additional includes") changed it to remove the original -I option.
Remove quotes (both double and single) before matching against the excluded patterns.
References: https://bugs.debian.org/856474
---
scripts/Kbuild.include +1 -1
@@ -207,7 +207,7 @@ hdr-inst := -f $(srctree)/scripts/Makefi # Prefix -I with $(srctree) if it is not an absolute path. # skip if -I has no parameter addtree = $(if $(patsubst -I%,%,$(1)), \ -$(if $(filter-out -I/% -I./% -I../%,$(1)),$(patsubst -I%,-I$(srctree)/%,$(1)),$(1))) +$(if $(filter-out -I/% -I./% -I../%,$(subst $(quote),,$(subst $(squote),,$(1)))),$(patsubst -I%,-I$(srctree)/%,$(1)),$(1))) # Find all -I options and call addtree flags = $(foreach o,$($(1)),$(if $(filter -I%,$(o)),$(call addtree,$(o)),$(o)))
+To Arnd
+To Michal
2017-03-19 6:58 GMT+09:00 Ben Hutchings <[email protected]>:
Quoted 26 lines
systemtap currently fails to build modules when the kernel source and object trees are separate.
systemtap adds something like -I"/usr/share/systemtap/runtime" to
EXTRA_CFLAGS, and addtree should not adjust this as it's specifying an absolute directory. But since make has no understanding of shell quoting, it does anyway.For a long time this didn't matter, because addtree would still emit the original -I option after the adjusted one. However, commit
db547ef19064("Kbuild: don't add obj tree in additional includes") changed it to remove the original -I option.Remove quotes (both double and single) before matching against the excluded patterns.
References: https://bugs.debian.org/856474
Reported-and-tested-by Jack Henschel <[email protected]>Reported-and-tested-by Ritesh Raj Sarraf <[email protected]>Fixes db547ef19064 ("Kbuild: don't add obj tree in additional includes")Cc [email protected] # 4.8+Signed-off-by Ben Hutchings <[email protected]>---
scripts/Kbuild.include hdr-inst := -f $(srctree)/scripts/Makefi · line 213
Quoted 8 lines
scripts/Kbuild.include +1
-1@@ -207,7 +207,7 @@ hdr-inst := -f $(srctree)/scripts/Makefi # Prefix -I with $(srctree) if it is not an absolute path. # skip if -I has no parameter addtree = $(if $(patsubst -I%,%,$(1)), \ -$(if $(filter-out -I/% -I./% -I../%,$(1)),$(patsubst -I%,-I$(srctree)/%,$(1)),$(1))) +$(if $(filter-out -I/% -I./% -I../%,$(subst $(quote),,$(subst $(squote),,$(1)))),$(patsubst -I%,-I$(srctree)/%,$(1)),$(1))) # Find all -I options and call addtree flags = $(foreach o,$($(1)),$(if $(filter -I%,$(o)),$(call addtree,$(o)),$(o)))
I have been thinking about this patch.
We touched addtree every time a new problem popped up.
It would be easy to pick up this patch, but it looks like a crazy idea
to play pattern-matching in the addtree even more.
[1] -Ifoo/bar/baz
[2] -I"foo/bar/baz"
[3] -I foo/bar/baz (whitespace(s) after -I)
All of these are valid notation,
and should be handled in the same way.
However, addtree expects only [1],
(then this patch is going to add [2] in the soup).
Each Makefile knows it wants to see additional headers in the source tree, or objtree.
I am guessing the right approach in a long run is, we require -I to specify $(srctree) or $(objtree) explicitly.
ccflags-y := -I$(srctree)/foo/bar/baz
or
ccflags-y := -I$(objtree)/foo/bar/baz
(For the latter, we can omit $(objtree)/ as it is ./)
Then, delete $(call flags,_c_flags) after the conversion.
Any comments?
Signature · 2 lines
Best Regards Masahiro Yamada
Quoted 10 lines · Masahiro Yamada
Each
Makefileknows it wants to see additional headers in the source tree, or objtree.I am guessing the right approach in a long run is, we require -I to specify $(srctree) or $(objtree) explicitly.
ccflags-y := -I$(srctree)/foo/bar/baz
or
Quoted 7 lines
ccflags-y := -I$(objtree)/foo/bar/baz
(For the latter, we can omit $(objtree)/ as it is ./)
Then, delete $(call flags,_c_flags) after the conversion.
Agreed. The addtree function is more of a hack to make things just work with O=, but AFAIK there is no clean way to implement VPATH for -I arguments. So it's sensible to get rid of the hack. It looks like it's going to be lot of work though:
$ git grep -e '-I' -- '*Makefile*' | wc -l 732 $ git grep -e '-I *\$(\(src\|obj\)tree)' -- '*Makefile*' | wc -l 166
Michal
Quoted 24 lines · Michal Marek
Quoted 10 lines · Masahiro Yamada
Each
Makefileknows it wants to see additional headers in the source tree, or objtree.I am guessing the right approach in a long run is, we require -I to specify $(srctree) or $(objtree) explicitly.
ccflags-y := -I$(srctree)/foo/bar/baz
or
Quoted 7 lines
ccflags-y := -I$(objtree)/foo/bar/baz
(For the latter, we can omit $(objtree)/ as it is ./)
Then, delete $(call flags,_c_flags) after the conversion.
Agreed. The addtree function is more of a hack to make things just work with O=, but AFAIK there is no clean way to implement VPATH for -I arguments. So it's sensible to get rid of the hack. It looks like it's going to be lot of work though:
Quoted 4 lines
$ git grep -e '-I' -- '*Makefile*' | wc -l 732 $ git grep -e '-I *\$(\(src\|obj\)tree)' -- '*Makefile*' | wc -l 166
There was a goal long time ago that moving the kernel source should not trigger a rebuild.
Any hardcoded path would violate this (like $(srctree), $(objtree))
I dunno if this is really something to aim for today.
I have personally from time to time renamed the directory where I have kernel soruce (which is seen like moving the kernel source), and would not be happy if this always triggered a full rebuild. But this is frankly a corner case.
Sam
Hi Sam,
2017-04-04 5:20 GMT+09:00 Sam Ravnborg <[email protected]>:
Quoted 25 lines · Michal Marek
Quoted 24 lines · Masahiro Yamada
Quoted 10 lines
Each
Makefileknows it wants to see additional headers in the source tree, or objtree.I am guessing the right approach in a long run is, we require -I to specify $(srctree) or $(objtree) explicitly.
ccflags-y := -I$(srctree)/foo/bar/baz
or
Quoted 7 lines
ccflags-y := -I$(objtree)/foo/bar/baz
(For the latter, we can omit $(objtree)/ as it is ./)
Then, delete $(call flags,_c_flags) after the conversion.
Agreed. The addtree function is more of a hack to make things just work with O=, but AFAIK there is no clean way to implement VPATH for -I arguments. So it's sensible to get rid of the hack. It looks like it's going to be lot of work though:
Quoted 8 lines
Quoted 4 lines
$ git grep -e '-I' -- '*Makefile*' | wc -l 732 $ git grep -e '-I *\$(\(src\|obj\)tree)' -- '*Makefile*' | wc -l 166There was a goal long time ago that moving the kernel source should not trigger a rebuild.
Any hardcoded path would violate this (like $(srctree), $(objtree))
Even if we avoid hard-coding $(srctree) in each Makefile, "addtree" will modify the -I paths and the modified paths will be recorded in .*.cmd files.
If we really want to achieve the goal,
my old patch seemed to have a point:
https://patchwork.kernel.org/patch/4511991/
This was rejected due to side effects, anyway.
Signature · 2 lines
Best Regards Masahiro Yamada
Dne 3.4.2017 v 22:20 Sam Ravnborg napsal(a):
Quoted 34 lines · Michal Marek
Quoted 24 lines · Masahiro Yamada
Quoted 10 lines
Each
Makefileknows it wants to see additional headers in the source tree, or objtree.I am guessing the right approach in a long run is, we require -I to specify $(srctree) or $(objtree) explicitly.
ccflags-y := -I$(srctree)/foo/bar/baz
or
Quoted 7 lines
ccflags-y := -I$(objtree)/foo/bar/baz
(For the latter, we can omit $(objtree)/ as it is ./)
Then, delete $(call flags,_c_flags) after the conversion.
Agreed. The addtree function is more of a hack to make things just work with O=, but AFAIK there is no clean way to implement VPATH for -I arguments. So it's sensible to get rid of the hack. It looks like it's going to be lot of work though:
Quoted 4 lines
$ git grep -e '-I' -- '*Makefile*' | wc -l 732 $ git grep -e '-I *\$(\(src\|obj\)tree)' -- '*Makefile*' | wc -l 166There was a goal long time ago that moving the kernel source should not trigger a rebuild.
Any hardcoded path would violate this (like $(srctree), $(objtree))
Quoted 6 lines
I dunno if this is really something to aim for today.
I have personally from time to time renamed the directory where I have kernel soruce (which is seen like moving the kernel source), and would not be happy if this always triggered a full rebuild. But this is frankly a corner case.
This should be working nowadays if you build without O= or if the buildtree points to a subdirectory of the source tree. The srctree variable is set to . or .. in such case. Although when I tried it now, it still does not look perfect:
$ cd /dev/shm/mmarek/linux-2.6 $ make O=build -s defconfig $ make O=build -s -j64 Setup is 15500 bytes (padded to 15872 bytes). System is 6530 kB CRC 4bc8f6e2 Kernel: arch/x86/boot/bzImage is ready (#1) $ grep -r $PWD build/ Binary file build/arch/x86/boot/setup.elf matches Binary file build/arch/x86/boot/header.o matches Binary file build/arch/x86/boot/video.o matches Binary file build/arch/x86/boot/cpu.o matches Binary file build/arch/x86/boot/printf.o matches Binary file build/arch/x86/boot/string.o matches Binary file build/arch/x86/boot/video-mode.o matches Binary file build/arch/x86/boot/video-vga.o matches Binary file build/arch/x86/boot/early_serial_console.o matches Binary file build/arch/x86/boot/video-vesa.o matches Binary file build/arch/x86/boot/cpucheck.o matches Binary file build/arch/x86/boot/video-bios.o matches Binary file build/arch/x86/boot/tty.o matches Binary file build/arch/x86/boot/memory.o matches Binary file build/arch/x86/boot/pm.o matches Binary file build/arch/x86/boot/cmdline.o matches Binary file build/arch/x86/boot/main.o matches Binary file build/arch/x86/boot/a20.o matches Binary file build/arch/x86/boot/regs.o matches Binary file build/arch/x86/boot/version.o matches Binary file build/arch/x86/boot/edd.o matches Binary file build/arch/x86/boot/cpuflags.o matches Binary file build/arch/x86/boot/pmjump.o matches Binary file build/arch/x86/boot/copy.o matches Binary file build/arch/x86/boot/bioscall.o matches Binary file build/arch/x86/realmode/rm/video-bios.o matches Binary file build/arch/x86/realmode/rm/video-mode.o matches Binary file build/arch/x86/realmode/rm/video-vga.o matches Binary file build/arch/x86/realmode/rm/video-vesa.o matches Binary file build/arch/x86/realmode/rm/wakemain.o matches Binary file build/arch/x86/realmode/rm/regs.o matches Binary file build/arch/x86/realmode/rm/trampoline_64.o matches Binary file build/arch/x86/realmode/rm/wakeup_asm.o matches Binary file build/arch/x86/realmode/rm/copy.o matches Binary file build/arch/x86/realmode/rm/bioscall.o matches Binary file build/arch/x86/realmode/rm/reboot.o matches
All the 32bit object files contain the full path, but none of the .cmd files do?! And we do trigger rebuild of a few files after moving the tree, but we do so even in the original location, so it looks like somebody got a custom rule wrong again.
Michal