| Message ID | 20231215160637.842748-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] kbuild: resolve symlinks for O= properly | expand |
On 2023-12-16 01:06:37 [+0900], Masahiro Yamada wrote: … > Using the physical directory structure for the O= option seems more > reasonable. > > The comment says "expand a shell special character '~'", but it has > already been expanded to the home directory in the command line. It might have been expanded, it might have not been expanded. Having a shell script: | #!/bin/sh | | exec make O=~/scratch/mk-check defconfig with bin/sh = dash results in: | make[1]: Entering directory '/home/bigeasy/linux/~/scratch/mk-check' while bin/sh = bash expands the ~ properly before for O=. Would it be too much to ask, to expand the ~? Sebastian
On Mon, Jan 22, 2024 at 03:12:03PM +0100, Sebastian Andrzej Siewior wrote: > On 2023-12-16 01:06:37 [+0900], Masahiro Yamada wrote: > … > > Using the physical directory structure for the O= option seems more > > reasonable. > > > > The comment says "expand a shell special character '~'", but it has > > already been expanded to the home directory in the command line. > > It might have been expanded, it might have not been expanded. Having a > shell script: > | #!/bin/sh > | > | exec make O=~/scratch/mk-check defconfig > > with bin/sh = dash results in: > > | make[1]: Entering directory '/home/bigeasy/linux/~/scratch/mk-check' > > while bin/sh = bash expands the ~ properly before for O=. Would it be > too much to ask, to expand the ~? Expanding tilde expandos is traditionally a shell feature, as you already mentioned; and bash supports also expandos like '~+' and '~-'. I think, we should leave the shell things in shells. Thus, please update your shell scripts to be compliant to their interpreting shell (e.g. use '$HOME' or switch the shell). Kind regards, Nicolas
On 2024-01-22 15:59:37 [+0100], Nicolas Schier wrote: > Expanding tilde expandos is traditionally a shell feature, as you > already mentioned; and bash supports also expandos like '~+' and '~-'. > I think, we should leave the shell things in shells. > > Thus, please update your shell scripts to be compliant to their > interpreting shell (e.g. use '$HOME' or switch the shell). I reported that this change silently broke dash users. If you accept this then you could acknowledge it instead of suggesting workarounds. > Kind regards, > Nicolas Sebastian
On Mon, Jan 22, 2024 at 04:05:01PM +0100, Sebastian Andrzej Siewior wrote: > On 2024-01-22 15:59:37 [+0100], Nicolas Schier wrote: > > Expanding tilde expandos is traditionally a shell feature, as you > > already mentioned; and bash supports also expandos like '~+' and '~-'. > > I think, we should leave the shell things in shells. > > > > Thus, please update your shell scripts to be compliant to their > > interpreting shell (e.g. use '$HOME' or switch the shell). > > I reported that this change silently broke dash users. If you accept > this then you could acknowledge it instead of suggesting workarounds. I'm sorry, I didn't mean to upset you. Yes, we were aware that the change breaks some usage patterns. Thanks for pointing that out. Kind regards, Nicolas
From: Sebastian Andrzej Siewior > Sent: 22 January 2024 14:12 > > On 2023-12-16 01:06:37 [+0900], Masahiro Yamada wrote: > … > > Using the physical directory structure for the O= option seems more > > reasonable. > > > > The comment says "expand a shell special character '~'", but it has > > already been expanded to the home directory in the command line. > > It might have been expanded, it might have not been expanded. Having a > shell script: > | #!/bin/sh > | > | exec make O=~/scratch/mk-check defconfig > > with bin/sh = dash results in: > > | make[1]: Entering directory '/home/bigeasy/linux/~/scratch/mk-check' > > while bin/sh = bash expands the ~ properly before for O=. Would it be > too much to ask, to expand the ~? Raise a bug on dash... Or split the lines: O=~/xxxx make O="$O" ... David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Mon, Jan 22, 2024 at 11:12 PM Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: > > On 2023-12-16 01:06:37 [+0900], Masahiro Yamada wrote: > … > > Using the physical directory structure for the O= option seems more > > reasonable. > > > > The comment says "expand a shell special character '~'", but it has > > already been expanded to the home directory in the command line. > > It might have been expanded, it might have not been expanded. Having a > shell script: > | #!/bin/sh > | > | exec make O=~/scratch/mk-check defconfig > > with bin/sh = dash results in: > > | make[1]: Entering directory '/home/bigeasy/linux/~/scratch/mk-check' > > while bin/sh = bash expands the ~ properly before for O=. Would it be > too much to ask, to expand the ~? Not only O=. If the shell does not expand the '~' character, there are more variables that do not work as expected. For example, $ make CROSS_COMPILE=~/path/to/compiler/dir $ make M=~/path/to/external/module/dir' It is strange to require only O= to expand the '~' character. So, Kbuild should be agnostic about '~'. This is consistent. > > Sebastian >
diff --git a/Makefile b/Makefile index 04a86c81b5c0..5036edc40f46 100644 --- a/Makefile +++ b/Makefile @@ -190,14 +190,11 @@ ifeq ("$(origin O)", "command line") endif ifneq ($(KBUILD_OUTPUT),) -# Make's built-in functions such as $(abspath ...), $(realpath ...) cannot -# expand a shell special character '~'. We use a somewhat tedious way here. -abs_objtree := $(shell mkdir -p $(KBUILD_OUTPUT) && cd $(KBUILD_OUTPUT) && pwd) -$(if $(abs_objtree),, \ - $(error failed to create output directory "$(KBUILD_OUTPUT)")) - +# $(realpath ...) gets empty if the path does not exist. Run 'mkdir -p' first. +$(shell mkdir -p "$(KBUILD_OUTPUT)") # $(realpath ...) resolves symlinks -abs_objtree := $(realpath $(abs_objtree)) +abs_objtree := $(realpath $(KBUILD_OUTPUT)) +$(if $(abs_objtree),,$(error failed to create output directory "$(KBUILD_OUTPUT)")) endif # ifneq ($(KBUILD_OUTPUT),) ifneq ($(words $(subst :, ,$(abs_srctree))), 1)
Currently, Kbuild follows the logical chain of directories for the O= option, just like 'cd' (or 'realpath --logical') does. Example: $ mkdir -p /tmp/a /tmp/x/y $ ln -s /tmp/x/y /tmp/a/b $ realpath /tmp/a/b/.. /tmp/x $ realpath --logical /tmp/a/b/.. /tmp/a $ make O=/tmp/a/b/.. defconfig make[1]: Entering directory '/tmp/a' [snip] make[1]: Leaving directory '/tmp/a' 'make O=/tmp/a/b/.. defconfig' creates the kernel configuration in /tmp/a instead of /tmp/x despite /tmp/a/b/.. resolves to the physical directory path /tmp/x. This is because Kbuild internally uses the 'cd ... && pwd' for the path resolution, but this behavior is not predictable for users. Additionally, it is not consistent with how the Kbuild handles the M= option or GNU Make works with 'make -C /tmp/a/b/..'. Using the physical directory structure for the O= option seems more reasonable. The comment says "expand a shell special character '~'", but it has already been expanded to the home directory in the command line. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nicolas Schier <n.schier@avm.de> --- Changes in v2: - Quote $(KBUILD_OUTPUT). If ~ is contained in KBUILD_OUTPUT, it should not expanded any further. Makefile | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)