| Message ID | 20170818222052.22375-3-nicolasporcel06@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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"
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,
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.
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,
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");
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(-)