[PATCH 0/2] Improve KCONFIG_CONFIG use in kconfig

[PATCH 0/2] Improve KCONFIG_CONFIG use in kconfig

[Nicolas Porcel]

This patch series will improve the use of KCONFIG_CONFIG in order to perform
out of tree kernel configuration. This is useful when the kernel source is not
on a writable directory.

Nicolas Porcel (2):
  kconfig: Use KCONFIG_CONFIG in buildtar
  kconfig: support out of tree KCONFIG_CONFIG

 scripts/kconfig/confdata.c | 35 ++++++++++++++++++++---------------
 scripts/package/buildtar   |  4 ++--
 2 files changed, 22 insertions(+), 17 deletions(-)

-- 
2.14.1

[PATCH 1/2] kconfig: Use KCONFIG_CONFIG in buildtar

[Nicolas Porcel]

Previously, .config was used in buildtar script regardless of the value of
KCONFIG_CONFIG.

Signed-off-by: Nicolas Porcel <nicolasporcel06@gmail.com>
---
 scripts/package/buildtar | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/package/buildtar b/scripts/package/buildtar
index e046bff33589..483ba00524d5 100755
--- a/scripts/package/buildtar
+++ b/scripts/package/buildtar
@@ -56,7 +56,7 @@ mkdir -p -- "${tmpdir}/boot"
 #
 # Try to install modules
 #
-if grep -q '^CONFIG_MODULES=y' "${objtree}/.config"; then
+if grep -q '^CONFIG_MODULES=y' "${KCONFIG_CONFIG}"; then
 	make ARCH="${ARCH}" O="${objtree}" KBUILD_SRC= INSTALL_MOD_PATH="${tmpdir}" modules_install
 fi
 
@@ -65,7 +65,7 @@ fi
 # Install basic kernel files
 #
 cp -v -- "${objtree}/System.map" "${tmpdir}/boot/System.map-${KERNELRELEASE}"
-cp -v -- "${objtree}/.config" "${tmpdir}/boot/config-${KERNELRELEASE}"
+cp -v -- "${KCONFIG_CONFIG}" "${tmpdir}/boot/config-${KERNELRELEASE}"
 cp -v -- "${objtree}/vmlinux" "${tmpdir}/boot/vmlinux-${KERNELRELEASE}"
 
 
-- 
2.14.1

[PATCH 2/2] kconfig: support out of tree KCONFIG_CONFIG

[Nicolas Porcel]

If KCONFIG_CONFIG is on a different file system than the kernel source,
oldconfig/defconfig will fail due to the use of rename in conf_write. It
will also fail if the kernel source directory is not writable.

This patch changes the behavior of conf_write to be more intuitive.

When no argument is passed to conf_write, KCONFIG_CONFIG is used instead
for the config path. The consequence is that the .tmpconfig.%(pid) is
written in the same directory as KCONFIG_CONFIG.

Some defaulting logic has been added, allowing the use of a directory
for KCONFIG_CONFIG. In that case, the .config file will be written in
this directory.

Signed-off-by: Nicolas Porcel <nicolasporcel06@gmail.com>
---
 scripts/kconfig/confdata.c | 35 ++++++++++++++++++++---------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 297b079ae4d9..3c2b7155a385 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -743,32 +743,37 @@ int conf_write(const char *name)
 	FILE *out;
 	struct symbol *sym;
 	struct menu *menu;
+	struct stat st;
 	const char *basename;
 	const char *str;
 	char dirname[PATH_MAX+1], tmpname[PATH_MAX+1], newname[PATH_MAX+1];
 	char *env;
+	char *slash;
+
+	const char *configname = conf_get_configname();
 
 	dirname[0] = 0;
