2018-02-28 00:19:17

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 0/6] kconfig: some clean-ups and rename silentoldconfig

Masahiro Yamada (6):
kconfig: do not call check_conf() for olddefconfig
kconfig: remove unneeded input_mode test in conf()
kconfig: remove redundant input_mode test for check_conf() loop
kconfig: hide irrelevant sub-menus for oldconfig
kconfig: invoke oldconfig instead of silentoldconfig from local*config
kconfig: rename silentoldconfig to syncconfig

Documentation/kbuild/kconfig.txt | 2 +-
Makefile | 2 +-
scripts/kconfig/Makefile | 8 ++++----
scripts/kconfig/conf.c | 41 ++++++++++++++++++++--------------------
4 files changed, 27 insertions(+), 26 deletions(-)

--
2.7.4



2018-02-28 00:18:57

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 3/6] kconfig: remove redundant input_mode test for check_conf() loop

check_conf() never increments conf_cnt for listnewconfig, so conf_cnt
is always zero.

In other words, conf_cnt is not zero, "input_mode != listnewconfig"
is met.

Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-by: Ulf Magnusson <[email protected]>
---

Changes in v2: None

scripts/kconfig/conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 1faa55f..59656d3 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -669,7 +669,7 @@ int main(int ac, char **av)
do {
conf_cnt = 0;
check_conf(&rootmenu);
- } while (conf_cnt && input_mode != listnewconfig);
+ } while (conf_cnt);
break;
case olddefconfig:
default:
--
2.7.4


2018-02-28 00:19:10

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 1/6] kconfig: do not call check_conf() for olddefconfig

check_conf() traverses the menu tree, but it is completely no-op for
olddefconfig because the following if-else block does nothing.

if (input_mode == listnewconfig) {
...
} else if (input_mode != olddefconfig) {
...
}

As the help message says, olddefconfig automatically sets new symbols
to their default value. There is no room for manual intervention.
So, calling check_conf() for olddefconfig is odd in the first place.

Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-by: Ulf Magnusson <[email protected]>
---

Changes in v2: None

scripts/kconfig/conf.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 822dc51..4227d3b 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -424,7 +424,7 @@ static void check_conf(struct menu *menu)
if (sym->name && !sym_is_choice_value(sym)) {
printf("%s%s\n", CONFIG_, sym->name);
}
- } else if (input_mode != olddefconfig) {
+ } else {
if (!conf_cnt++)
printf(_("*\n* Restart config...\n*\n"));
rootEntry = menu_get_parent_menu(menu);
@@ -666,15 +666,15 @@ int main(int ac, char **av)
/* fall through */
case oldconfig:
case listnewconfig:
- case olddefconfig:
case silentoldconfig:
/* Update until a loop caused no more changes */
do {
conf_cnt = 0;
check_conf(&rootmenu);
- } while (conf_cnt &&
- (input_mode != listnewconfig &&
- input_mode != olddefconfig));
+ } while (conf_cnt && input_mode != listnewconfig);
+ break;
+ case olddefconfig:
+ default:
break;
}

--
2.7.4


2018-02-28 00:19:24

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 2/6] kconfig: remove unneeded input_mode test in conf()

conf() is never called for listnewconfig / olddefconfig.

Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-by: Ulf Magnusson <[email protected]>
---

Changes in v2: None

scripts/kconfig/conf.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 4227d3b..1faa55f 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -358,9 +358,7 @@ static void conf(struct menu *menu)

