fix dependency generation
PATCH 1/1 fix dependency generation
Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way the split config tree is built, but failed to also adjust fixdep accordingly - if changing a config option from or to m, files referencing the respective CONFIG_..._MODULE (but not the corresponding CONFIG_...) didn't get rebuilt.
Once at it, also eliminate false dependencies due to use of ...CONFIG_... identifiers.
scripts/basic/fixdep.c +6 -3
@@ -29,8 +29,7 @@* option which is mentioned in any of the listed prequisites. * * To be exact, split-include populates a tree in include/config/, -* e.g. include/config/his/driver.h, which contains the #define/#undef -* for the CONFIG_HIS_DRIVER option. +* e.g. include/config/his/driver.h, consiting of empty files. * * So if the user changes his CONFIG_HIS_DRIVER option, only the objects * which depend on "include/linux/config/his/driver.h" will be rebuilt,
@@ -223,7 +222,7 @@ void use_config(char *m, int slen)void parse_config_file (char *map,size_t len) {int *end = (int *) (map + len); -/* start at +1, so that p can never be < map */ +/* start at +1, so that p can never be <= map */ int *m = (int *) map +1 ;char *p, *q;
@@ -235,6 +234,8 @@ void parse_config_file(char *map, size_tcontinue ; conf:if (p > map + len -7 ) +break ; +if (isalnum (p[-1 ]) || p[-1 ] =='_' )continue ;if (memcmp (p,"CONFIG_" ,7 ))continue ;
@@ -245,6 +246,8 @@ void parse_config_file(char *map, size_tcontinue ; found: +if (!memcmp (q -7 ,"_MODULE" ,7 )) + q -=7 ;use_config (p+7 , q-p-7 ); } }
Quoted 14 lines · On Thu, 29 Mar 2007 10:27:14 +0100 Jan Beulich wrote:
Commit
2e3646e51b2d6415549b310655df63e7e0d7a080changed the way the split config tree is built, but failed to also adjust fixdep accordingly - if changing a config option from or to m, files referencing the respective CONFIG_..._MODULE (but not the correspondingCONFIG_...) didn't get rebuilt.Once at it, also eliminate false dependencies due to use of ...CONFIG_... identifiers.
Signed-off-by Jan Beulich <[email protected]>
scripts/basic/fixdep.c line 33
Quoted 5 lines
scripts/basic/fixdep.c +0
-2@@ -29,8 +29,7 @@* option which is mentioned in any of the listed prequisites. * * To be exact, split-include populates a tree in include/config/, -* e.g. include/config/his/driver.h, which contains the #define/#undef -* for the CONFIG_HIS_DRIVER option.
I don't see why you deleted the line above.
scripts/basic/fixdep.c line 32
Quoted 1 line
scripts/basic/fixdep.c +1
-0+* e.g. include/config/his/driver.h, consiting of empty files.
consisting
Quoted 22 lines
scripts/basic/fixdep.c +3
-1* * So if the user changes his CONFIG_HIS_DRIVER option, only the objects * which depend on "include/linux/config/his/driver.h" will be rebuilt, @@ -223,7 +222,7 @@ void use_config(char *m, int slen)void parse_config_file (char *map,size_t len) {int *end = (int *) (map + len); -/* start at +1, so that p can never be < map */ +/* start at +1, so that p can never be <= map */ int *m = (int *) map +1 ;char *p, *q;@@ -235,6 +234,8 @@ void parse_config_file(char *map, size_tcontinue ; conf:if (p > map + len -7 ) +break ; +if (isalnum (p[-1 ]) || p[-1 ] =='_' )continue ;if (memcmp (p,"CONFIG_" ,7 ))continue ;
scripts/basic/fixdep.c void parse_config_file(char *map, size_t · line 253
Quoted 8 lines
scripts/basic/fixdep.c +2
-0@@ -245,6 +246,8 @@ void parse_config_file(char *map, size_tcontinue ; found: +if (!memcmp (q -7 ,"_MODULE" ,7 )) + q -=7 ;use_config (p+7 , q-p-7 ); } }
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
Quoted 11 lines
scripts/basic/fixdep.c line 33
Quoted 9 lines
Quoted 1 line
Randy Dunlap <[email protected]> 29.03.07 17:39 >>>
scripts/basic/fixdep.c +0
-2@@ -29,8 +29,7 @@* option which is mentioned in any of the listed prequisites. * * To be exact, split-include populates a tree in include/config/, -* e.g. include/config/his/driver.h, which contains the #define/#undef -* for the CONFIG_HIS_DRIVER option. I don't see why you deleted the line above.
Because it is no longer true. These files are empty as of 2.6.18.
Jan
Quoted 12 lines · On Thu, 29 Mar 2007 17:06:24 +0100 Jan Beulich wrote:
Quoted 11 lines
scripts/basic/fixdep.c line 33
Quoted 9 lines
Quoted 1 line
Randy Dunlap <[email protected]> 29.03.07 17:39 >>>
scripts/basic/fixdep.c +0
-2@@ -29,8 +29,7 @@* option which is mentioned in any of the listed prequisites. * * To be exact, split-include populates a tree in include/config/, -* e.g. include/config/his/driver.h, which contains the #define/#undef -* for the CONFIG_HIS_DRIVER option. I don't see why you deleted the line above.
Quoted 1 line
Because it is no longer true. These files are empty as of 2.6.18.
We seem to be talking about different lines above. Yes, the files are empty, but they are named based on the CONFIG_symbol name, which is what I was trying to get at. So how about a comment like this:
* To be exact, split-include populates a tree in include/config/,
* e.g., include/config/sysctl/syscall.h,
* for the CONFIG_SYSCTL_SYSCALL option, when that option
* is enabled (=y or =m).
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
Quoted 17 lines
Quoted 1 line
Quoted 1 line
Randy Dunlap <[email protected]> 29.03.07 18:38 >>>
Quoted 12 lines · On Thu, 29 Mar 2007 17:06:24 +0100 Jan Beulich wrote:
Quoted 11 lines
scripts/basic/fixdep.c line 33
Quoted 9 lines
Quoted 1 line
Randy Dunlap <[email protected]> 29.03.07 17:39 >>>
scripts/basic/fixdep.c +0
-2@@ -29,8 +29,7 @@* option which is mentioned in any of the listed prequisites. * * To be exact, split-include populates a tree in include/config/, -* e.g. include/config/his/driver.h, which contains the #define/#undef -* for the CONFIG_HIS_DRIVER option. I don't see why you deleted the line above.
Quoted 1 line
Because it is no longer true. These files are empty as of 2.6.18.
Quoted 8 lines
We seem to be talking about different lines above. Yes, the files are empty, but they are named based on the
CONFIG_symbolname, which is what I was trying to get at. So how about a comment like this:* To be exact, split-include populates a tree in
include/config/,
* e.g.,include/config/sysctl/syscall.h,
* for theCONFIG_SYSCTL_SYSCALLoption, when that option
* is enabled (=y or =m).
Shouldn't that then be '..., when that option is or ever was enabled (=y or =m) since last cleaning the tree'?
Jan
Quoted 18 lines · Jan Beulich
Quoted 18 lines
Quoted 1 line
Quoted 1 line
Randy Dunlap <[email protected]> 29.03.07 18:38 >>>
Quoted 11 lines · On Thu, 29 Mar 2007 17:06:24 +0100 Jan Beulich wrote:
Quoted 10 lines
scripts/basic/fixdep.c line 33
Quoted 9 lines
Quoted 1 line
Randy Dunlap <[email protected]> 29.03.07 17:39 >>>
scripts/basic/fixdep.c +0
-2@@ -29,8 +29,7 @@* option which is mentioned in any of the listed prequisites. * * To be exact, split-include populates a tree in include/config/, -* e.g. include/config/his/driver.h, which contains the #define/#undef -* for the CONFIG_HIS_DRIVER option. I don't see why you deleted the line above.
Because it is no longer true. These files are empty as of 2.6.18.
We seem to be talking about different lines above. Yes, the files are empty, but they are named based on the
CONFIG_symbolname, which is what I was trying to get at. So how about a comment like this:
Quoted 7 lines
Quoted 4 lines
* To be exact, split-include populates a tree in
include/config/,
* e.g.,include/config/sysctl/syscall.h,
* for theCONFIG_SYSCTL_SYSCALLoption, when that option
* is enabled (=y or =m).Shouldn't that then be '..., when that option is or ever was enabled (=y or =m) since last cleaning the tree'?
Uh, I have no idea, but I guess that's OK with me.
Signature · 1 line
~Randy
Quoted 5 lines · Jan Beulich
Commit
2e3646e51b2d6415549b310655df63e7e0d7a080changed the way the split config tree is built, but failed to also adjust fixdep accordingly - if changing a config option from or to m, files referencing the respective CONFIG_..._MODULE (but not the correspondingCONFIG_...) didn't get rebuilt.
Do you have a test case for this?
I want to play a little with this before I submit it.
Quoted 3 lines
Once at it, also eliminate false dependencies due to use of ...CONFIG_... identifiers.
But that will break UM - no??
See following note from fixdep:
* Note 2: if somebody writes HELLO_CONFIG_BOOM in a file, it will depend onto
* CONFIG_BOOM. This could seem a bug (not too hard to fix), but please do not
* fix it! Some UserModeLinux files (look at arch/um/) call CONFIG_BOOM as
* UML_CONFIG_BOOM, to avoid conflicts with /usr/include/linux/autoconf.h,
* through arch/um/include/uml-config.h; this fixdep "bug" makes sure that
* those files will have correct dependencies.
Sam
Quoted 44 lines
Signed-off-by Jan Beulich <[email protected]>scripts/basic/fixdep.c +6
-3@@ -29,8 +29,7 @@* option which is mentioned in any of the listed prequisites. * * To be exact, split-include populates a tree in include/config/, -* e.g. include/config/his/driver.h, which contains the #define/#undef -* for the CONFIG_HIS_DRIVER option. +* e.g. include/config/his/driver.h, consiting of empty files. * * So if the user changes his CONFIG_HIS_DRIVER option, only the objects * which depend on "include/linux/config/his/driver.h" will be rebuilt, @@ -223,7 +222,7 @@ void use_config(char *m, int slen)void parse_config_file (char *map,size_t len) {int *end = (int *) (map + len); -/* start at +1, so that p can never be < map */ +/* start at +1, so that p can never be <= map */ int *m = (int *) map +1 ;char *p, *q;@@ -235,6 +234,8 @@ void parse_config_file(char *map, size_tcontinue ; conf:if (p > map + len -7 ) +break ; +if (isalnum (p[-1 ]) || p[-1 ] =='_' )continue ;if (memcmp (p,"CONFIG_" ,7 ))continue ;@@ -245,6 +246,8 @@ void parse_config_file(char *map, size_tcontinue ; found: +if (!memcmp (q -7 ,"_MODULE" ,7 )) + q -=7 ;use_config (p+7 , q-p-7 ); } }
Quoted 9 lines
Quoted 1 line
Quoted 1 line
Sam Ravnborg <[email protected]> 30.03.07 17:08 >>>
Quoted 5 lines · Jan Beulich
Commit
2e3646e51b2d6415549b310655df63e7e0d7a080changed the way the split config tree is built, but failed to also adjust fixdep accordingly - if changing a config option from or to m, files referencing the respective CONFIG_..._MODULE (but not the correspondingCONFIG_...) didn't get rebuilt.Do you have a test case for this?
I want to play a little with this before I submit it.
On i386, set CONFIG_APM=y, build, then change it to m.
Quoted 11 lines
Quoted 3 lines
Once at it, also eliminate false dependencies due to use of ...CONFIG_... identifiers.
But that will break UM - no??
See following note from fixdep:
* Note 2: if somebody writesHELLO_CONFIG_BOOMin a file, it will depend onto
*CONFIG_BOOM. This could seem a bug (not too hard to fix), but please do not
* fix it! Some UserModeLinux files (look atarch/um/) callCONFIG_BOOMas
*UML_CONFIG_BOOM, to avoid conflicts with/usr/include/linux/autoconf.h,
* througharch/um/include/uml-config.h; this fixdep "bug" makes sure that
* those files will have correct dependencies.
Hmm, didn't see this note. Then this might warrant special casing UML, but penalizing all code due to this seems at least odd to me.
Jan
Quoted 11 lines · Jan Beulich
Quoted 8 lines
But that will break UM - no??
See following note from fixdep:
* Note 2: if somebody writesHELLO_CONFIG_BOOMin a file, it will depend onto
*CONFIG_BOOM. This could seem a bug (not too hard to fix), but please do not
* fix it! Some UserModeLinux files (look atarch/um/) callCONFIG_BOOMas
*UML_CONFIG_BOOM, to avoid conflicts with/usr/include/linux/autoconf.h,
* througharch/um/include/uml-config.h; this fixdep "bug" makes sure that
* those files will have correct dependencies.Hmm, didn't see this note. Then this might warrant special casing UML, but penalizing all code due to this seems at least odd to me.
If I understand this, I would think that special-casing UML_CONFIG_* instead of *_CONFIG_* would be the way to go.
Jeff
Signature · 1 line
Work email - jdike at linux dot intel dot com
Quoted 5 lines · Jan Beulich
Commit
2e3646e51b2d6415549b310655df63e7e0d7a080changed the way the split config tree is built, but failed to also adjust fixdep accordingly - if changing a config option from or to m, files referencing the respective CONFIG_..._MODULE (but not the correspondingCONFIG_...) didn't get rebuilt.
The problem is that tristate symbol represent three values. =n => CONFIG_SYMBOL is undefined
=y => CONFIG_SYMBOL is defined
=m => COMFIG_SYMBOL_MODULE is defined
The function split_config does not take into account the different values and 'fixing' this in fixdep is wrong. Because fixdep does not know if the variable is a tristate symbol or not so it can either blindly remove _MODULE (your patch)
or each time it encounters _MODULE check for a symbol with and without _MODULE.
The better fix is to teach the split_config function that for tristate symbols two files shall be created in the include/config hirachy. So for apm this gets:include/config/apm.hinclude/config/apm/module.h
This will make kconfig behave correct the day that someone add a config symbol with a _MODULE suffix.
I will follow-up with two patches that implement the changes to split_config. The first is a pure code refactoring preparing for the second patch.
Roman - please ack/nack these this since they touches kconfig backend.
Sam
Quoted 47 lines
Once at it, also eliminate false dependencies due to use of ...CONFIG_... identifiers.
Signed-off-by Jan Beulich <[email protected]>scripts/basic/fixdep.c +6
-3@@ -29,8 +29,7 @@* option which is mentioned in any of the listed prequisites. * * To be exact, split-include populates a tree in include/config/, -* e.g. include/config/his/driver.h, which contains the #define/#undef -* for the CONFIG_HIS_DRIVER option. +* e.g. include/config/his/driver.h, consiting of empty files. * * So if the user changes his CONFIG_HIS_DRIVER option, only the objects * which depend on "include/linux/config/his/driver.h" will be rebuilt, @@ -223,7 +222,7 @@ void use_config(char *m, int slen)void parse_config_file (char *map,size_t len) {int *end = (int *) (map + len); -/* start at +1, so that p can never be < map */ +/* start at +1, so that p can never be <= map */ int *m = (int *) map +1 ;char *p, *q;@@ -235,6 +234,8 @@ void parse_config_file(char *map, size_tcontinue ; conf:if (p > map + len -7 ) +break ; +if (isalnum (p[-1 ]) || p[-1 ] =='_' )continue ;if (memcmp (p,"CONFIG_" ,7 ))continue ;@@ -245,6 +246,8 @@ void parse_config_file(char *map, size_tcontinue ; found: +if (!memcmp (q -7 ,"_MODULE" ,7 )) + q -=7 ;use_config (p+7 , q-p-7 ); } }
PATCH 1/2 kconfig: factor out code in conf_spilt_config
Quoted 1 line
From
e9fcc3bf8d1c71df1ae650d5291c5d4b15d71656Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <[email protected]>
Date: Sat, 31 Mar 2007 09:15:12 +0200
Subject: [PATCH] kconfig: factor out code in conf_spilt_config
This patch simply factor out code and do not introduce any functional changes.
--- scripts/kconfig/confdata.c | 75 ++++++++++++++++++++++++-------------------- 1 files changed, 41 insertions(+), 34 deletions(-)
scripts/kconfig/confdata.c +41 -34
@@ -533,13 +533,48 @@ int conf_write(const char *name)return 0 ; } +/* Touch the file specified (adding .h to the name) */ +static int touch_file (const char *file) +{ +struct stat sb; +int fd; +char *d; +char name[128 ]; + +strcpy (name, file); +strcat (name,".h" ); + +/* Open existing file */ + fd =open (name,O_WRONLY |O_CREAT |O_TRUNC ,0644 ); +if (fd ==-1 ) { +if (errno !=ENOENT ) +return 1 ; +/* +* Create directory components, +* unless they exist already. +*/ + d = name; +while ((d =strchr (d,'/' ))) { + *d =0 ; +if (stat (name, &sb) &&mkdir (name,0755 )) +return 1 ; + *d++ ='/' ; + } +/* Directories created, now create file. */ + fd =open (name,O_WRONLY |O_CREAT |O_TRUNC ,0644 ); +if (fd ==-1 ) +return 1 ; + } +close (fd); +return 0 ; +} +int conf_split_config (void ) {char *name, path[128 ];char *s, *d, c;struct symbol *sym; -struct stat sb; -int res, i, fd; +int res, i; name =getenv ("KCONFIG_AUTOCONFIG" );if (!name)
@@ -601,45 +636,17 @@ int conf_split_config(void)* different from 'no'). */ -/* Replace all '_' and append ".h" */ +/* Replace all '_' with '/' */ s = sym->name; d = path;while ((c = *s++)) { c =tolower (c); *d++ = (c =='_' ) ?'/' : c; } -strcpy (d,".h" ); - -/* Assume directory path already exists. */ - fd =open (path,O_WRONLY |O_CREAT |O_TRUNC ,0644 ); -if (fd ==-1 ) { -if (errno !=ENOENT ) { - res =1 ; -break ; - } -/* -* Create directory components, -* unless they exist already. -*/ - d = path; -while ((d =strchr (d,'/' ))) { - *d =0 ; -if (stat (path, &sb) &&mkdir (path,0755 )) { - res =1 ; - goto out; - } - *d++ ='/' ; - } -/* Try it again. */ - fd =open (path,O_WRONLY |O_CREAT |O_TRUNC ,0644 ); -if (fd ==-1 ) { - res =1 ; -break ; - } - } -close (fd); + *d ='\0' ; +if (touch_file (path)) +return 1 ; } -out:if (chdir ("../.." ))return 1 ;
Signature · 1 line
1.5.1.rc3.gaa453
PATCH 2/2 kconfig/kbuild: fix dependency problem
In reply to Sam RavnborgQuoted 1 line
From
bbc89026f3e5d9e437ce4cd26d3013fe226103e2Mon Sep 17 00:00:00 2001
From: Sam Ravnborg <[email protected]>
Date: Sat, 31 Mar 2007 09:34:46 +0200
Subject: [PATCH] kconfig/kbuild: fix dependency problem
Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way the split config tree is built, but failed to also adjust fixdep accordingly - if changing a config option from or to m, files referencing the respective CONFIG_..._MODULE (but not the corresponding CONFIG_...) didn't get rebuilt.
This happens because tristate symbol has three values represented by different CONFIG_ symbols:
=n => CONFIG_SYMBOL undefined
=y => CONFIG_SYMBOL equals 1
=m => CONFIG_SYMBOL_MODULE equals 1
But conf_split_config did not support the _MODULE syntax and therefore no include/config/symbol/module.h file was generated/touched when changing a symbol to/from m. Thus make did nt pick up the change and rebuild failed.
This patch teaches conf_split_config to support the _MODULE variant.
This fixes a problem reported by Randy Dunlap <[email protected]>. arch/i386/kernel/apm.o revealed this bug.
Original fix was posted by: "Jan Beulich" <[email protected]> which inspired this better fix.
--- scripts/kconfig/confdata.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
scripts/kconfig/confdata.c +6 -0
@@ -646,6 +646,12 @@ int conf_split_config(void) *d ='\0' ;if (touch_file (path))return 1 ; +/* For tristate symbols we need to touch symbol/module.h too */ +if (sym->type ==S_TRISTATE ) { +strcat (path,"/module" ); +if (touch_file (path)) +return 1 ; + } }if (chdir ("../.." ))return 1 ;
Signature · 1 line
1.5.1.rc3.gaa453
Hi,
Quoted 11 lines · Sam Ravnborg
The problem is that tristate symbol represent three values. =n =>
CONFIG_SYMBOLis undefined
=y =>CONFIG_SYMBOLis defined
=m =>COMFIG_SYMBOL_MODULEis definedThe function
split_configdoes not take into account the different values and 'fixing' this in fixdep is wrong. Because fixdep does not know if the variable is a tristate symbol or not so it can either blindly remove_MODULE(your patch)
or each time it encounters_MODULEcheck for a symbol with and without_MODULE.
What really matters is that CONFIG_SYMBOL changed, one could optimize for the COMFIG_SYMBOL_MODULE case, but I don't think it's worth it, especially
...
Quoted 5 lines
The better fix is to teach the
split_configfunction that for tristate symbols two files shall be created in theinclude/confighirachy. So for apm this gets:include/config/apm.hinclude/config/apm/module.h
if it requires thousands of new inodes for a feature which should be rarely used.
Quoted 2 lines
This will make kconfig behave correct the day that someone add a config symbol with a
_MODULEsuffix.
I'd rather reserve that namespace, if it allows for the simpler version to just map all symbols to the basic config symbol name.
bye, Roman
Quoted 26 lines · Roman Zippel
Hi,
Quoted 11 lines · Sam Ravnborg
The problem is that tristate symbol represent three values. =n =>
CONFIG_SYMBOLis undefined
=y =>CONFIG_SYMBOLis defined
=m =>COMFIG_SYMBOL_MODULEis definedThe function
split_configdoes not take into account the different values and 'fixing' this in fixdep is wrong. Because fixdep does not know if the variable is a tristate symbol or not so it can either blindly remove_MODULE(your patch)
or each time it encounters_MODULEcheck for a symbol with and without_MODULE.What really matters is that
CONFIG_SYMBOLchanged, one could optimize for theCOMFIG_SYMBOL_MODULEcase, but I don't think it's worth it, especially
...Quoted 5 lines
The better fix is to teach the
split_configfunction that for tristate symbols two files shall be created in theinclude/confighirachy. So for apm this gets:include/config/apm.hinclude/config/apm/module.h
Quoted 8 lines
if it requires thousands of new inodes for a feature which should be rarely used.
Quoted 2 lines
This will make kconfig behave correct the day that someone add a config symbol with a
_MODULEsuffix.I'd rather reserve that namespace, if it allows for the simpler version to just map all symbols to the basic config symbol name.
OK, I have forwarded Jan's patch for inclusion (the bug-fix part).
Sam
Quoted 2 lines
bye, Roman