-	if (name && name[0]) {
-		struct stat st;
-		char *slash;
-
-		if (!stat(name, &st) && S_ISDIR(st.st_mode)) {
-			strcpy(dirname, name);
-			strcat(dirname, "/");
-			basename = conf_get_configname();
-		} else if ((slash = strrchr(name, '/'))) {
+	if (!name || !name[0])
+		name = configname;
+
+	if (!stat(name, &st) && S_ISDIR(st.st_mode)) {
+		strcpy(dirname, name);
+		strcat(dirname, "/");
+		if (name == configname || strchr(configname, '/'))
+			basename =  ".config";
+		else
+			basename = configname;
+	} else {
+		slash = strrchr(name, '/');
+		if (slash) {
 			int size = slash - name + 1;
+
 			memcpy(dirname, name, size);
 			dirname[size] = 0;
-			if (slash[1])
-				basename = slash + 1;
-			else
-				basename = conf_get_configname();
+			basename = slash + 1;
 		} else
 			basename = name;
-	} else
-		basename = conf_get_configname();
+	}
 
 	sprintf(newname, "%s%s", dirname, basename);
 	env = getenv("KCONFIG_OVERWRITECONFIG");
-- 
2.14.1

Re: [PATCH 0/2] Improve KCONFIG_CONFIG use in kconfig

[Masahiro Yamada]

2017-08-19 7:20 GMT+09:00 Nicolas Porcel <nicolasporcel06@gmail.com>:
> This patch series will improve the use of KCONFIG_CONFIG in order to perform
> out of tree kernel configuration. This is useful when the kernel source is not
> on a writable directory.
>
> Nicolas Porcel (2):
>   kconfig: Use KCONFIG_CONFIG in buildtar
>   kconfig: support out of tree KCONFIG_CONFIG
>

I do not get it.

We are supposed to use O=path/to/build/dir
(or KBUILD_OUTPUT).

Even if you patch the location of .config,
you can not put object files to the read-only kernel source.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 1/2] kconfig: Use KCONFIG_CONFIG in buildtar

[Masahiro Yamada]

2017-08-19 7:20 GMT+09:00 Nicolas Porcel <nicolasporcel06@gmail.com>:
> Previously, .config was used in buildtar script regardless of the value of
> KCONFIG_CONFIG.
>
> Signed-off-by: Nicolas Porcel <nicolasporcel06@gmail.com>


This one looks good.

Applied to linux-kbuild.




-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/2] kconfig: support out of tree KCONFIG_CONFIG

[Masahiro Yamada]

2017-08-19 7:20 GMT+09:00 Nicolas Porcel <nicolasporcel06@gmail.com>:
> If KCONFIG_CONFIG is on a different file system than the kernel source,
> oldconfig/defconfig will fail due to the use of rename in conf_write. It
> will also fail if the kernel source directory is not writable.

If the kernel source is not writable, O= should be used.


> This patch changes the behavior of conf_write to be more intuitive.
>
> When no argument is passed to conf_write, KCONFIG_CONFIG is used instead
> for the config path. The consequence is that the .tmpconfig.%(pid) is
> written in the same directory as KCONFIG_CONFIG.
>
> Some defaulting logic has been added, allowing the use of a directory
> for KCONFIG_CONFIG. In that case, the .config file will be written in
> this directory.



I think KCONFIG_CONFIG only contains a file name
and this restriction is intentional
because conf_write() might be called with a directory path (for
example from gconf.c)


If we change the behavior,
I think conf_write() should just error-out
when directory path is given.



