* [PATCH 0/2] tools/bootconfig: Add O= option and enhance error message
@ 2020-03-03 11:24 Masami Hiramatsu
2020-03-03 11:24 ` [PATCH 1/2] bootconfig: Support O=<builddir> option Masami Hiramatsu
2020-03-03 11:24 ` [PATCH 2/2] tools/bootconfig: Show line and column in parse error Masami Hiramatsu
0 siblings, 2 replies; 20+ messages in thread
From: Masami Hiramatsu @ 2020-03-03 11:24 UTC (permalink / raw)
To: Steven Rostedt
Cc: Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar,
Randy Dunlap, Andrew Morton, Masami Hiramatsu, Peter Zijlstra
Hi,
Here are patches to update tools/bootconfig to add O= option
support and enhance error message so that user can easily
locate the error position.
Thank you,
---
Masami Hiramatsu (2):
bootconfig: Support O=<builddir> option
tools/bootconfig: Show line and column in parse error
include/linux/bootconfig.h | 3 ++-
init/main.c | 14 ++++++++++----
lib/bootconfig.c | 35 ++++++++++++++++++++++++++---------
tools/bootconfig/Makefile | 27 +++++++++++++++++----------
tools/bootconfig/main.c | 35 +++++++++++++++++++++++++++++++----
tools/bootconfig/test-bootconfig.sh | 14 ++++++++++----
6 files changed, 96 insertions(+), 32 deletions(-)
--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH 1/2] bootconfig: Support O=<builddir> option 2020-03-03 11:24 [PATCH 0/2] tools/bootconfig: Add O= option and enhance error message Masami Hiramatsu @ 2020-03-03 11:24 ` Masami Hiramatsu 2020-03-04 23:04 ` Randy Dunlap 2020-03-03 11:24 ` [PATCH 2/2] tools/bootconfig: Show line and column in parse error Masami Hiramatsu 1 sibling, 1 reply; 20+ messages in thread From: Masami Hiramatsu @ 2020-03-03 11:24 UTC (permalink / raw) To: Steven Rostedt Cc: Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar, Randy Dunlap, Andrew Morton, Masami Hiramatsu, Peter Zijlstra Support O=<builddir> option to build bootconfig tool in the other directory. As same as other tools, if you specify O=<builddir>, bootconfig command is build under <builddir>. Reported-by: Randy Dunlap <rdunlap@infradead.org> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- tools/bootconfig/Makefile | 27 +++++++++++++++++---------- tools/bootconfig/test-bootconfig.sh | 14 ++++++++++---- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/tools/bootconfig/Makefile b/tools/bootconfig/Makefile index a6146ac64458..da5975775337 100644 --- a/tools/bootconfig/Makefile +++ b/tools/bootconfig/Makefile @@ -1,23 +1,30 @@ # SPDX-License-Identifier: GPL-2.0 # Makefile for bootconfig command +include ../scripts/Makefile.include bindir ?= /usr/bin -HEADER = include/linux/bootconfig.h -CFLAGS = -Wall -g -I./include +ifeq ($(srctree),) +srctree := $(patsubst %/,%,$(dir $(CURDIR))) +srctree := $(patsubst %/,%,$(dir $(srctree))) +endif -PROGS = bootconfig +LIBSRC = $(srctree)/lib/bootconfig.c $(srctree)/include/linux/bootconfig.h +CFLAGS = -Wall -g -I$(CURDIR)/include -all: $(PROGS) +ALL_TARGETS := bootconfig +ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS)) -bootconfig: ../../lib/bootconfig.c main.c $(HEADER) +all: $(ALL_PROGRAMS) + +$(OUTPUT)bootconfig: main.c $(LIBSRC) $(CC) $(filter %.c,$^) $(CFLAGS) -o $@ -install: $(PROGS) - install bootconfig $(DESTDIR)$(bindir) +test: $(ALL_PROGRAMS) test-bootconfig.sh + ./test-bootconfig.sh $(OUTPUT) -test: bootconfig - ./test-bootconfig.sh +install: $(ALL_PROGRAMS) + install $(OUTPUT)bootconfig $(DESTDIR)$(bindir) clean: - $(RM) -f *.o bootconfig + $(RM) -f $(OUTPUT)*.o $(ALL_PROGRAMS) diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh index 1411f4c3454f..81b350ffd03f 100755 --- a/tools/bootconfig/test-bootconfig.sh +++ b/tools/bootconfig/test-bootconfig.sh @@ -3,9 +3,16 @@ echo "Boot config test script" -BOOTCONF=./bootconfig -INITRD=`mktemp initrd-XXXX` -TEMPCONF=`mktemp temp-XXXX.bconf` +if [ -d "$1" ]; then + TESTDIR=$1 +else + TESTDIR=. +fi +BOOTCONF=${TESTDIR}/bootconfig + +INITRD=`mktemp ${TESTDIR}/initrd-XXXX` +TEMPCONF=`mktemp ${TESTDIR}/temp-XXXX.bconf` +OUTFILE=`mktemp ${TESTDIR}/tempout-XXXX` NG=0 cleanup() { @@ -65,7 +72,6 @@ new_size=$(stat -c %s $INITRD) xpass test $new_size -eq $initrd_size echo "No error messge while applying" -OUTFILE=`mktemp tempout-XXXX` dd if=/dev/zero of=$INITRD bs=4096 count=1 printf " \0\0\0 \0\0\0" >> $INITRD $BOOTCONF -a $TEMPCONF $INITRD > $OUTFILE 2>&1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] bootconfig: Support O=<builddir> option 2020-03-03 11:24 ` [PATCH 1/2] bootconfig: Support O=<builddir> option Masami Hiramatsu @ 2020-03-04 23:04 ` Randy Dunlap 2020-03-05 2:05 ` Masami Hiramatsu 2020-03-05 3:17 ` Steven Rostedt 0 siblings, 2 replies; 20+ messages in thread From: Randy Dunlap @ 2020-03-04 23:04 UTC (permalink / raw) To: Masami Hiramatsu, Steven Rostedt Cc: Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar, Andrew Morton, Peter Zijlstra On 3/3/20 3:24 AM, Masami Hiramatsu wrote: > Support O=<builddir> option to build bootconfig tool in > the other directory. As same as other tools, if you specify > O=<builddir>, bootconfig command is build under <builddir>. Hm. If I use $ make O=~/tmp -C tools/bootconfig that works: it builds bootconfig in ~/tmp. OTOH, if I sit at the top of the kernel source tree and I enter $ mkdir builddir $ make O=builddir -C tools/bootconfig I get this: make: Entering directory '/home/rdunlap/lnx/next/linux-next-20200304/tools/bootconfig' ../scripts/Makefile.include:4: *** O=builddir does not exist. Stop. make: Leaving directory '/home/rdunlap/lnx/next/linux-next-20200304/tools/bootconfig' so it looks like tools/scripts/Makefile.include doesn't handle this case correctly (which is how I do all of my builds). > Reported-by: Randy Dunlap <rdunlap@infradead.org> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > --- > tools/bootconfig/Makefile | 27 +++++++++++++++++---------- > tools/bootconfig/test-bootconfig.sh | 14 ++++++++++---- > 2 files changed, 27 insertions(+), 14 deletions(-) > > diff --git a/tools/bootconfig/Makefile b/tools/bootconfig/Makefile > index a6146ac64458..da5975775337 100644 > --- a/tools/bootconfig/Makefile > +++ b/tools/bootconfig/Makefile > @@ -1,23 +1,30 @@ > # SPDX-License-Identifier: GPL-2.0 > # Makefile for bootconfig command > +include ../scripts/Makefile.include > > bindir ?= /usr/bin > > -HEADER = include/linux/bootconfig.h > -CFLAGS = -Wall -g -I./include > +ifeq ($(srctree),) > +srctree := $(patsubst %/,%,$(dir $(CURDIR))) > +srctree := $(patsubst %/,%,$(dir $(srctree))) > +endif > > -PROGS = bootconfig > +LIBSRC = $(srctree)/lib/bootconfig.c $(srctree)/include/linux/bootconfig.h > +CFLAGS = -Wall -g -I$(CURDIR)/include > > -all: $(PROGS) > +ALL_TARGETS := bootconfig > +ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS)) > > -bootconfig: ../../lib/bootconfig.c main.c $(HEADER) > +all: $(ALL_PROGRAMS) > + > +$(OUTPUT)bootconfig: main.c $(LIBSRC) > $(CC) $(filter %.c,$^) $(CFLAGS) -o $@ > > -install: $(PROGS) > - install bootconfig $(DESTDIR)$(bindir) > +test: $(ALL_PROGRAMS) test-bootconfig.sh > + ./test-bootconfig.sh $(OUTPUT) > > -test: bootconfig > - ./test-bootconfig.sh > +install: $(ALL_PROGRAMS) > + install $(OUTPUT)bootconfig $(DESTDIR)$(bindir) > > clean: > - $(RM) -f *.o bootconfig > + $(RM) -f $(OUTPUT)*.o $(ALL_PROGRAMS) > diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh > index 1411f4c3454f..81b350ffd03f 100755 > --- a/tools/bootconfig/test-bootconfig.sh > +++ b/tools/bootconfig/test-bootconfig.sh > @@ -3,9 +3,16 @@ > > echo "Boot config test script" > > -BOOTCONF=./bootconfig > -INITRD=`mktemp initrd-XXXX` > -TEMPCONF=`mktemp temp-XXXX.bconf` > +if [ -d "$1" ]; then > + TESTDIR=$1 > +else > + TESTDIR=. > +fi > +BOOTCONF=${TESTDIR}/bootconfig > + > +INITRD=`mktemp ${TESTDIR}/initrd-XXXX` > +TEMPCONF=`mktemp ${TESTDIR}/temp-XXXX.bconf` > +OUTFILE=`mktemp ${TESTDIR}/tempout-XXXX` > NG=0 > > cleanup() { > @@ -65,7 +72,6 @@ new_size=$(stat -c %s $INITRD) > xpass test $new_size -eq $initrd_size > > echo "No error messge while applying" > -OUTFILE=`mktemp tempout-XXXX` > dd if=/dev/zero of=$INITRD bs=4096 count=1 > printf " \0\0\0 \0\0\0" >> $INITRD > $BOOTCONF -a $TEMPCONF $INITRD > $OUTFILE 2>&1 > -- ~Randy Reported-by: Randy Dunlap <rdunlap@infradead.org> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] bootconfig: Support O=<builddir> option 2020-03-04 23:04 ` Randy Dunlap @ 2020-03-05 2:05 ` Masami Hiramatsu 2020-03-05 3:17 ` Steven Rostedt 1 sibling, 0 replies; 20+ messages in thread From: Masami Hiramatsu @ 2020-03-05 2:05 UTC (permalink / raw) To: Randy Dunlap Cc: Steven Rostedt, Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar, Andrew Morton, Peter Zijlstra On Wed, 4 Mar 2020 15:04:43 -0800 Randy Dunlap <rdunlap@infradead.org> wrote: > On 3/3/20 3:24 AM, Masami Hiramatsu wrote: > > Support O=<builddir> option to build bootconfig tool in > > the other directory. As same as other tools, if you specify > > O=<builddir>, bootconfig command is build under <builddir>. > > Hm. If I use > $ make O=~/tmp -C tools/bootconfig > > that works: it builds bootconfig in ~/tmp. > > OTOH, if I sit at the top of the kernel source tree > and I enter > $ mkdir builddir > $ make O=builddir -C tools/bootconfig > > I get this: > make: Entering directory '/home/rdunlap/lnx/next/linux-next-20200304/tools/bootconfig' > ../scripts/Makefile.include:4: *** O=builddir does not exist. Stop. > make: Leaving directory '/home/rdunlap/lnx/next/linux-next-20200304/tools/bootconfig' > > so it looks like tools/scripts/Makefile.include doesn't handle this case correctly > (which is how I do all of my builds). Yes, I think that should be fixed in another patch. What about below? diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include index ded7a950dc40..6d2f3a1b2249 100644 --- a/tools/scripts/Makefile.include +++ b/tools/scripts/Makefile.include @@ -1,8 +1,8 @@ # SPDX-License-Identifier: GPL-2.0 ifneq ($(O),) ifeq ($(origin O), command line) - dummy := $(if $(shell test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) - ABSOLUTE_O := $(shell cd $(O) ; pwd) + dummy := $(if $(shell cd $(PWD); test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) + ABSOLUTE_O := $(shell cd $(PWD); cd $(O) ; pwd) OUTPUT := $(ABSOLUTE_O)/$(if $(subdir),$(subdir)/) COMMAND_O := O=$(ABSOLUTE_O) ifeq ($(objtree),) Thank you, -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] bootconfig: Support O=<builddir> option 2020-03-04 23:04 ` Randy Dunlap 2020-03-05 2:05 ` Masami Hiramatsu @ 2020-03-05 3:17 ` Steven Rostedt 2020-03-05 4:52 ` Randy Dunlap 1 sibling, 1 reply; 20+ messages in thread From: Steven Rostedt @ 2020-03-05 3:17 UTC (permalink / raw) To: Randy Dunlap Cc: Masami Hiramatsu, Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar, Andrew Morton, Peter Zijlstra On Wed, 4 Mar 2020 15:04:43 -0800 Randy Dunlap <rdunlap@infradead.org> wrote: > On 3/3/20 3:24 AM, Masami Hiramatsu wrote: > > Support O=<builddir> option to build bootconfig tool in > > the other directory. As same as other tools, if you specify > > O=<builddir>, bootconfig command is build under <builddir>. > > Hm. If I use > $ make O=~/tmp -C tools/bootconfig > > that works: it builds bootconfig in ~/tmp. > > OTOH, if I sit at the top of the kernel source tree > and I enter > $ mkdir builddir > $ make O=builddir -C tools/bootconfig > > I get this: > make: Entering directory '/home/rdunlap/lnx/next/linux-next-20200304/tools/bootconfig' > ../scripts/Makefile.include:4: *** O=builddir does not exist. Stop. > make: Leaving directory '/home/rdunlap/lnx/next/linux-next-20200304/tools/bootconfig' > > so it looks like tools/scripts/Makefile.include doesn't handle this case correctly > (which is how I do all of my builds). > Do you build perf that way? $ mkdir buildir $ make O=buildir -C tools/perf/ make: Entering directory '/work/git/linux-test.git/tools/perf' BUILD: Doing 'make -j24' parallel build ../scripts/Makefile.include:4: *** O=/work/git/linux-test.git/tools/perf/buildir does not exist. Stop. make: *** [Makefile:70: all] Error 2 make: Leaving directory '/work/git/linux-test.git/tools/perf' -- Steve ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] bootconfig: Support O=<builddir> option 2020-03-05 3:17 ` Steven Rostedt @ 2020-03-05 4:52 ` Randy Dunlap 2020-03-05 6:03 ` [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option Masami Hiramatsu 2020-03-05 7:41 ` [PATCH 1/2] bootconfig: Support O=<builddir> option Geert Uytterhoeven 0 siblings, 2 replies; 20+ messages in thread From: Randy Dunlap @ 2020-03-05 4:52 UTC (permalink / raw) To: Steven Rostedt Cc: Masami Hiramatsu, Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar, Andrew Morton, Peter Zijlstra On 3/4/20 7:17 PM, Steven Rostedt wrote: > On Wed, 4 Mar 2020 15:04:43 -0800 > Randy Dunlap <rdunlap@infradead.org> wrote: > >> On 3/3/20 3:24 AM, Masami Hiramatsu wrote: >>> Support O=<builddir> option to build bootconfig tool in >>> the other directory. As same as other tools, if you specify >>> O=<builddir>, bootconfig command is build under <builddir>. >> >> Hm. If I use >> $ make O=~/tmp -C tools/bootconfig >> >> that works: it builds bootconfig in ~/tmp. >> >> OTOH, if I sit at the top of the kernel source tree >> and I enter >> $ mkdir builddir >> $ make O=builddir -C tools/bootconfig >> >> I get this: >> make: Entering directory '/home/rdunlap/lnx/next/linux-next-20200304/tools/bootconfig' >> ../scripts/Makefile.include:4: *** O=builddir does not exist. Stop. >> make: Leaving directory '/home/rdunlap/lnx/next/linux-next-20200304/tools/bootconfig' >> >> so it looks like tools/scripts/Makefile.include doesn't handle this case correctly >> (which is how I do all of my builds). >> > > Do you build perf that way? No. It should also be fixed. > $ mkdir buildir > $ make O=buildir -C tools/perf/ > make: Entering directory '/work/git/linux-test.git/tools/perf' > BUILD: Doing 'make -j24' parallel build > ../scripts/Makefile.include:4: *** O=/work/git/linux-test.git/tools/perf/buildir does not exist. Stop. > make: *** [Makefile:70: all] Error 2 > make: Leaving directory '/work/git/linux-test.git/tools/perf' > > -- Steve Thanks. -- ~Randy ^ permalink raw reply [flat|nested] 20+ messages in thread
* [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option 2020-03-05 4:52 ` Randy Dunlap @ 2020-03-05 6:03 ` Masami Hiramatsu 2020-03-05 14:51 ` Greg KH ` (2 more replies) 2020-03-05 7:41 ` [PATCH 1/2] bootconfig: Support O=<builddir> option Geert Uytterhoeven 1 sibling, 3 replies; 20+ messages in thread From: Masami Hiramatsu @ 2020-03-05 6:03 UTC (permalink / raw) To: Steven Rostedt Cc: Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar, Randy Dunlap, Andrew Morton, Masami Hiramatsu, Peter Zijlstra, stable, Sasha Levin When I compiled tools/bootconfig from top directory with -C option, the O= option didn't work correctly if I passed a relative path. $ make O=./builddir/ -C tools/bootconfig/ make: Entering directory '/home/mhiramat/ksrc/linux/tools/bootconfig' ../scripts/Makefile.include:4: *** O=./builddir/ does not exist. Stop. make: Leaving directory '/home/mhiramat/ksrc/linux/tools/bootconfig' The O= directory existence check failed because the check script ran in the build target directory instead of the directory where I ran the make command. To fix that, once change directory to $(PWD) and check O= directory, since the PWD is set to where the make command runs. Fixes: c883122acc0d ("perf tools: Let O= makes handle relative paths") Reported-by: Randy Dunlap <rdunlap@infradead.org> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- tools/scripts/Makefile.include | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include index ded7a950dc40..6d2f3a1b2249 100644 --- a/tools/scripts/Makefile.include +++ b/tools/scripts/Makefile.include @@ -1,8 +1,8 @@ # SPDX-License-Identifier: GPL-2.0 ifneq ($(O),) ifeq ($(origin O), command line) - dummy := $(if $(shell test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) - ABSOLUTE_O := $(shell cd $(O) ; pwd) + dummy := $(if $(shell cd $(PWD); test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) + ABSOLUTE_O := $(shell cd $(PWD); cd $(O) ; pwd) OUTPUT := $(ABSOLUTE_O)/$(if $(subdir),$(subdir)/) COMMAND_O := O=$(ABSOLUTE_O) ifeq ($(objtree),) ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option 2020-03-05 6:03 ` [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option Masami Hiramatsu @ 2020-03-05 14:51 ` Greg KH 2020-03-05 18:50 ` Randy Dunlap 2020-03-06 7:52 ` Geert Uytterhoeven 2 siblings, 0 replies; 20+ messages in thread From: Greg KH @ 2020-03-05 14:51 UTC (permalink / raw) To: Masami Hiramatsu Cc: Steven Rostedt, Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar, Randy Dunlap, Andrew Morton, Peter Zijlstra, stable, Sasha Levin On Thu, Mar 05, 2020 at 03:03:03PM +0900, Masami Hiramatsu wrote: > When I compiled tools/bootconfig from top directory with > -C option, the O= option didn't work correctly if I passed > a relative path. > > $ make O=./builddir/ -C tools/bootconfig/ > make: Entering directory '/home/mhiramat/ksrc/linux/tools/bootconfig' > ../scripts/Makefile.include:4: *** O=./builddir/ does not exist. Stop. > make: Leaving directory '/home/mhiramat/ksrc/linux/tools/bootconfig' > > The O= directory existence check failed because the check > script ran in the build target directory instead of the > directory where I ran the make command. > > To fix that, once change directory to $(PWD) and check O= > directory, since the PWD is set to where the make command > runs. > > Fixes: c883122acc0d ("perf tools: Let O= makes handle relative paths") > Reported-by: Randy Dunlap <rdunlap@infradead.org> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > --- > tools/scripts/Makefile.include | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) <formletter> This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly. </formletter> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option 2020-03-05 6:03 ` [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option Masami Hiramatsu 2020-03-05 14:51 ` Greg KH @ 2020-03-05 18:50 ` Randy Dunlap 2020-03-06 1:39 ` Masami Hiramatsu 2020-03-06 7:52 ` Geert Uytterhoeven 2 siblings, 1 reply; 20+ messages in thread From: Randy Dunlap @ 2020-03-05 18:50 UTC (permalink / raw) To: Masami Hiramatsu, Steven Rostedt Cc: Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar, Andrew Morton, Peter Zijlstra, stable, Sasha Levin On 3/4/20 10:03 PM, Masami Hiramatsu wrote: > When I compiled tools/bootconfig from top directory with > -C option, the O= option didn't work correctly if I passed > a relative path. > > $ make O=./builddir/ -C tools/bootconfig/ > make: Entering directory '/home/mhiramat/ksrc/linux/tools/bootconfig' > ../scripts/Makefile.include:4: *** O=./builddir/ does not exist. Stop. > make: Leaving directory '/home/mhiramat/ksrc/linux/tools/bootconfig' > > The O= directory existence check failed because the check > script ran in the build target directory instead of the > directory where I ran the make command. > > To fix that, once change directory to $(PWD) and check O= > directory, since the PWD is set to where the make command > runs. > > Fixes: c883122acc0d ("perf tools: Let O= makes handle relative paths") > Reported-by: Randy Dunlap <rdunlap@infradead.org> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> Hi Masami, This patch doesn't fix anything AFAICT. Didn't help in my testing. Thanks. > --- > tools/scripts/Makefile.include | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include > index ded7a950dc40..6d2f3a1b2249 100644 > --- a/tools/scripts/Makefile.include > +++ b/tools/scripts/Makefile.include > @@ -1,8 +1,8 @@ > # SPDX-License-Identifier: GPL-2.0 > ifneq ($(O),) > ifeq ($(origin O), command line) > - dummy := $(if $(shell test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) > - ABSOLUTE_O := $(shell cd $(O) ; pwd) > + dummy := $(if $(shell cd $(PWD); test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) > + ABSOLUTE_O := $(shell cd $(PWD); cd $(O) ; pwd) > OUTPUT := $(ABSOLUTE_O)/$(if $(subdir),$(subdir)/) > COMMAND_O := O=$(ABSOLUTE_O) > ifeq ($(objtree),) > -- ~Randy Reported-by: Randy Dunlap <rdunlap@infradead.org> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option 2020-03-05 18:50 ` Randy Dunlap @ 2020-03-06 1:39 ` Masami Hiramatsu 2020-03-06 5:17 ` Randy Dunlap 0 siblings, 1 reply; 20+ messages in thread From: Masami Hiramatsu @ 2020-03-06 1:39 UTC (permalink / raw) To: Randy Dunlap Cc: Steven Rostedt, Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar, Andrew Morton, Peter Zijlstra, stable, Sasha Levin On Thu, 5 Mar 2020 10:50:43 -0800 Randy Dunlap <rdunlap@infradead.org> wrote: > On 3/4/20 10:03 PM, Masami Hiramatsu wrote: > > When I compiled tools/bootconfig from top directory with > > -C option, the O= option didn't work correctly if I passed > > a relative path. > > > > $ make O=./builddir/ -C tools/bootconfig/ > > make: Entering directory '/home/mhiramat/ksrc/linux/tools/bootconfig' > > ../scripts/Makefile.include:4: *** O=./builddir/ does not exist. Stop. > > make: Leaving directory '/home/mhiramat/ksrc/linux/tools/bootconfig' > > > > The O= directory existence check failed because the check > > script ran in the build target directory instead of the > > directory where I ran the make command. > > > > To fix that, once change directory to $(PWD) and check O= > > directory, since the PWD is set to where the make command > > runs. > > > > Fixes: c883122acc0d ("perf tools: Let O= makes handle relative paths") > > Reported-by: Randy Dunlap <rdunlap@infradead.org> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > > Hi Masami, > > This patch doesn't fix anything AFAICT. > Didn't help in my testing. > Hmm, what happens on your case? Maybe PWD is not set? Thank you, > Thanks. > > > --- > > tools/scripts/Makefile.include | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include > > index ded7a950dc40..6d2f3a1b2249 100644 > > --- a/tools/scripts/Makefile.include > > +++ b/tools/scripts/Makefile.include > > @@ -1,8 +1,8 @@ > > # SPDX-License-Identifier: GPL-2.0 > > ifneq ($(O),) > > ifeq ($(origin O), command line) > > - dummy := $(if $(shell test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) > > - ABSOLUTE_O := $(shell cd $(O) ; pwd) > > + dummy := $(if $(shell cd $(PWD); test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) > > + ABSOLUTE_O := $(shell cd $(PWD); cd $(O) ; pwd) > > OUTPUT := $(ABSOLUTE_O)/$(if $(subdir),$(subdir)/) > > COMMAND_O := O=$(ABSOLUTE_O) > > ifeq ($(objtree),) > > > > > -- > ~Randy > Reported-by: Randy Dunlap <rdunlap@infradead.org> -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option 2020-03-06 1:39 ` Masami Hiramatsu @ 2020-03-06 5:17 ` Randy Dunlap 0 siblings, 0 replies; 20+ messages in thread From: Randy Dunlap @ 2020-03-06 5:17 UTC (permalink / raw) To: Masami Hiramatsu Cc: Steven Rostedt, Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar, Andrew Morton, Peter Zijlstra, stable, Sasha Levin On 3/5/20 5:39 PM, Masami Hiramatsu wrote: > On Thu, 5 Mar 2020 10:50:43 -0800 > Randy Dunlap <rdunlap@infradead.org> wrote: > >> On 3/4/20 10:03 PM, Masami Hiramatsu wrote: >>> When I compiled tools/bootconfig from top directory with >>> -C option, the O= option didn't work correctly if I passed >>> a relative path. >>> >>> $ make O=./builddir/ -C tools/bootconfig/ >>> make: Entering directory '/home/mhiramat/ksrc/linux/tools/bootconfig' >>> ../scripts/Makefile.include:4: *** O=./builddir/ does not exist. Stop. >>> make: Leaving directory '/home/mhiramat/ksrc/linux/tools/bootconfig' >>> >>> The O= directory existence check failed because the check >>> script ran in the build target directory instead of the >>> directory where I ran the make command. >>> >>> To fix that, once change directory to $(PWD) and check O= >>> directory, since the PWD is set to where the make command >>> runs. >>> >>> Fixes: c883122acc0d ("perf tools: Let O= makes handle relative paths") >>> Reported-by: Randy Dunlap <rdunlap@infradead.org> >>> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> >> >> Hi Masami, >> >> This patch doesn't fix anything AFAICT. >> Didn't help in my testing. >> > > Hmm, what happens on your case? Maybe PWD is not set? > > Thank you, The binary is still built in $srctree/tools/bootconfig/. If I 'echo $PWD' at a prompt, I get the correct pwd. > >> Thanks. >> >>> --- >>> tools/scripts/Makefile.include | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include >>> index ded7a950dc40..6d2f3a1b2249 100644 >>> --- a/tools/scripts/Makefile.include >>> +++ b/tools/scripts/Makefile.include >>> @@ -1,8 +1,8 @@ >>> # SPDX-License-Identifier: GPL-2.0 >>> ifneq ($(O),) >>> ifeq ($(origin O), command line) >>> - dummy := $(if $(shell test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) >>> - ABSOLUTE_O := $(shell cd $(O) ; pwd) >>> + dummy := $(if $(shell cd $(PWD); test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) >>> + ABSOLUTE_O := $(shell cd $(PWD); cd $(O) ; pwd) >>> OUTPUT := $(ABSOLUTE_O)/$(if $(subdir),$(subdir)/) >>> COMMAND_O := O=$(ABSOLUTE_O) >>> ifeq ($(objtree),) -- ~Randy ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option 2020-03-05 6:03 ` [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option Masami Hiramatsu 2020-03-05 14:51 ` Greg KH 2020-03-05 18:50 ` Randy Dunlap @ 2020-03-06 7:52 ` Geert Uytterhoeven 2020-03-06 15:07 ` Masami Hiramatsu 2020-03-06 16:45 ` Steven Rostedt 2 siblings, 2 replies; 20+ messages in thread From: Geert Uytterhoeven @ 2020-03-06 7:52 UTC (permalink / raw) To: Masami Hiramatsu Cc: Steven Rostedt, Borislav Petkov, LKML, Ingo Molnar, Randy Dunlap, Andrew Morton, Peter Zijlstra, Sasha Levin CC +kbuild, -stable On Thu, Mar 5, 2020 at 7:03 AM Masami Hiramatsu <mhiramat@kernel.org> wrote: > When I compiled tools/bootconfig from top directory with > -C option, the O= option didn't work correctly if I passed > a relative path. > > $ make O=./builddir/ -C tools/bootconfig/ > make: Entering directory '/home/mhiramat/ksrc/linux/tools/bootconfig' > ../scripts/Makefile.include:4: *** O=./builddir/ does not exist. Stop. > make: Leaving directory '/home/mhiramat/ksrc/linux/tools/bootconfig' > > The O= directory existence check failed because the check > script ran in the build target directory instead of the > directory where I ran the make command. > > To fix that, once change directory to $(PWD) and check O= > directory, since the PWD is set to where the make command > runs. > > Fixes: c883122acc0d ("perf tools: Let O= makes handle relative paths") > Reported-by: Randy Dunlap <rdunlap@infradead.org> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > --- > tools/scripts/Makefile.include | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include > index ded7a950dc40..6d2f3a1b2249 100644 > --- a/tools/scripts/Makefile.include > +++ b/tools/scripts/Makefile.include > @@ -1,8 +1,8 @@ > # SPDX-License-Identifier: GPL-2.0 > ifneq ($(O),) > ifeq ($(origin O), command line) > - dummy := $(if $(shell test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) > - ABSOLUTE_O := $(shell cd $(O) ; pwd) > + dummy := $(if $(shell cd $(PWD); test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) > + ABSOLUTE_O := $(shell cd $(PWD); cd $(O) ; pwd) > OUTPUT := $(ABSOLUTE_O)/$(if $(subdir),$(subdir)/) > COMMAND_O := O=$(ABSOLUTE_O) > ifeq ($(objtree),) > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option 2020-03-06 7:52 ` Geert Uytterhoeven @ 2020-03-06 15:07 ` Masami Hiramatsu 2020-03-06 16:26 ` Randy Dunlap 2020-03-06 16:45 ` Steven Rostedt 1 sibling, 1 reply; 20+ messages in thread From: Masami Hiramatsu @ 2020-03-06 15:07 UTC (permalink / raw) To: Geert Uytterhoeven, Randy Dunlap Cc: Steven Rostedt, Borislav Petkov, LKML, Ingo Molnar, Randy Dunlap, Andrew Morton, Peter Zijlstra, Sasha Levin Thanks Geert, So Randy, what you will get if you add "echo $(PWD)" instead of "cd $(PWD)" ? Is that still empty or shows the tools/bootconfig directory? Thanks, On Fri, 6 Mar 2020 08:52:40 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote: > CC +kbuild, -stable > > On Thu, Mar 5, 2020 at 7:03 AM Masami Hiramatsu <mhiramat@kernel.org> wrote: > > When I compiled tools/bootconfig from top directory with > > -C option, the O= option didn't work correctly if I passed > > a relative path. > > > > $ make O=./builddir/ -C tools/bootconfig/ > > make: Entering directory '/home/mhiramat/ksrc/linux/tools/bootconfig' > > ../scripts/Makefile.include:4: *** O=./builddir/ does not exist. Stop. > > make: Leaving directory '/home/mhiramat/ksrc/linux/tools/bootconfig' > > > > The O= directory existence check failed because the check > > script ran in the build target directory instead of the > > directory where I ran the make command. > > > > To fix that, once change directory to $(PWD) and check O= > > directory, since the PWD is set to where the make command > > runs. > > > > Fixes: c883122acc0d ("perf tools: Let O= makes handle relative paths") > > Reported-by: Randy Dunlap <rdunlap@infradead.org> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > > --- > > tools/scripts/Makefile.include | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include > > index ded7a950dc40..6d2f3a1b2249 100644 > > --- a/tools/scripts/Makefile.include > > +++ b/tools/scripts/Makefile.include > > @@ -1,8 +1,8 @@ > > # SPDX-License-Identifier: GPL-2.0 > > ifneq ($(O),) > > ifeq ($(origin O), command line) > > - dummy := $(if $(shell test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) > > - ABSOLUTE_O := $(shell cd $(O) ; pwd) > > + dummy := $(if $(shell cd $(PWD); test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) > > + ABSOLUTE_O := $(shell cd $(PWD); cd $(O) ; pwd) > > OUTPUT := $(ABSOLUTE_O)/$(if $(subdir),$(subdir)/) > > COMMAND_O := O=$(ABSOLUTE_O) > > ifeq ($(objtree),) > > -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option 2020-03-06 15:07 ` Masami Hiramatsu @ 2020-03-06 16:26 ` Randy Dunlap 2020-03-06 18:10 ` Masami Hiramatsu 0 siblings, 1 reply; 20+ messages in thread From: Randy Dunlap @ 2020-03-06 16:26 UTC (permalink / raw) To: Masami Hiramatsu, Geert Uytterhoeven Cc: Steven Rostedt, Borislav Petkov, LKML, Ingo Molnar, Andrew Morton, Peter Zijlstra, Sasha Levin On 3/6/20 7:07 AM, Masami Hiramatsu wrote: > Thanks Geert, > > So Randy, what you will get if you add "echo $(PWD)" instead of "cd $(PWD)" ? > Is that still empty or shows the tools/bootconfig directory? > > Thanks, OK, in these lines: + dummy := $(if $(shell cd $(PWD); test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) + ABSOLUTE_O := $(shell cd $(PWD); cd $(O) ; pwd) I changed both "cd $(PWD)" to "echo $(PWD)" and did $ make O=BUILD -C tools/bootconfig/ and this is the build log: make: Entering directory '/home/rdunlap/lnx/next/linux-next-20200306/tools/bootconfig' cc ../../lib/bootconfig.c main.c -Wall -g -I./include -o bootconfig make: Leaving directory '/home/rdunlap/lnx/next/linux-next-20200306/tools/bootconfig' Does that help? Thanks. > On Fri, 6 Mar 2020 08:52:40 +0100 > Geert Uytterhoeven <geert@linux-m68k.org> wrote: > >> CC +kbuild, -stable >> >> On Thu, Mar 5, 2020 at 7:03 AM Masami Hiramatsu <mhiramat@kernel.org> wrote: >>> When I compiled tools/bootconfig from top directory with >>> -C option, the O= option didn't work correctly if I passed >>> a relative path. >>> >>> $ make O=./builddir/ -C tools/bootconfig/ >>> make: Entering directory '/home/mhiramat/ksrc/linux/tools/bootconfig' >>> ../scripts/Makefile.include:4: *** O=./builddir/ does not exist. Stop. >>> make: Leaving directory '/home/mhiramat/ksrc/linux/tools/bootconfig' >>> >>> The O= directory existence check failed because the check >>> script ran in the build target directory instead of the >>> directory where I ran the make command. >>> >>> To fix that, once change directory to $(PWD) and check O= >>> directory, since the PWD is set to where the make command >>> runs. >>> >>> Fixes: c883122acc0d ("perf tools: Let O= makes handle relative paths") >>> Reported-by: Randy Dunlap <rdunlap@infradead.org> >>> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> >>> --- >>> tools/scripts/Makefile.include | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include >>> index ded7a950dc40..6d2f3a1b2249 100644 >>> --- a/tools/scripts/Makefile.include >>> +++ b/tools/scripts/Makefile.include >>> @@ -1,8 +1,8 @@ >>> # SPDX-License-Identifier: GPL-2.0 >>> ifneq ($(O),) >>> ifeq ($(origin O), command line) >>> - dummy := $(if $(shell test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) >>> - ABSOLUTE_O := $(shell cd $(O) ; pwd) >>> + dummy := $(if $(shell cd $(PWD); test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) >>> + ABSOLUTE_O := $(shell cd $(PWD); cd $(O) ; pwd) >>> OUTPUT := $(ABSOLUTE_O)/$(if $(subdir),$(subdir)/) >>> COMMAND_O := O=$(ABSOLUTE_O) >>> ifeq ($(objtree),) >>> > > -- ~Randy Reported-by: Randy Dunlap <rdunlap@infradead.org> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option 2020-03-06 16:26 ` Randy Dunlap @ 2020-03-06 18:10 ` Masami Hiramatsu 2020-03-06 18:26 ` Randy Dunlap 0 siblings, 1 reply; 20+ messages in thread From: Masami Hiramatsu @ 2020-03-06 18:10 UTC (permalink / raw) To: Randy Dunlap Cc: Geert Uytterhoeven, Steven Rostedt, Borislav Petkov, LKML, Ingo Molnar, Andrew Morton, Peter Zijlstra, Sasha Levin On Fri, 6 Mar 2020 08:26:43 -0800 Randy Dunlap <rdunlap@infradead.org> wrote: > On 3/6/20 7:07 AM, Masami Hiramatsu wrote: > > Thanks Geert, > > > > So Randy, what you will get if you add "echo $(PWD)" instead of "cd $(PWD)" ? > > Is that still empty or shows the tools/bootconfig directory? > > > > Thanks, > > OK, in these lines: > + dummy := $(if $(shell cd $(PWD); test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) > + ABSOLUTE_O := $(shell cd $(PWD); cd $(O) ; pwd) > > I changed both "cd $(PWD)" to "echo $(PWD)" and did > $ make O=BUILD -C tools/bootconfig/ > > and this is the build log: > > make: Entering directory '/home/rdunlap/lnx/next/linux-next-20200306/tools/bootconfig' > cc ../../lib/bootconfig.c main.c -Wall -g -I./include -o bootconfig > make: Leaving directory '/home/rdunlap/lnx/next/linux-next-20200306/tools/bootconfig' > > > Does that help? Hmm, did you apply "[PATCH 1/2] bootconfig: Support O=<builddir> option" too? Also, I found this is not enough for perf. perf does more tricky thing in its Makefile. Thank you, -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option 2020-03-06 18:10 ` Masami Hiramatsu @ 2020-03-06 18:26 ` Randy Dunlap 0 siblings, 0 replies; 20+ messages in thread From: Randy Dunlap @ 2020-03-06 18:26 UTC (permalink / raw) To: Masami Hiramatsu Cc: Geert Uytterhoeven, Steven Rostedt, Borislav Petkov, LKML, Ingo Molnar, Andrew Morton, Peter Zijlstra, Sasha Levin On 3/6/20 10:10 AM, Masami Hiramatsu wrote: > On Fri, 6 Mar 2020 08:26:43 -0800 > Randy Dunlap <rdunlap@infradead.org> wrote: > >> On 3/6/20 7:07 AM, Masami Hiramatsu wrote: >>> Thanks Geert, >>> >>> So Randy, what you will get if you add "echo $(PWD)" instead of "cd $(PWD)" ? >>> Is that still empty or shows the tools/bootconfig directory? >>> >>> Thanks, >> >> OK, in these lines: >> + dummy := $(if $(shell cd $(PWD); test -d $(O) || echo $(O)),$(error O=$(O) does not exist),) >> + ABSOLUTE_O := $(shell cd $(PWD); cd $(O) ; pwd) >> >> I changed both "cd $(PWD)" to "echo $(PWD)" and did >> $ make O=BUILD -C tools/bootconfig/ >> >> and this is the build log: >> >> make: Entering directory '/home/rdunlap/lnx/next/linux-next-20200306/tools/bootconfig' >> cc ../../lib/bootconfig.c main.c -Wall -g -I./include -o bootconfig >> make: Leaving directory '/home/rdunlap/lnx/next/linux-next-20200306/tools/bootconfig' >> >> >> Does that help? > > Hmm, did you apply "[PATCH 1/2] bootconfig: Support O=<builddir> option" too? oh crud. nope. Sorry about that. Building bootconfig with O=BUILD is now working. Thanks for your time and patience. > Also, I found this is not enough for perf. perf does more tricky thing in its Makefile. Ack. -- ~Randy ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option 2020-03-06 7:52 ` Geert Uytterhoeven 2020-03-06 15:07 ` Masami Hiramatsu @ 2020-03-06 16:45 ` Steven Rostedt 1 sibling, 0 replies; 20+ messages in thread From: Steven Rostedt @ 2020-03-06 16:45 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Masami Hiramatsu, Borislav Petkov, LKML, Ingo Molnar, Randy Dunlap, Andrew Morton, Peter Zijlstra, Sasha Levin On Fri, 6 Mar 2020 08:52:40 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote: > CC +kbuild, -stable Thank you, as these are kbuild specific changes and not bootconfig (as building anything in tools doesn't work here with top level O= command). Masami, any patches you send for this, they should go to the kbuild maintainers. -- Steve ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] bootconfig: Support O=<builddir> option 2020-03-05 4:52 ` Randy Dunlap 2020-03-05 6:03 ` [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option Masami Hiramatsu @ 2020-03-05 7:41 ` Geert Uytterhoeven 2020-03-05 18:51 ` Randy Dunlap 1 sibling, 1 reply; 20+ messages in thread From: Geert Uytterhoeven @ 2020-03-05 7:41 UTC (permalink / raw) To: Randy Dunlap Cc: Steven Rostedt, Masami Hiramatsu, Borislav Petkov, LKML, Ingo Molnar, Andrew Morton, Peter Zijlstra Hi Randy, On Thu, Mar 5, 2020 at 5:53 AM Randy Dunlap <rdunlap@infradead.org> wrote: > On 3/4/20 7:17 PM, Steven Rostedt wrote: > > On Wed, 4 Mar 2020 15:04:43 -0800 > > Randy Dunlap <rdunlap@infradead.org> wrote: > > > >> On 3/3/20 3:24 AM, Masami Hiramatsu wrote: > >>> Support O=<builddir> option to build bootconfig tool in > >>> the other directory. As same as other tools, if you specify > >>> O=<builddir>, bootconfig command is build under <builddir>. > >> > >> Hm. If I use > >> $ make O=~/tmp -C tools/bootconfig > >> > >> that works: it builds bootconfig in ~/tmp. > >> > >> OTOH, if I sit at the top of the kernel source tree > >> and I enter > >> $ mkdir builddir > >> $ make O=builddir -C tools/bootconfig > >> > >> I get this: > >> make: Entering directory '/home/rdunlap/lnx/next/linux-next-20200304/tools/bootconfig' > >> ../scripts/Makefile.include:4: *** O=builddir does not exist. Stop. > >> make: Leaving directory '/home/rdunlap/lnx/next/linux-next-20200304/tools/bootconfig' > >> > >> so it looks like tools/scripts/Makefile.include doesn't handle this case correctly > >> (which is how I do all of my builds). > >> > > > > Do you build perf that way? > > No. It should also be fixed. There are lots of issues when (cross)building the tools and selftest with O=. I tried to fix some of them a while ago, but I lost interest. https://lore.kernel.org/lkml/20190114135144.26096-1-geert+renesas@glider.be/ The only thing you can rely on when (cross)building with O=, is the kernel itself ;-) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/2] bootconfig: Support O=<builddir> option 2020-03-05 7:41 ` [PATCH 1/2] bootconfig: Support O=<builddir> option Geert Uytterhoeven @ 2020-03-05 18:51 ` Randy Dunlap 0 siblings, 0 replies; 20+ messages in thread From: Randy Dunlap @ 2020-03-05 18:51 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Steven Rostedt, Masami Hiramatsu, Borislav Petkov, LKML, Ingo Molnar, Andrew Morton, Peter Zijlstra On 3/4/20 11:41 PM, Geert Uytterhoeven wrote: > Hi Randy, > > On Thu, Mar 5, 2020 at 5:53 AM Randy Dunlap <rdunlap@infradead.org> wrote: >> On 3/4/20 7:17 PM, Steven Rostedt wrote: >>> On Wed, 4 Mar 2020 15:04:43 -0800 >>> Randy Dunlap <rdunlap@infradead.org> wrote: >>> >>>> On 3/3/20 3:24 AM, Masami Hiramatsu wrote: >>>>> Support O=<builddir> option to build bootconfig tool in >>>>> the other directory. As same as other tools, if you specify >>>>> O=<builddir>, bootconfig command is build under <builddir>. >>>> >>>> Hm. If I use >>>> $ make O=~/tmp -C tools/bootconfig >>>> >>>> that works: it builds bootconfig in ~/tmp. >>>> >>>> OTOH, if I sit at the top of the kernel source tree >>>> and I enter >>>> $ mkdir builddir >>>> $ make O=builddir -C tools/bootconfig >>>> >>>> I get this: >>>> make: Entering directory '/home/rdunlap/lnx/next/linux-next-20200304/tools/bootconfig' >>>> ../scripts/Makefile.include:4: *** O=builddir does not exist. Stop. >>>> make: Leaving directory '/home/rdunlap/lnx/next/linux-next-20200304/tools/bootconfig' >>>> >>>> so it looks like tools/scripts/Makefile.include doesn't handle this case correctly >>>> (which is how I do all of my builds). >>>> >>> >>> Do you build perf that way? >> >> No. It should also be fixed. > > There are lots of issues when (cross)building the tools and selftest with O=. > I tried to fix some of them a while ago, but I lost interest. > https://lore.kernel.org/lkml/20190114135144.26096-1-geert+renesas@glider.be/ > > The only thing you can rely on when (cross)building with O=, is the kernel > itself ;-) Yeah, oh well. I'm not ready to just give up on it though. thanks. -- ~Randy ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] tools/bootconfig: Show line and column in parse error 2020-03-03 11:24 [PATCH 0/2] tools/bootconfig: Add O= option and enhance error message Masami Hiramatsu 2020-03-03 11:24 ` [PATCH 1/2] bootconfig: Support O=<builddir> option Masami Hiramatsu @ 2020-03-03 11:24 ` Masami Hiramatsu 1 sibling, 0 replies; 20+ messages in thread From: Masami Hiramatsu @ 2020-03-03 11:24 UTC (permalink / raw) To: Steven Rostedt Cc: Geert Uytterhoeven, Borislav Petkov, LKML, Ingo Molnar, Randy Dunlap, Andrew Morton, Masami Hiramatsu, Peter Zijlstra Show line and column when we got a parse error in bootconfig tool. Current lib/bootconfig shows the parse error with byte offset, but that is not human readable. This makes xbc_init() not showing error message itself but able to pass the error message and position to caller, so that the caller can decode it and show the error message with line number and columns. With this patch, bootconfig tool shows an error with line:column as below. $ cat samples/bad-dotword.bconf # do not start keyword with . key { .word = 1 } $ ./bootconfig -a samples/bad-dotword.bconf initrd Parse Error: Invalid keyword at 3:3 Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- include/linux/bootconfig.h | 3 ++- init/main.c | 14 ++++++++++---- lib/bootconfig.c | 35 ++++++++++++++++++++++++++--------- tools/bootconfig/main.c | 35 +++++++++++++++++++++++++++++++---- 4 files changed, 69 insertions(+), 18 deletions(-) diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h index d11e183fcb54..9903088891fa 100644 --- a/include/linux/bootconfig.h +++ b/include/linux/bootconfig.h @@ -216,7 +216,8 @@ static inline int __init xbc_node_compose_key(struct xbc_node *node, } /* XBC node initializer */ -int __init xbc_init(char *buf); +int __init xbc_init(char *buf, const char **emsg, int *epos); + /* XBC cleanup data structures */ void __init xbc_destroy_all(void); diff --git a/init/main.c b/init/main.c index c9b1ee6bbb8d..c749da3ce070 100644 --- a/init/main.c +++ b/init/main.c @@ -353,6 +353,8 @@ static int __init bootconfig_params(char *param, char *val, static void __init setup_boot_config(const char *cmdline) { static char tmp_cmdline[COMMAND_LINE_SIZE] __initdata; + const char *msg; + int pos; u32 size, csum; char *data, *copy; u32 *hdr; @@ -400,10 +402,14 @@ static void __init setup_boot_config(const char *cmdline) memcpy(copy, data, size); copy[size] = '\0'; - ret = xbc_init(copy); - if (ret < 0) - pr_err("Failed to parse bootconfig\n"); - else { + ret = xbc_init(copy, &msg, &pos); + if (ret < 0) { + if (pos < 0) + pr_err("Failed to init bootconfig: %s.\n", msg); + else + pr_err("Failed to parse bootconfig: %s at %d.\n", + msg, pos); + } else { pr_info("Load bootconfig: %d bytes %d nodes\n", size, ret); /* keys starting with "kernel." are passed via cmdline */ extra_command_line = xbc_make_cmdline("kernel"); diff --git a/lib/bootconfig.c b/lib/bootconfig.c index ec3ce7fd299f..912ef4921398 100644 --- a/lib/bootconfig.c +++ b/lib/bootconfig.c @@ -29,12 +29,14 @@ static int xbc_node_num __initdata; static char *xbc_data __initdata; static size_t xbc_data_size __initdata; static struct xbc_node *last_parent __initdata; +static const char *xbc_err_msg __initdata; +static int xbc_err_pos __initdata; static int __init xbc_parse_error(const char *msg, const char *p) { - int pos = p - xbc_data; + xbc_err_msg = msg; + xbc_err_pos = (int)(p - xbc_data); - pr_err("Parse error at pos %d: %s\n", pos, msg); return -EINVAL; } @@ -738,33 +740,44 @@ void __init xbc_destroy_all(void) /** * xbc_init() - Parse given XBC file and build XBC internal tree * @buf: boot config text + * @emsg: A pointer of const char * to store the error message + * @epos: A pointer of int to store the error position * * This parses the boot config text in @buf. @buf must be a * null terminated string and smaller than XBC_DATA_MAX. * Return the number of stored nodes (>0) if succeeded, or -errno * if there is any error. + * In error cases, @emsg will be updated with an error message and + * @epos will be updated with the error position which is the byte offset + * of @buf. If the error is not a parser error, @epos will be -1. */ -int __init xbc_init(char *buf) +int __init xbc_init(char *buf, const char **emsg, int *epos) { char *p, *q; int ret, c; + if (epos) + *epos = -1; + if (xbc_data) { - pr_err("Error: bootconfig is already initialized.\n"); + if (emsg) + *emsg = "Bootconfig is already initialized"; return -EBUSY; } ret = strlen(buf); if (ret > XBC_DATA_MAX - 1 || ret == 0) { - pr_err("Error: Config data is %s.\n", - ret ? "too big" : "empty"); + if (emsg) + *emsg = ret ? "Config data is too big" : + "Config data is empty"; return -ERANGE; } xbc_nodes = memblock_alloc(sizeof(struct xbc_node) * XBC_NODE_MAX, SMP_CACHE_BYTES); if (!xbc_nodes) { - pr_err("Failed to allocate memory for bootconfig nodes.\n"); + if (emsg) + *emsg = "Failed to allocate bootconfig nodes"; return -ENOMEM; } memset(xbc_nodes, 0, sizeof(struct xbc_node) * XBC_NODE_MAX); @@ -814,9 +827,13 @@ int __init xbc_init(char *buf) if (!ret) ret = xbc_verify_tree(); - if (ret < 0) + if (ret < 0) { + if (epos) + *epos = xbc_err_pos; + if (emsg) + *emsg = xbc_err_msg; xbc_destroy_all(); - else + } else ret = xbc_node_num; return ret; diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c index a9b97814d1a9..16b9a420e6fd 100644 --- a/tools/bootconfig/main.c +++ b/tools/bootconfig/main.c @@ -130,6 +130,7 @@ int load_xbc_from_initrd(int fd, char **buf) int ret; u32 size = 0, csum = 0, rcsum; char magic[BOOTCONFIG_MAGIC_LEN]; + const char *msg; ret = fstat(fd, &stat); if (ret < 0) @@ -182,10 +183,12 @@ int load_xbc_from_initrd(int fd, char **buf) return -EINVAL; } - ret = xbc_init(*buf); + ret = xbc_init(*buf, &msg, NULL); /* Wrong data */ - if (ret < 0) + if (ret < 0) { + pr_err("parse error: %s.\n", msg); return ret; + } return size; } @@ -244,11 +247,34 @@ int delete_xbc(const char *path) return ret; } +static void show_xbc_error(const char *data, const char *msg, int pos) +{ + int lin = 1, col, i; + + if (pos < 0) { + pr_err("Error: %s.\n", msg); + return; + } + + /* Note that pos starts from 0 but lin and col should start from 1. */ + col = pos + 1; + for (i = 0; i < pos; i++) { + if (data[i] == '\n') { + lin++; + col = pos - i; + } + } + pr_err("Parse Error: %s at %d:%d\n", msg, lin, col); + +} + int apply_xbc(const char *path, const char *xbc_path) { u32 size, csum; char *buf, *data; int ret, fd; + const char *msg; + int pos; ret = load_xbc_file(xbc_path, &buf); if (ret < 0) { @@ -267,11 +293,12 @@ int apply_xbc(const char *path, const char *xbc_path) *(u32 *)(data + size + 4) = csum; /* Check the data format */ - ret = xbc_init(buf); + ret = xbc_init(buf, &msg, &pos); if (ret < 0) { - pr_err("Failed to parse %s: %d\n", xbc_path, ret); + show_xbc_error(data, msg, pos); free(data); free(buf); + return ret; } printf("Apply %s to %s\n", xbc_path, path); ^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2020-03-06 18:26 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-03 11:24 [PATCH 0/2] tools/bootconfig: Add O= option and enhance error message Masami Hiramatsu 2020-03-03 11:24 ` [PATCH 1/2] bootconfig: Support O=<builddir> option Masami Hiramatsu 2020-03-04 23:04 ` Randy Dunlap 2020-03-05 2:05 ` Masami Hiramatsu 2020-03-05 3:17 ` Steven Rostedt 2020-03-05 4:52 ` Randy Dunlap 2020-03-05 6:03 ` [BUGFIX PATCH] tools: Let O= makes handle a relative path with -C option Masami Hiramatsu 2020-03-05 14:51 ` Greg KH 2020-03-05 18:50 ` Randy Dunlap 2020-03-06 1:39 ` Masami Hiramatsu 2020-03-06 5:17 ` Randy Dunlap 2020-03-06 7:52 ` Geert Uytterhoeven 2020-03-06 15:07 ` Masami Hiramatsu 2020-03-06 16:26 ` Randy Dunlap 2020-03-06 18:10 ` Masami Hiramatsu 2020-03-06 18:26 ` Randy Dunlap 2020-03-06 16:45 ` Steven Rostedt 2020-03-05 7:41 ` [PATCH 1/2] bootconfig: Support O=<builddir> option Geert Uytterhoeven 2020-03-05 18:51 ` Randy Dunlap 2020-03-03 11:24 ` [PATCH 2/2] tools/bootconfig: Show line and column in parse error Masami Hiramatsu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).