switch (prop->type) {
case P_MENU:
- if ((input_mode == silentoldconfig ||
- input_mode == listnewconfig ||
- input_mode == olddefconfig) &&
+ if (input_mode == silentoldconfig &&
rootEntry != menu) {
check_conf(menu);
return;
--
2.7.4


2018-02-28 00:19:26

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 6/6] kconfig: rename silentoldconfig to syncconfig

As commit cedd55d49dee ("kconfig: Remove silentoldconfig from help
and docs; fix kconfig/conf's help") mentioned, 'silentoldconfig' is a
historical misnomer. That commit removed it from help and docs since
it is an internal interface. If so, it should be allowed to rename
it to something more intuitive. 'syncconfig' is the one I came up
with because it updates the .config if necessary, then synchronize
other files with it.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Changes in v2:
- newly added

Documentation/kbuild/kconfig.txt | 2 +-
Makefile | 2 +-
scripts/kconfig/Makefile | 4 ++--
scripts/kconfig/conf.c | 20 ++++++++++----------
4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/Documentation/kbuild/kconfig.txt b/Documentation/kbuild/kconfig.txt
index bbc99c0..7233118 100644
--- a/Documentation/kbuild/kconfig.txt
+++ b/Documentation/kbuild/kconfig.txt
@@ -119,7 +119,7 @@ Examples:
15% of tristates will be set to 'y', 15% to 'm', 70% to 'n'

______________________________________________________________________
-Environment variables for 'silentoldconfig'
+Environment variables for 'syncconfig'

KCONFIG_NOSILENTUPDATE
--------------------------------------------------
diff --git a/Makefile b/Makefile
index 8706bf2..ea23d9b 100644
--- a/Makefile
+++ b/Makefile
@@ -598,7 +598,7 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
# include/generated/ and include/config/. Update them if .config is newer than
# include/config/auto.conf (which mirrors .config).
include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
- $(Q)$(MAKE) -f $(srctree)/Makefile silentoldconfig
+ $(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
else
# external modules needs include/generated/autoconf.h and include/config/auto.conf
# but do not care if they are up-to-date. Use auto.conf to trigger the test
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index bf9289a..988258a 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -3,7 +3,7 @@
# Kernel configuration targets
# These targets are used from top-level makefile

-PHONY += xconfig gconfig menuconfig config silentoldconfig update-po-config \
+PHONY += xconfig gconfig menuconfig config syncconfig update-po-config \
localmodconfig localyesconfig

ifdef KBUILD_KCONFIG
@@ -36,7 +36,7 @@ nconfig: $(obj)/nconf

# This has become an internal implementation detail and is now deprecated
# for external use.
-silentoldconfig: $(obj)/conf
+syncconfig: $(obj)/conf
$(Q)mkdir -p include/config include/generated
$(Q)test -e include/generated/autoksyms.h || \
touch include/generated/autoksyms.h
diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 11a4e45..4e08121 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -23,7 +23,7 @@ static void check_conf(struct menu *menu);

enum input_mode {
oldaskconfig,
- silentoldconfig,
+ syncconfig,
oldconfig,
allnoconfig,
allyesconfig,
@@ -100,7 +100,7 @@ static int conf_askvalue(struct symbol *sym, const char *def)

switch (input_mode) {
case oldconfig:
- case silentoldconfig:
+ case syncconfig:
if (sym_has_value(sym)) {
printf("%s\n", def);
return 0;
@@ -293,7 +293,7 @@ static int conf_choice(struct menu *menu)
printf("[1-%d?]: ", cnt);
switch (input_mode) {
case oldconfig:
- case silentoldconfig:
+ case syncconfig:
if (!is_new) {
cnt = def;
printf("%d\n", cnt);
@@ -441,7 +441,7 @@ static void check_conf(struct menu *menu)
static struct option long_opts[] = {
{"oldaskconfig", no_argument, NULL, oldaskconfig},
{"oldconfig", no_argument, NULL, oldconfig},
- {"silentoldconfig", no_argument, NULL, silentoldconfig},
+ {"syncconfig", no_argument, NULL, syncconfig},
{"defconfig", optional_argument, NULL, defconfig},
{"savedefconfig", required_argument, NULL, savedefconfig},
{"allnoconfig", no_argument, NULL, allnoconfig},
@@ -468,8 +468,8 @@ static void conf_usage(const char *progname)
printf(" --listnewconfig List new options\n");
printf(" --oldaskconfig Start a new configuration using a line-oriented program\n");
printf(" --oldconfig Update a configuration using a provided .config as base\n");
- printf(" --silentoldconfig Similar to oldconfig but generates configuration in\n"
- " include/{generated/,config/} (oldconfig used to be more verbose)\n");
+ printf(" --syncconfig Similar to oldconfig but generates configuration in\n"
+ " include/{generated/,config/}\n");
printf(" --olddefconfig Same as oldconfig but sets new symbols to their default value\n");
printf(" --oldnoconfig An alias of olddefconfig\n");
printf(" --defconfig <file> New config with default defined in <file>\n");
@@ -501,7 +501,7 @@ int main(int ac, char **av)
}
input_mode = (enum input_mode)opt;
switch (opt) {
- case silentoldconfig:
+ case syncconfig:
sync_kconfig = 1;
break;
case defconfig:
@@ -583,7 +583,7 @@ int main(int ac, char **av)
}
break;
case savedefconfig:
- case silentoldconfig:
+ case syncconfig:
case oldaskconfig:
case oldconfig:
case listnewconfig:
@@ -667,7 +667,7 @@ int main(int ac, char **av)
/* fall through */
case oldconfig:
case listnewconfig:
- case silentoldconfig:
+ case syncconfig:
/* Update until a loop caused no more changes */
do {
conf_cnt = 0;
@@ -680,7 +680,7 @@ int main(int ac, char **av)
}

if (sync_kconfig) {
- /* silentoldconfig is used during the build so we shall update autoconf.
+ /* syncconfig is used during the build so we shall update autoconf.
* All other commands are only used to generate a config.
*/
if (conf_get_changed() && conf_write(NULL)) {
--
2.7.4


2018-02-28 00:19:42

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 5/6] kconfig: invoke oldconfig instead of silentoldconfig from local*config

The purpose of local{yes,mod}config is to arrange the .config file
based on actually loaded modules. It is unnecessary to update
include/generated/autoconf.h and include/config/* stuff here.

They will be automatically updated during the build.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Changes in v2:
- newly added

scripts/kconfig/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index c5d1d1a..bf9289a 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -49,11 +49,11 @@ localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf
cmp -s .tmp.config .config || \
(mv -f .config .config.old.1; \
mv -f .tmp.config .config; \
- $(obj)/conf $(silent) --silentoldconfig $(Kconfig); \
+ $(obj)/conf $(silent) --oldconfig $(Kconfig); \
mv -f .config.old.1 .config.old) \
else \
mv -f .tmp.config .config; \
- $(obj)/conf $(silent) --silentoldconfig $(Kconfig); \
+ $(obj)/conf $(silent) --oldconfig $(Kconfig); \
fi
$(Q)rm -f .tmp.config

--
2.7.4


2018-02-28 00:20:06

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 4/6] kconfig: hide irrelevant sub-menus for oldconfig

Historically, "make oldconfig" has changed its behavior several times,
quieter or louder. (I attached the history below.) Currently, it is
not as quiet as it should be. This commit addresses it.

Test Case
---------

---------------------------(Kconfig)----------------------------
menu "menu"

config FOO
bool "foo"

menu "sub menu"

config BAR
bool "bar"

endmenu

endmenu

menu "sibling menu"

config BAZ
bool "baz"

endmenu
----------------------------------------------------------------

---------------------------(.config)----------------------------
CONFIG_BAR=y
CONFIG_BAZ=y
----------------------------------------------------------------

With the Kconfig and .config above, "make silentoldconfig" and
"make oldconfig" work differently, like follows:

$ make silentoldconfig
scripts/kconfig/conf --silentoldconfig Kconfig
*
* Restart config...
*
*
* menu
*
foo (FOO) [N/y/?] (NEW) y
#
# configuration written to .config
#

$ make oldconfig
scripts/kconfig/conf --oldconfig Kconfig
*
* Restart config...
*
*
* menu
*
foo (FOO) [N/y/?] (NEW) y
*
* sub menu
*
bar (BAR) [Y/n/?] y
#
# configuration written to .config
#

Both hide "sibling node" since it is irrelevant. The difference is
that silentoldconfig hides "sub menu" whereas oldconfig does not.
The behavior of silentoldconfig is preferred since the "sub menu"
does not contain any new symbol.

The root cause is in conf(). There are three input modes that can
call conf(); oldaskconfig, oldconfig, and silentoldconfig.

Everytime conf() encounters a menu entry, it calls check_conf() to
check if it contains new symbols. If no new symbol is found, the
menu is just skipped.

Currently, this happens only when input_mode == silentoldconfig.
The oldaskconfig enters into the check_conf() loop as silentoldconfig,
so oldaskconfig works likewise for the second loop or later, but it
never happens for oldconfig. So, irrelevant sub-menus are shown for
oldconfig.

Change the test condition to "input_mode != oldaskconfig". This is
false only for the first loop of oldaskconfig; it must ask the user
all symbols, so no need to call check_conf().

History of oldconfig
--------------------

[0] Originally, "make oldconfig" was as loud as "make config" (It
showed the entire .config file)

[1] Commit cd9140e1e73a ("kconfig: make oldconfig is now less chatty")
made oldconfig quieter, but it was still less quieter than
silentoldconfig. (oldconfig did not hide sub-menus)

[2] Commit 204c96f60904 ("kconfig: fix silentoldconfig") changed
the input_mode of oldconfig to "ask_silent" from "ask_new".
So, oldconfig really became as quiet as silentoldconfig.
(oldconfig hided irrelevant sub-menus)

[3] Commit 4062f1a4c030 ("kconfig: use long options in conf") made
oldconfig as loud as [0] due to misconversion.

[4] Commit 14828349719a ("kconfig: fix make oldconfig") addressed
the misconversion of [3], but it made oldconfig quieter only to
the same level as [1], not [2].

This commit is restoring the behavior of [2].

Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-by: Ulf Magnusson <[email protected]>
---

Changes in v2:
- Add a brief comment

scripts/kconfig/conf.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 59656d3..11a4e45 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -358,8 +358,11 @@ static void conf(struct menu *menu)

switch (prop->type) {
case P_MENU:
- if (input_mode == silentoldconfig &&
- rootEntry != menu) {
+ /*
+ * Except in oldaskconfig mode, we show only menus that
+ * contain new symbols.
+ */
+ if (input_mode != oldaskconfig && rootEntry != menu) {
check_conf(menu);
return;
}
@@ -660,7 +663,7 @@ int main(int ac, char **av)
case oldaskconfig:
rootEntry = &rootmenu;
conf(&rootmenu);
- input_mode = silentoldconfig;
+ input_mode = oldconfig;
/* fall through */
case oldconfig:
case listnewconfig:
--
2.7.4


2018-02-28 02:11:20

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] kconfig: rename silentoldconfig to syncconfig

On 02/27/2018 04:15 PM, Masahiro Yamada wrote:
> As commit cedd55d49dee ("kconfig: Remove silentoldconfig from help
> and docs; fix kconfig/conf's help") mentioned, 'silentoldconfig' is a
> historical misnomer. That commit removed it from help and docs since
> it is an internal interface. If so, it should be allowed to rename
> it to something more intuitive. 'syncconfig' is the one I came up
> with because it updates the .config if necessary, then synchronize
> other files with it.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> Changes in v2:
> - newly added
>
> Documentation/kbuild/kconfig.txt | 2 +-
> Makefile | 2 +-
> scripts/kconfig/Makefile | 4 ++--
> scripts/kconfig/conf.c | 20 ++++++++++----------
> 4 files changed, 14 insertions(+), 14 deletions(-)

It wouldn't hurt to update this line also: (I would just drop "silentoldconfig")

Make oldconfig/silentoldconfig/menuconfig/etc.

in Documentation/networking/i40e.txt.


thanks,
--
~Randy

2018-02-28 05:16:34

by Ulf Magnusson

[permalink] [raw]
Subject: Re: [PATCH v2 5/6] kconfig: invoke oldconfig instead of silentoldconfig from local*config

On Wed, Feb 28, 2018 at 09:15:25AM +0900, Masahiro Yamada wrote:
> The purpose of local{yes,mod}config is to arrange the .config file
> based on actually loaded modules. It is unnecessary to update
> include/generated/autoconf.h and include/config/* stuff here.
>
> They will be automatically updated during the build.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> Changes in v2:
> - newly added
>
> scripts/kconfig/Makefile | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> index c5d1d1a..bf9289a 100644
> --- a/scripts/kconfig/Makefile
> +++ b/scripts/kconfig/Makefile
> @@ -49,11 +49,11 @@ localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf

(Unrelated: $(obj)/streamline_config.pl is a checked-in file, so I
wonder if there's any point to having it as a prerequisite of the phony
targets local{yes,mod}config.)

> cmp -s .tmp.config .config || \
> (mv -f .config .config.old.1; \
> mv -f .tmp.config .config; \
> - $(obj)/conf $(silent) --silentoldconfig $(Kconfig); \
> + $(obj)/conf $(silent) --oldconfig $(Kconfig); \

Maybe add extra space to keep \ aligned.

> mv -f .config.old.1 .config.old) \
> else \
> mv -f .tmp.config .config; \
> - $(obj)/conf $(silent) --silentoldconfig $(Kconfig); \
> + $(obj)/conf $(silent) --oldconfig $(Kconfig); \

Ditto here.

> fi
> $(Q)rm -f .tmp.config
>
> --
> 2.7.4
>

I'm not an expert on the Makefiles, but seems reasonable to me.

Reviewed-by: Ulf Magnusson <[email protected]>

Cheers,
Ulf

2018-02-28 05:42:45

by Ulf Magnusson

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] kconfig: rename silentoldconfig to syncconfig

On Wed, Feb 28, 2018 at 09:15:26AM +0900, Masahiro Yamada wrote:
> As commit cedd55d49dee ("kconfig: Remove silentoldconfig from help
> and docs; fix kconfig/conf's help") mentioned, 'silentoldconfig' is a
> historical misnomer. That commit removed it from help and docs since
> it is an internal interface. If so, it should be allowed to rename
> it to something more intuitive. 'syncconfig' is the one I came up
> with because it updates the .config if necessary, then synchronize
> other files with it.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> Changes in v2:
> - newly added
>
> Documentation/kbuild/kconfig.txt | 2 +-
> Makefile | 2 +-
> scripts/kconfig/Makefile | 4 ++--
> scripts/kconfig/conf.c | 20 ++++++++++----------
> 4 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/kbuild/kconfig.txt b/Documentation/kbuild/kconfig.txt
> index bbc99c0..7233118 100644
> --- a/Documentation/kbuild/kconfig.txt
> +++ b/Documentation/kbuild/kconfig.txt
> @@ -119,7 +119,7 @@ Examples:
> 15% of tristates will be set to 'y', 15% to 'm', 70% to 'n'
>
> ______________________________________________________________________
> -Environment variables for 'silentoldconfig'
> +Environment variables for 'syncconfig'
>
> KCONFIG_NOSILENTUPDATE
> --------------------------------------------------
> diff --git a/Makefile b/Makefile
> index 8706bf2..ea23d9b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -598,7 +598,7 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
> # include/generated/ and include/config/. Update them if .config is newer than
> # include/config/auto.conf (which mirrors .config).
> include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
> - $(Q)$(MAKE) -f $(srctree)/Makefile silentoldconfig
> + $(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
> else
> # external modules needs include/generated/autoconf.h and include/config/auto.conf
> # but do not care if they are up-to-date. Use auto.conf to trigger the test
> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> index bf9289a..988258a 100644
> --- a/scripts/kconfig/Makefile
> +++ b/scripts/kconfig/Makefile
> @@ -3,7 +3,7 @@
> # Kernel configuration targets
> # These targets are used from top-level makefile
>
> -PHONY += xconfig gconfig menuconfig config silentoldconfig update-po-config \
> +PHONY += xconfig gconfig menuconfig config syncconfig update-po-config \
> localmodconfig localyesconfig
>
> ifdef KBUILD_KCONFIG
> @@ -36,7 +36,7 @@ nconfig: $(obj)/nconf
>
> # This has become an internal implementation detail and is now deprecated
> # for external use.
> -silentoldconfig: $(obj)/conf
> +syncconfig: $(obj)/conf
> $(Q)mkdir -p include/config include/generated
> $(Q)test -e include/generated/autoksyms.h || \
> touch include/generated/autoksyms.h
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 11a4e45..4e08121 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -23,7 +23,7 @@ static void check_conf(struct menu *menu);
>
> enum input_mode {
> oldaskconfig,
> - silentoldconfig,
> + syncconfig,
> oldconfig,
> allnoconfig,
> allyesconfig,
> @@ -100,7 +100,7 @@ static int conf_askvalue(struct symbol *sym, const char *def)
>
> switch (input_mode) {
> case oldconfig:
> - case silentoldconfig:
> + case syncconfig:
> if (sym_has_value(sym)) {
> printf("%s\n", def);
> return 0;
> @@ -293,7 +293,7 @@ static int conf_choice(struct menu *menu)
> printf("[1-%d?]: ", cnt);
> switch (input_mode) {
> case oldconfig:
> - case silentoldconfig:
> + case syncconfig:
> if (!is_new) {
> cnt = def;
> printf("%d\n", cnt);
> @@ -441,7 +441,7 @@ static void check_conf(struct menu *menu)
> static struct option long_opts[] = {
> {"oldaskconfig", no_argument, NULL, oldaskconfig},
> {"oldconfig", no_argument, NULL, oldconfig},
> - {"silentoldconfig", no_argument, NULL, silentoldconfig},
> + {"syncconfig", no_argument, NULL, syncconfig},
> {"defconfig", optional_argument, NULL, defconfig},
> {"savedefconfig", required_argument, NULL, savedefconfig},
> {"allnoconfig", no_argument, NULL, allnoconfig},
> @@ -468,8 +468,8 @@ static void conf_usage(const char *progname)
> printf(" --listnewconfig List new options\n");
> printf(" --oldaskconfig Start a new configuration using a line-oriented program\n");
> printf(" --oldconfig Update a configuration using a provided .config as base\n");
> - printf(" --silentoldconfig Similar to oldconfig but generates configuration in\n"
> - " include/{generated/,config/} (oldconfig used to be more verbose)\n");
> + printf(" --syncconfig Similar to oldconfig but generates configuration in\n"
> + " include/{generated/,config/}\n");
> printf(" --olddefconfig Same as oldconfig but sets new symbols to their default value\n");
> printf(" --oldnoconfig An alias of olddefconfig\n");
> printf(" --defconfig <file> New config with default defined in <file>\n");
> @@ -501,7 +501,7 @@ int main(int ac, char **av)
> }
> input_mode = (enum input_mode)opt;
> switch (opt) {
> - case silentoldconfig:
> + case syncconfig:
> sync_kconfig = 1;
> break;
> case defconfig:
> @@ -583,7 +583,7 @@ int main(int ac, char **av)
> }
> break;
> case savedefconfig:
> - case silentoldconfig:
> + case syncconfig:
> case oldaskconfig:
> case oldconfig:
> case listnewconfig:
> @@ -667,7 +667,7 @@ int main(int ac, char **av)
> /* fall through */
> case oldconfig:
> case listnewconfig:
> - case silentoldconfig:
> + case syncconfig:
> /* Update until a loop caused no more changes */
> do {
> conf_cnt = 0;
> @@ -680,7 +680,7 @@ int main(int ac, char **av)
> }
>
> if (sync_kconfig) {
> - /* silentoldconfig is used during the build so we shall update autoconf.
> + /* syncconfig is used during the build so we shall update autoconf.
> * All other commands are only used to generate a config.
> */
> if (conf_get_changed() && conf_write(NULL)) {
> --
> 2.7.4
>

I wonder if it might be helpful to keep the silentoldconfig target for a
while and have it just fail with a message like the following:

silentoldconfig has been renamed to syncconfig and is now an
internal implementation detail. What you probably want is
oldconfig.

Going on Google and https://lkml.org/lkml/2018/2/12/1084, there might be
quite a lot of scripts and the like that call silentoldconfig.

Alternatively, it could call through to syncconfig and also generate a
warning that it's about to removed (like for olddefconfig).

Cheers,
Ulf

2018-02-28 05:44:50

by Ulf Magnusson

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] kconfig: rename silentoldconfig to syncconfig

On Wed, Feb 28, 2018 at 6:41 AM, Ulf Magnusson <[email protected]> wrote:
> On Wed, Feb 28, 2018 at 09:15:26AM +0900, Masahiro Yamada wrote:
>> As commit cedd55d49dee ("kconfig: Remove silentoldconfig from help
>> and docs; fix kconfig/conf's help") mentioned, 'silentoldconfig' is a
>> historical misnomer. That commit removed it from help and docs since
>> it is an internal interface. If so, it should be allowed to rename
>> it to something more intuitive. 'syncconfig' is the one I came up
>> with because it updates the .config if necessary, then synchronize
>> other files with it.
>>
>> Signed-off-by: Masahiro Yamada <[email protected]>
>> ---
>>
>> Changes in v2:
>> - newly added
>>
>> Documentation/kbuild/kconfig.txt | 2 +-
>> Makefile | 2 +-
>> scripts/kconfig/Makefile | 4 ++--
>> scripts/kconfig/conf.c | 20 ++++++++++----------
>> 4 files changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/Documentation/kbuild/kconfig.txt b/Documentation/kbuild/kconfig.txt
>> index bbc99c0..7233118 100644
>> --- a/Documentation/kbuild/kconfig.txt
>> +++ b/Documentation/kbuild/kconfig.txt
>> @@ -119,7 +119,7 @@ Examples:
>> 15% of tristates will be set to 'y', 15% to 'm', 70% to 'n'
>>
>> ______________________________________________________________________
>> -Environment variables for 'silentoldconfig'
>> +Environment variables for 'syncconfig'
>>
>> KCONFIG_NOSILENTUPDATE
>> --------------------------------------------------
>> diff --git a/Makefile b/Makefile
>> index 8706bf2..ea23d9b 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -598,7 +598,7 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
>> # include/generated/ and include/config/. Update them if .config is newer than
>> # include/config/auto.conf (which mirrors .config).
>> include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
>> - $(Q)$(MAKE) -f $(srctree)/Makefile silentoldconfig
>> + $(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
>> else
>> # external modules needs include/generated/autoconf.h and include/config/auto.conf
>> # but do not care if they are up-to-date. Use auto.conf to trigger the test
>> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
>> index bf9289a..988258a 100644
>> --- a/scripts/kconfig/Makefile
>> +++ b/scripts/kconfig/Makefile
>> @@ -3,7 +3,7 @@
>> # Kernel configuration targets
>> # These targets are used from top-level makefile
>>
>> -PHONY += xconfig gconfig menuconfig config silentoldconfig update-po-config \
>> +PHONY += xconfig gconfig menuconfig config syncconfig update-po-config \
>> localmodconfig localyesconfig
>>
>> ifdef KBUILD_KCONFIG
>> @@ -36,7 +36,7 @@ nconfig: $(obj)/nconf
>>
>> # This has become an internal implementation detail and is now deprecated
>> # for external use.
>> -silentoldconfig: $(obj)/conf
>> +syncconfig: $(obj)/conf
>> $(Q)mkdir -p include/config include/generated
>> $(Q)test -e include/generated/autoksyms.h || \
>> touch include/generated/autoksyms.h
>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>> index 11a4e45..4e08121 100644
>> --- a/scripts/kconfig/conf.c
>> +++ b/scripts/kconfig/conf.c
>> @@ -23,7 +23,7 @@ static void check_conf(struct menu *menu);
>>
>> enum input_mode {
>> oldaskconfig,
>> - silentoldconfig,
>> + syncconfig,
>> oldconfig,
>> allnoconfig,
>> allyesconfig,
>> @@ -100,7 +100,7 @@ static int conf_askvalue(struct symbol *sym, const char *def)
>>
>> switch (input_mode) {
>> case oldconfig:
>> - case silentoldconfig:
>> + case syncconfig:
>> if (sym_has_value(sym)) {
>> printf("%s\n", def);
>> return 0;
>> @@ -293,7 +293,7 @@ static int conf_choice(struct menu *menu)
>> printf("[1-%d?]: ", cnt);
>> switch (input_mode) {
>> case oldconfig:
>> - case silentoldconfig:
>> + case syncconfig:
>> if (!is_new) {
>> cnt = def;
>> printf("%d\n", cnt);
>> @@ -441,7 +441,7 @@ static void check_conf(struct menu *menu)
>> static struct option long_opts[] = {
>> {"oldaskconfig", no_argument, NULL, oldaskconfig},
>> {"oldconfig", no_argument, NULL, oldconfig},
>> - {"silentoldconfig", no_argument, NULL, silentoldconfig},
>> + {"syncconfig", no_argument, NULL, syncconfig},
>> {"defconfig", optional_argument, NULL, defconfig},
>> {"savedefconfig", required_argument, NULL, savedefconfig},
>> {"allnoconfig", no_argument, NULL, allnoconfig},
>> @@ -468,8 +468,8 @@ static void conf_usage(const char *progname)
>> printf(" --listnewconfig List new options\n");
>> printf(" --oldaskconfig Start a new configuration using a line-oriented program\n");
>> printf(" --oldconfig Update a configuration using a provided .config as base\n");
>> - printf(" --silentoldconfig Similar to oldconfig but generates configuration in\n"
>> - " include/{generated/,config/} (oldconfig used to be more verbose)\n");
>> + printf(" --syncconfig Similar to oldconfig but generates configuration in\n"
>> + " include/{generated/,config/}\n");
>> printf(" --olddefconfig Same as oldconfig but sets new symbols to their default value\n");
>> printf(" --oldnoconfig An alias of olddefconfig\n");
>> printf(" --defconfig <file> New config with default defined in <file>\n");
>> @@ -501,7 +501,7 @@ int main(int ac, char **av)
>> }
>> input_mode = (enum input_mode)opt;
>> switch (opt) {
>> - case silentoldconfig:
>> + case syncconfig:
>> sync_kconfig = 1;
>> break;
>> case defconfig:
>> @@ -583,7 +583,7 @@ int main(int ac, char **av)
>> }
>> break;
>> case savedefconfig:
>> - case silentoldconfig:
>> + case syncconfig:
>> case oldaskconfig:
>> case oldconfig:
>> case listnewconfig:
>> @@ -667,7 +667,7 @@ int main(int ac, char **av)
>> /* fall through */
>> case oldconfig:
>> case listnewconfig:
>> - case silentoldconfig:
>> + case syncconfig:
>> /* Update until a loop caused no more changes */
>> do {
>> conf_cnt = 0;
>> @@ -680,7 +680,7 @@ int main(int ac, char **av)
>> }
>>
>> if (sync_kconfig) {
>> - /* silentoldconfig is used during the build so we shall update autoconf.
>> + /* syncconfig is used during the build so we shall update autoconf.
>> * All other commands are only used to generate a config.
>> */
>> if (conf_get_changed() && conf_write(NULL)) {
>> --
>> 2.7.4
>>
>
> I wonder if it might be helpful to keep the silentoldconfig target for a
> while and have it just fail with a message like the following:
>
> silentoldconfig has been renamed to syncconfig and is now an
> internal implementation detail. What you probably want is
> oldconfig.
>
> Going on Google and https://lkml.org/lkml/2018/2/12/1084, there might be
> quite a lot of scripts and the like that call silentoldconfig.
>
> Alternatively, it could call through to syncconfig and also generate a
> warning that it's about to removed (like for olddefconfig).

*oldnoconfig

Cheers,
Ulf

2018-02-28 15:31:03

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] kconfig: some clean-ups and rename silentoldconfig

2018-02-28 9:15 GMT+09:00 Masahiro Yamada <[email protected]>:
> Masahiro Yamada (6):
> kconfig: do not call check_conf() for olddefconfig
> kconfig: remove unneeded input_mode test in conf()
> kconfig: remove redundant input_mode test for check_conf() loop
> kconfig: hide irrelevant sub-menus for oldconfig
> kconfig: invoke oldconfig instead of silentoldconfig from local*config
> kconfig: rename silentoldconfig to syncconfig
>
> Documentation/kbuild/kconfig.txt | 2 +-
> Makefile | 2 +-
> scripts/kconfig/Makefile | 8 ++++----
> scripts/kconfig/conf.c | 41 ++++++++++++++++++++--------------------
> 4 files changed, 27 insertions(+), 26 deletions(-)
>
> --


1-4 applied to linux-kbuild/kconfig
because they are re-post.

I will re-spin 5 and 6 as commented.


--
Best Regards
Masahiro Yamada

2018-03-01 07:45:56

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v2 5/6] kconfig: invoke oldconfig instead of silentoldconfig from local*config

2018-02-28 14:15 GMT+09:00 Ulf Magnusson <[email protected]>:
> On Wed, Feb 28, 2018 at 09:15:25AM +0900, Masahiro Yamada wrote:
>> The purpose of local{yes,mod}config is to arrange the .config file
>> based on actually loaded modules. It is unnecessary to update
>> include/generated/autoconf.h and include/config/* stuff here.
>>
>> They will be automatically updated during the build.
>>
>> Signed-off-by: Masahiro Yamada <[email protected]>
>> ---
>>
>> Changes in v2:
>> - newly added
>>
>> scripts/kconfig/Makefile | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
>> index c5d1d1a..bf9289a 100644
>> --- a/scripts/kconfig/Makefile
>> +++ b/scripts/kconfig/Makefile
>> @@ -49,11 +49,11 @@ localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf
>
> (Unrelated: $(obj)/streamline_config.pl is a checked-in file, so I
> wonder if there's any point to having it as a prerequisite of the phony
> targets local{yes,mod}config.)
>

Indeed.

It it unrelated, but I think it is worth fixing.

Care to send a patch, please?




--
Best Regards
Masahiro Yamada

2018-03-01 11:19:50

by Ulf Magnusson

[permalink] [raw]
Subject: [PATCH] kconfig: remove redundant streamline_config.pl prerequisite

The local{yes,mod}config targets currently have streamline_config.pl as
a prerequisite. This is redundant, because streamline_config.pl is a
checked-in file with no prerequisites.

Remove the prerequisite and reference streamline_config.pl directly in
the recipe of the rule instead.

Signed-off-by: Ulf Magnusson <[email protected]>
---
scripts/kconfig/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index 1f74336d4e23..58be52cb464d 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -77,9 +77,9 @@ silentoldconfig: $(obj)/conf
touch include/generated/autoksyms.h
$< $(silent) --$@ $(Kconfig)

-localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf
+localyesconfig localmodconfig: $(obj)/conf
$(Q)mkdir -p include/config include/generated
- $(Q)perl $< --$@ $(srctree) $(Kconfig) > .tmp.config
+ $(Q)perl $(obj)/streamline_config.pl --$@ $(srctree) $(Kconfig) > .tmp.config
$(Q)if [ -f .config ]; then \
cmp -s .tmp.config .config || \
(mv -f .config .config.old.1; \
--
2.14.1


2018-03-01 14:41:22

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kconfig: remove redundant streamline_config.pl prerequisite

2018-03-01 20:18 GMT+09:00 Ulf Magnusson <[email protected]>:
> The local{yes,mod}config targets currently have streamline_config.pl as
> a prerequisite. This is redundant, because streamline_config.pl is a
> checked-in file with no prerequisites.
>
> Remove the prerequisite and reference streamline_config.pl directly in
> the recipe of the rule instead.
>
> Signed-off-by: Ulf Magnusson <[email protected]>
> ---
> scripts/kconfig/Makefile | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> index 1f74336d4e23..58be52cb464d 100644


Thanks! Almost good.

Just small nits.


> --- a/scripts/kconfig/Makefile
> +++ b/scripts/kconfig/Makefile
> @@ -77,9 +77,9 @@ silentoldconfig: $(obj)/conf
> touch include/generated/autoksyms.h
> $< $(silent) --$@ $(Kconfig)
>
> -localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf
> +localyesconfig localmodconfig: $(obj)/conf
> $(Q)mkdir -p include/config include/generated
> - $(Q)perl $< --$@ $(srctree) $(Kconfig) > .tmp.config
> + $(Q)perl $(obj)/streamline_config.pl --$@ $(srctree) $(Kconfig) > .tmp.config



'$(src)/streamline_config.pl' is better than '$(obj)/streamline_config.pl'
since it is a checked-in file.

'$(src)' and '$(obj)' are always the same.
(https://github.com/torvalds/linux/blob/master/scripts/Makefile.build#L6)


So, there is no effective difference.
It is just a coding convention to use $(obj)/ for generated files,
and $(src)/ for source files.

The original code already used $(obj)/, so this is not your fault
but I want to fix it while we are here.



One more nit.

$(obj)/conf $(silent) --silentoldconfig $(Kconfig);

can be

$< $(silent) --silentoldconfig $(Kconfig);


I guess you did not touch this line to avoid
conflict with my patch.



If you agree those two, shall I fix it up
when I apply it?





> $(Q)if [ -f .config ]; then \
> cmp -s .tmp.config .config || \
> (mv -f .config .config.old.1; \
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Best Regards
Masahiro Yamada

2018-03-01 14:46:35

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kconfig: remove redundant streamline_config.pl prerequisite

2018-03-01 23:39 GMT+09:00 Masahiro Yamada <[email protected]>:
> 2018-03-01 20:18 GMT+09:00 Ulf Magnusson <[email protected]>:
>> The local{yes,mod}config targets currently have streamline_config.pl as
>> a prerequisite. This is redundant, because streamline_config.pl is a
>> checked-in file with no prerequisites.
>>
>> Remove the prerequisite and reference streamline_config.pl directly in
>> the recipe of the rule instead.
>>
>> Signed-off-by: Ulf Magnusson <[email protected]>
>> ---
>> scripts/kconfig/Makefile | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
>> index 1f74336d4e23..58be52cb464d 100644
>
>
> Thanks! Almost good.
>
> Just small nits.
>
>
>> --- a/scripts/kconfig/Makefile
>> +++ b/scripts/kconfig/Makefile
>> @@ -77,9 +77,9 @@ silentoldconfig: $(obj)/conf
>> touch include/generated/autoksyms.h
>> $< $(silent) --$@ $(Kconfig)
>>
>> -localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf
>> +localyesconfig localmodconfig: $(obj)/conf
>> $(Q)mkdir -p include/config include/generated
>> - $(Q)perl $< --$@ $(srctree) $(Kconfig) > .tmp.config
>> + $(Q)perl $(obj)/streamline_config.pl --$@ $(srctree) $(Kconfig) > .tmp.config
>
>
>
> '$(src)/streamline_config.pl' is better than '$(obj)/streamline_config.pl'
> since it is a checked-in file.
>
> '$(src)' and '$(obj)' are always the same.
> (https://github.com/torvalds/linux/blob/master/scripts/Makefile.build#L6)
>
>
> So, there is no effective difference.
> It is just a coding convention to use $(obj)/ for generated files,
> and $(src)/ for source files.
>
> The original code already used $(obj)/, so this is not your fault
> but I want to fix it while we are here.


No.

This is not a matter of taste,
but a bug that must be fixed.


This patch breaks out-of-tree build.

If O=... is given, it is error.


$ make O=foo localyesconfig
make[1]: Leaving directory '/home/masahiro/workspace/linux-yamada/foo'
masahiro@grover:~/workspace/linux-yamada$ make O=foo localyesconfig
make[1]: Entering directory '/home/masahiro/workspace/linux-yamada/foo'
GEN ./Makefile
Can't open perl script "scripts/kconfig/streamline_config.pl": No such
file or directory
../scripts/kconfig/Makefile:46: recipe for target 'localyesconfig' failed
make[2]: *** [localyesconfig] Error 2
/home/masahiro/workspace/linux-yamada/Makefile:521: recipe for target
'localyesconfig' failed
make[1]: *** [localyesconfig] Error 2
make[1]: Leaving directory '/home/masahiro/workspace/linux-yamada/foo'
Makefile:146: recipe for target 'sub-make' failed
make: *** [sub-make] Error 2



The right fix is $(srctree)/$(src)/streamline_config.pl



--
Best Regards
Masahiro Yamada

2018-03-01 14:54:30

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kconfig: remove redundant streamline_config.pl prerequisite

2018-03-01 23:44 GMT+09:00 Masahiro Yamada <[email protected]>:
> 2018-03-01 23:39 GMT+09:00 Masahiro Yamada <[email protected]>:
>> 2018-03-01 20:18 GMT+09:00 Ulf Magnusson <[email protected]>:
>>> The local{yes,mod}config targets currently have streamline_config.pl as
>>> a prerequisite. This is redundant, because streamline_config.pl is a
>>> checked-in file with no prerequisites.
>>>
>>> Remove the prerequisite and reference streamline_config.pl directly in
>>> the recipe of the rule instead.
>>>
>>> Signed-off-by: Ulf Magnusson <[email protected]>
>>> ---
>>> scripts/kconfig/Makefile | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
>>> index 1f74336d4e23..58be52cb464d 100644
>>
>>
>> Thanks! Almost good.
>>
>> Just small nits.
>>
>>
>>> --- a/scripts/kconfig/Makefile
>>> +++ b/scripts/kconfig/Makefile
>>> @@ -77,9 +77,9 @@ silentoldconfig: $(obj)/conf
>>> touch include/generated/autoksyms.h
>>> $< $(silent) --$@ $(Kconfig)
>>>
>>> -localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf
>>> +localyesconfig localmodconfig: $(obj)/conf
>>> $(Q)mkdir -p include/config include/generated
>>> - $(Q)perl $< --$@ $(srctree) $(Kconfig) > .tmp.config
>>> + $(Q)perl $(obj)/streamline_config.pl --$@ $(srctree) $(Kconfig) > .tmp.config
>>
>>
>>
>> '$(src)/streamline_config.pl' is better than '$(obj)/streamline_config.pl'
>> since it is a checked-in file.
>>
>> '$(src)' and '$(obj)' are always the same.
>> (https://github.com/torvalds/linux/blob/master/scripts/Makefile.build#L6)
>>
>>
>> So, there is no effective difference.
>> It is just a coding convention to use $(obj)/ for generated files,
>> and $(src)/ for source files.
>>
>> The original code already used $(obj)/, so this is not your fault
>> but I want to fix it while we are here.
>
>
> No.
>
> This is not a matter of taste,
> but a bug that must be fixed.
>
>
> This patch breaks out-of-tree build.
>
> If O=... is given, it is error.
>
>
> $ make O=foo localyesconfig
> make[1]: Leaving directory '/home/masahiro/workspace/linux-yamada/foo'
> masahiro@grover:~/workspace/linux-yamada$ make O=foo localyesconfig
> make[1]: Entering directory '/home/masahiro/workspace/linux-yamada/foo'
> GEN ./Makefile
> Can't open perl script "scripts/kconfig/streamline_config.pl": No such
> file or directory
> ../scripts/kconfig/Makefile:46: recipe for target 'localyesconfig' failed
> make[2]: *** [localyesconfig] Error 2
> /home/masahiro/workspace/linux-yamada/Makefile:521: recipe for target
> 'localyesconfig' failed
> make[1]: *** [localyesconfig] Error 2
> make[1]: Leaving directory '/home/masahiro/workspace/linux-yamada/foo'
> Makefile:146: recipe for target 'sub-make' failed
> make: *** [sub-make] Error 2
>
>
>
> The right fix is $(srctree)/$(src)/streamline_config.pl
>
>




For somebody wondering why the current code works
for out-of-tree building.


Kbuild sets VPATH
(https://github.com/torvalds/linux/blob/master/Makefile#L208)


So, Make searches for pre-requisites in both objtree and srctree
when building out-of-tree.

VPATH does not work for recipe lines.
So, we need to specify the exact path to streamline_config.pl



--
Best Regards
Masahiro Yamada

2018-03-01 15:25:25

by Ulf Magnusson

[permalink] [raw]
Subject: Re: [PATCH] kconfig: remove redundant streamline_config.pl prerequisite

On Thu, Mar 1, 2018 at 3:39 PM, Masahiro Yamada
<[email protected]> wrote:
> 2018-03-01 20:18 GMT+09:00 Ulf Magnusson <[email protected]>:
>> The local{yes,mod}config targets currently have streamline_config.pl as
>> a prerequisite. This is redundant, because streamline_config.pl is a
>> checked-in file with no prerequisites.
>>
>> Remove the prerequisite and reference streamline_config.pl directly in
>> the recipe of the rule instead.
>>
>> Signed-off-by: Ulf Magnusson <[email protected]>
>> ---
>> scripts/kconfig/Makefile | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
>> index 1f74336d4e23..58be52cb464d 100644
>
>
> Thanks! Almost good.
>
> Just small nits.
>
>
>> --- a/scripts/kconfig/Makefile
>> +++ b/scripts/kconfig/Makefile
>> @@ -77,9 +77,9 @@ silentoldconfig: $(obj)/conf
>> touch include/generated/autoksyms.h
>> $< $(silent) --$@ $(Kconfig)
>>
>> -localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf
>> +localyesconfig localmodconfig: $(obj)/conf
>> $(Q)mkdir -p include/config include/generated
>> - $(Q)perl $< --$@ $(srctree) $(Kconfig) > .tmp.config
>> + $(Q)perl $(obj)/streamline_config.pl --$@ $(srctree) $(Kconfig) > .tmp.config
>
>
>
> '$(src)/streamline_config.pl' is better than '$(obj)/streamline_config.pl'
> since it is a checked-in file.
>
> '$(src)' and '$(obj)' are always the same.
> (https://github.com/torvalds/linux/blob/master/scripts/Makefile.build#L6)
>
>
> So, there is no effective difference.
> It is just a coding convention to use $(obj)/ for generated files,
> and $(src)/ for source files.
>
> The original code already used $(obj)/, so this is not your fault
> but I want to fix it while we are here.
>
>
>
> One more nit.
>
> $(obj)/conf $(silent) --silentoldconfig $(Kconfig);
>
> can be
>
> $< $(silent) --silentoldconfig $(Kconfig);
>
>
> I guess you did not touch this line to avoid
> conflict with my patch.

Yep, plus I wanted to keep the patch focused.

>
>
>
> If you agree those two, shall I fix it up
> when I apply it?

That's fine by me. I'll assume less sanity from the existing code from
now on. :)

>
>
>
>
>
>> $(Q)if [ -f .config ]; then \
>> cmp -s .tmp.config .config || \
>> (mv -f .config .config.old.1; \
>> --
>> 2.14.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Best Regards
> Masahiro Yamada

Thanks,
Ulf

2018-03-06 09:50:47

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kconfig: remove redundant streamline_config.pl prerequisite

2018-03-02 0:23 GMT+09:00 Ulf Magnusson <[email protected]>:
> On Thu, Mar 1, 2018 at 3:39 PM, Masahiro Yamada
> <[email protected]> wrote:
>> 2018-03-01 20:18 GMT+09:00 Ulf Magnusson <[email protected]>:
>>> The local{yes,mod}config targets currently have streamline_config.pl as
>>> a prerequisite. This is redundant, because streamline_config.pl is a
>>> checked-in file with no prerequisites.
>>>
>>> Remove the prerequisite and reference streamline_config.pl directly in
>>> the recipe of the rule instead.
>>>
>>> Signed-off-by: Ulf Magnusson <[email protected]>
>>> ---
>>> scripts/kconfig/Makefile | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
>>> index 1f74336d4e23..58be52cb464d 100644
>>
>>
>> Thanks! Almost good.
>>
>> Just small nits.
>>
>>
>>> --- a/scripts/kconfig/Makefile
>>> +++ b/scripts/kconfig/Makefile
>>> @@ -77,9 +77,9 @@ silentoldconfig: $(obj)/conf
>>> touch include/generated/autoksyms.h
>>> $< $(silent) --$@ $(Kconfig)
>>>
>>> -localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf
>>> +localyesconfig localmodconfig: $(obj)/conf
>>> $(Q)mkdir -p include/config include/generated
>>> - $(Q)perl $< --$@ $(srctree) $(Kconfig) > .tmp.config
>>> + $(Q)perl $(obj)/streamline_config.pl --$@ $(srctree) $(Kconfig) > .tmp.config
>>
>>
>>
>> '$(src)/streamline_config.pl' is better than '$(obj)/streamline_config.pl'
>> since it is a checked-in file.
>>
>> '$(src)' and '$(obj)' are always the same.
>> (https://github.com/torvalds/linux/blob/master/scripts/Makefile.build#L6)
>>
>>
>> So, there is no effective difference.
>> It is just a coding convention to use $(obj)/ for generated files,
>> and $(src)/ for source files.
>>
>> The original code already used $(obj)/, so this is not your fault
>> but I want to fix it while we are here.
>>
>>
>>
>> One more nit.
>>
>> $(obj)/conf $(silent) --silentoldconfig $(Kconfig);
>>
>> can be
>>
>> $< $(silent) --silentoldconfig $(Kconfig);
>>
>>
>> I guess you did not touch this line to avoid
>> conflict with my patch.
>
> Yep, plus I wanted to keep the patch focused.




Applied to linux-kbuild/kconfig. Thanks!


I changed

[1] $(obj)/streamline_config.pl
-> $(srctree)/$(src)/streamline_config.pl

Mandatory change to avoid out-of-tree build error

[2] $(obj)/conf -> $<

Clean-up




--
Best Regards
Masahiro Yamada