> Signed-off-by: Nicolas Porcel <nicolasporcel06@gmail.com>
> ---
>  scripts/kconfig/confdata.c | 35 ++++++++++++++++++++---------------
>  1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 297b079ae4d9..3c2b7155a385 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -743,32 +743,37 @@ int conf_write(const char *name)
>         FILE *out;
>         struct symbol *sym;
>         struct menu *menu;
> +       struct stat st;
>         const char *basename;
>         const char *str;
>         char dirname[PATH_MAX+1], tmpname[PATH_MAX+1], newname[PATH_MAX+1];
>         char *env;
> +       char *slash;
> +
> +       const char *configname = conf_get_configname();
>
>         dirname[0] = 0;
> -       if (name && name[0]) {
> -               struct stat st;
> -               char *slash;
> -
> -               if (!stat(name, &st) && S_ISDIR(st.st_mode)) {
> -                       strcpy(dirname, name);
> -                       strcat(dirname, "/");
> -                       basename = conf_get_configname();
> -               } else if ((slash = strrchr(name, '/'))) {
> +       if (!name || !name[0])
> +               name = configname;
> +
> +       if (!stat(name, &st) && S_ISDIR(st.st_mode)) {
> +               strcpy(dirname, name);
> +               strcat(dirname, "/");
> +               if (name == configname || strchr(configname, '/'))
> +                       basename =  ".config";


Hmm, I think the intention of conf_get_configname() is
to avoid the bare use of ".config"






-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/2] kconfig: support out of tree KCONFIG_CONFIG

[Nicolas Porcel]

On Sat, Sep 02, 2017 at 04:31:44PM +0900, Masahiro Yamada wrote:
> 2017-08-19 7:20 GMT+09:00 Nicolas Porcel <nicolasporcel06@gmail.com>:
> 
> If the kernel source is not writable, O= should be used.

You are right, I didn't know O= worked for kernel config.

> I think KCONFIG_CONFIG only contains a file name
> and this restriction is intentional
> because conf_write() might be called with a directory path (for
> example from gconf.c)
> 
> 
> If we change the behavior,
> I think conf_write() should just error-out
> when directory path is given.
> 

When I build a kernel, I set KCONFIG_CONFIG to reference a file on a
different file-system than the build directory. It used to work before
kernel 4.12, hence my patch.

I now realize that I misunderstood the purpose of this variable. It
also means that it is not possible to store the configuration file
outside the build directory when using silentconfig for instance.

Of course, I can use a symlink with KCONFIG_OVERWRITECONFIG, but it
may be an interesting feature to be able to specify an absolute path
for the kernel config file.

Best regards,

-- 
Nicolas Porcel

Re: [PATCH 2/2] kconfig: support out of tree KCONFIG_CONFIG

[Masahiro Yamada]

2017-09-03 7:21 GMT+09:00 Nicolas Porcel <nicolasporcel06@gmail.com>:
> On Sat, Sep 02, 2017 at 04:31:44PM +0900, Masahiro Yamada wrote:
>> 2017-08-19 7:20 GMT+09:00 Nicolas Porcel <nicolasporcel06@gmail.com>:
>>
>> If the kernel source is not writable, O= should be used.
>
> You are right, I didn't know O= worked for kernel config.
>
>> I think KCONFIG_CONFIG only contains a file name
>> and this restriction is intentional
>> because conf_write() might be called with a directory path (for
>> example from gconf.c)
>>
>>
>> If we change the behavior,
>> I think conf_write() should just error-out
>> when directory path is given.
>>
>
> When I build a kernel, I set KCONFIG_CONFIG to reference a file on a
> different file-system than the build directory. It used to work before
> kernel 4.12, hence my patch.
>
> I now realize that I misunderstood the purpose of this variable. It
> also means that it is not possible to store the configuration file
> outside the build directory when using silentconfig for instance.


Some targets can choose the save path of the .config file.

For example, run "make gconfig"
then choose "File  ->  Save as"
from the menu.


But, silentoldconfig does not have a way to save .config
out out the build tree.


> Of course, I can use a symlink with KCONFIG_OVERWRITECONFIG, but it
> may be an interesting feature to be able to specify an absolute path
> for the kernel config file.



conf_write() may be called with a directory path,
in this case, the KCONFIG_OVERWRITECONFIG is appended
as the base name.

If we support an absolute path in KCONFIG_OVERWRITECONFIG,
I think it is strange.




-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/2] kconfig: support out of tree KCONFIG_CONFIG

[Nicolas Porcel]

On Wed, Sep 20, 2017 at 06:56:17PM +0900, Masahiro Yamada wrote:
> Some targets can choose the save path of the .config file.
> 
> For example, run "make gconfig"
> then choose "File  ->  Save as"
> from the menu.
>
> But, silentoldconfig does not have a way to save .config
> out out the build tree.

Yes, I noticed this feature when writing the patch and I tried not to
break it. However, running make without target will call silentconfig
before building and it is not possible to use a file outside the build
folder.

The reason I did this patch is because I am keeping my kernel
configuration files in an external git repository and I am using
KCONFIG_CONFIG to select the reference this config. It was working well
before 4.13. But maybe my workflow is wrong.

> 
> conf_write() may be called with a directory path,
> in this case, the KCONFIG_OVERWRITECONFIG is appended
> as the base name.
> 
> If we support an absolute path in KCONFIG_OVERWRITECONFIG,
> I think it is strange.
> 

According to the documentation, KCONFIG_OVERWRITECONFIG is a boolean
that should be set if the .config file is a symlink. In the code, it
conf_write will directly write the config file unstead of creating a
temporary file and renaming it to .config.

So it allows me to use any file on the machine by symlinking it to the
.config file of the build folder so that I can keep my current workflow.
Unfortunately, I didn't know about it before writing the patch.

The strange part is that KCONFIG_CONFIG supports absolute path in most
of the build process. It only breaks when referencing a file on a
different file-system than the build folder, and only because of the
silentconfig call.

Best regards,

-- 
Nicolas Porcel