* [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
* [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
* 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: [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: [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: [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
* 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 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: [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
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).