uclibc: fix handling of configuration file

PATCH Superseded Thomas De Schampheleire 2 participants 10 messages Jul 18, 2014 – Jul 30, 2014

Share & tools

Thomas De Schampheleire

author

2014-07-18 18:54 UTC #0 lore raw

PATCH v3 1/1 uclibc: fix handling of configuration file

This patch series (part of it poster earlier as RFC) has as main goal to fix the uclibc kconfig handling with following requirements in mind:

- neither foo-menuconfig nor foo-update-config should have a dependency on the toolchain (which means we cannot depend on foo-configure)

- 'clean foo-menuconfig' should start from the specified (custom) config file

- 'foo-menuconfig foo-update-config' should preserve the changes made in the menuconfig step (this problem is reported for uclibc with bug #7154
(https://bugs.busybox.net/show_bug.cgi?id=7154).

- 'make foo-menuconfig; make foo-menuconfig' should preserve the changes made in the menuconfig step.

- 'make clean foo-update-config' should copy the initial file, do any fixups, and then save the config

Once this series is accepted, the plan is:
- to extract the kconfig-related bits to a kconfig-package infrastructure.
- to convert busybox, linux and barebox to this kconfig-package infra too.

Signed-off-by Thomas De Schampheleire <[email protected]>

---
v3: only update commit message of third patch

 package/uclibc/uclibc.mk |  24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

Thomas De Schampheleire

author

2014-07-18 18:54 UTC #1 lore raw

PATCH v3 1/1 uclibc: rename SETUP_DOT_CONFIG to FIXUP_DOT_CONFIG

In a subsequent patch, the behavior of UCLIBC_SETUP_DOT_CONFIG will change so that it only applies fixups to the (already copied) configuration file. This patch renames this function to UCLIBC_FIXUP_DOT_CONFIG to better match the future behavior.

Signed-off-by Thomas De Schampheleire <[email protected]>

Acked-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

---
v3: no changes
v2: update commit message (ThomasP)

 package/uclibc/uclibc.mk |  4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

package/uclibc/uclibc.mk +2 -2

diff -r 67d0973240f2 -r 282d34c926c8 package/uclibc/uclibc.mk
@@ -393,7 +393,7 @@
 	UCLIBC_EXTRA_CFLAGS="$(UCLIBC_EXTRA_CFLAGS) $(TARGET_ABI)" \
 	HOSTCC="$(HOSTCC)"
 
-define UCLIBC_SETUP_DOT_CONFIG
+define UCLIBC_FIXUP_DOT_CONFIG
 	$(INSTALL) -m 0644 $(UCLIBC_CONFIG_FILE) $(@D)/.config
 	$(call KCONFIG_SET_OPT,CROSS_COMPILER_PREFIX,"$(TARGET_CROSS)",$(@D)/.config)
 	$(call KCONFIG_ENABLE_OPT,TARGET_$(UCLIBC_TARGET_ARCH),$(@D)/.config)

@@ -433,7 +433,7 @@
 endef
 
 define UCLIBC_CONFIGURE_CMDS
-	$(UCLIBC_SETUP_DOT_CONFIG)
+	$(UCLIBC_FIXUP_DOT_CONFIG)
 	$(MAKE1) -C $(UCLIBC_DIR) \
 		$(UCLIBC_MAKE_FLAGS) \
 		PREFIX=$(STAGING_DIR) \

Thomas De Schampheleire

author

2014-07-18 18:54 UTC #2 lore raw

PATCH v3 1/1 uclibc: menuconfig: take into account initial settings from config file

In reply to Thomas De Schampheleire

When executing the sequence 'make clean uclibc-menuconfig', the configured config file is not taken into account and one starts from the default settings.

This patch adds an explicit target for the config file and lets the configure and menuconfig steps depend on it, fixing the problem.

Signed-off-by Thomas De Schampheleire <[email protected]>

---
v3: no changes
v2:
- use UCLIBC_TARGET_CONFIGURE instead of specifying the stamp path in full (Arnout)
rfc->patch:
- add target for .config to avoid menuconfig copying the config file every time (ThomasP, Arnout)
- move dependency on patch step from menuconfig to the .config target

 package/uclibc/uclibc.mk |  12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

package/uclibc/uclibc.mk +8 -4

diff -r 282d34c926c8 -r 29ba03b43b12 package/uclibc/uclibc.mk
@@ -394,7 +394,6 @@
 	HOSTCC="$(HOSTCC)"
 
 define UCLIBC_FIXUP_DOT_CONFIG
-	$(INSTALL) -m 0644 $(UCLIBC_CONFIG_FILE) $(@D)/.config
 	$(call KCONFIG_SET_OPT,CROSS_COMPILER_PREFIX,"$(TARGET_CROSS)",$(@D)/.config)
 	$(call KCONFIG_ENABLE_OPT,TARGET_$(UCLIBC_TARGET_ARCH),$(@D)/.config)
 	$(call KCONFIG_SET_OPT,TARGET_ARCH,"$(UCLIBC_TARGET_ARCH)",$(@D)/.config)

@@ -533,7 +532,14 @@
 	$(UCLIBC_INSTALL_UTILS_STAGING)
 endef
 
-uclibc-menuconfig: uclibc-patch
+$(eval $(generic-package))
+
+$(UCLIBC_DIR)/.config: $(UCLIBC_CONFIG_FILE) | uclibc-patch
+	$(INSTALL) -m 0644 $(UCLIBC_CONFIG_FILE) $(UCLIBC_DIR)/.config
+
+$(UCLIBC_TARGET_CONFIGURE): $(UCLIBC_DIR)/.config
+
+uclibc-menuconfig: $(UCLIBC_DIR)/.config
 	$(MAKE1) -C $(UCLIBC_DIR) \
 		$(UCLIBC_MAKE_FLAGS) \
 		PREFIX=$(STAGING_DIR) \

@@ -542,8 +548,6 @@
 		menuconfig
 	rm -f $(UCLIBC_DIR)/.stamp_{configured,built,target_installed,staging_installed}
 
-$(eval $(generic-package))
-
 uclibc-update-config: $(UCLIBC_DIR)/.stamp_configured
 	cp -f $(UCLIBC_DIR)/.config $(UCLIBC_CONFIG_FILE)

Thomas De Schampheleire

author

2014-07-18 18:54 UTC #3 lore raw

PATCH v3 1/1 uclibc: update-config: preserve freshly configured settings

In reply to Thomas De Schampheleire

In the sequence:

make uclibc-menuconfig
make uclibc-update-config

the freshly configured settings from the menuconfig are lost during the update-config step. This is because update-config depends on the configure step, which starts by copying the config file to the build directory.

Instead, stop depending on the configure step from update-config, and introduce a new stamp file .stamp_config_fixup_done, which applies any fixups on the .config file.
An alternative solution would be to add a call to UCLIBC_FIXUP_DOT_CONFIG to the relevant targets instead of depending on a new stamp file. The advantage of the stamp file, though, is that we avoid redoing the fixup unnecessarily. Moreover, in the light of the plan to extract the kconfig-specific bits into a separate kconfig-package infrastructure, the stamp file rules are more easily moved into such an infrastructure, while the alternative solution requires the package .mk file to explicitly call the FIXUP rules which may more easily be forgotten.

No longer depending on the configure step has the added bonus that 'uclibc-update-config' no longer needs the toolchain to be available, which makes:
make clean uclibc-menuconfig uclibc-update-config
much faster and user-friendly.

Additionally, make sure that 'make clean uclibc-update-config' works properly, by depending on .stamp_config_fixup_done so that the config file is present and fixed.

Fixes bug #7154 https://bugs.busybox.net/show_bug.cgi?id=7154

Signed-off-by Thomas De Schampheleire <[email protected]>

---
v3:
- further clarification in the commit message
v2:
- use $(Q)touch iso touch (Arnout)
- update commit message to clarify stamp file choice (Arnout)

rfc->patch:
- rebase
- rename .stamp_config_file_fixed into .stamp_config_fixup_done
- add dependency on .config from .stamp_config_file_fixed (Arnout)
- remove explicit call to UCLIBC_FIXUP_DOT_CONFIG from configure commands, and instead depend on .stamp_config_fixup_done.

 package/uclibc/uclibc.mk |  12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

package/uclibc/uclibc.mk +8 -4

diff -r 29ba03b43b12 -r f8c110cf3b83 package/uclibc/uclibc.mk
@@ -432,7 +432,6 @@
 endef
 
 define UCLIBC_CONFIGURE_CMDS
-	$(UCLIBC_FIXUP_DOT_CONFIG)
 	$(MAKE1) -C $(UCLIBC_DIR) \
 		$(UCLIBC_MAKE_FLAGS) \
 		PREFIX=$(STAGING_DIR) \

@@ -537,7 +536,11 @@
 $(UCLIBC_DIR)/.config: $(UCLIBC_CONFIG_FILE) | uclibc-patch
 	$(INSTALL) -m 0644 $(UCLIBC_CONFIG_FILE) $(UCLIBC_DIR)/.config
 
-$(UCLIBC_TARGET_CONFIGURE): $(UCLIBC_DIR)/.config
+$(UCLIBC_DIR)/.stamp_config_fixup_done: $(UCLIBC_DIR)/.config
+	$(UCLIBC_FIXUP_DOT_CONFIG)
+	$(Q)touch $@
+
+$(UCLIBC_TARGET_CONFIGURE): $(UCLIBC_DIR)/.stamp_config_fixup_done
 
 uclibc-menuconfig: $(UCLIBC_DIR)/.config
 	$(MAKE1) -C $(UCLIBC_DIR) \

@@ -546,9 +549,10 @@
 		DEVEL_PREFIX=/usr/ \
 		RUNTIME_PREFIX=$(STAGING_DIR)/ \
 		menuconfig
-	rm -f $(UCLIBC_DIR)/.stamp_{configured,built,target_installed,staging_installed}
+	rm -f $(UCLIBC_DIR)/.stamp_{config_fixup_done,configured,built}
+	rm -f $(UCLIBC_DIR)/.stamp_{target,staging}_installed
 
-uclibc-update-config: $(UCLIBC_DIR)/.stamp_configured
+uclibc-update-config: $(UCLIBC_DIR)/.stamp_config_fixup_done
 	cp -f $(UCLIBC_DIR)/.config $(UCLIBC_CONFIG_FILE)
 
 # Before uClibc is built, we must have the second stage cross-compiler

Thomas Petazzoni

2014-07-21 20:17 UTC #4 lore raw

PATCH v3 1/1 uclibc: update-config: preserve freshly configured settings

Dear Thomas De Schampheleire,

Quoted 25 lines · Thomas De Schampheleire

diff -r 29ba03b43b12 -r f8c110cf3b83 package/uclibc/uclibc.mk

package/uclibc/uclibc.mk +5 -2

@@ -432,7 +432,6 @@
 endef
 
 define UCLIBC_CONFIGURE_CMDS
-	$(UCLIBC_FIXUP_DOT_CONFIG)
 	$(MAKE1) -C $(UCLIBC_DIR) \
 		$(UCLIBC_MAKE_FLAGS) \
 		PREFIX=$(STAGING_DIR) \

@@ -537,7 +536,11 @@
 $(UCLIBC_DIR)/.config: $(UCLIBC_CONFIG_FILE) | uclibc-patch
 	$(INSTALL) -m 0644 $(UCLIBC_CONFIG_FILE) $(UCLIBC_DIR)/.config
 
-$(UCLIBC_TARGET_CONFIGURE): $(UCLIBC_DIR)/.config
+$(UCLIBC_DIR)/.stamp_config_fixup_done: $(UCLIBC_DIR)/.config
+	$(UCLIBC_FIXUP_DOT_CONFIG)
+	$(Q)touch $@
+
+$(UCLIBC_TARGET_CONFIGURE): $(UCLIBC_DIR)/.stamp_config_fixup_done
 
 uclibc-menuconfig: $(UCLIBC_DIR)/.config
 	$(MAKE1) -C $(UCLIBC_DIR) \

package/uclibc/uclibc.mk line 556

Quoted 10 lines

package/uclibc/uclibc.mk +3 -2

@@ -546,9 +549,10 @@
 		DEVEL_PREFIX=/usr/ \
 		RUNTIME_PREFIX=$(STAGING_DIR)/ \
 		menuconfig
-	rm -f $(UCLIBC_DIR)/.stamp_{configured,built,target_installed,staging_installed}
+	rm -f $(UCLIBC_DIR)/.stamp_{config_fixup_done,configured,built}
+	rm -f $(UCLIBC_DIR)/.stamp_{target,staging}_installed
 
-uclibc-update-config: $(UCLIBC_DIR)/.stamp_configured
+uclibc-update-config: $(UCLIBC_DIR)/.stamp_config_fixup_done
 	cp -f $(UCLIBC_DIR)/.config $(UCLIBC_CONFIG_FILE)

Is a separate stamp file actually needed here? This patch changes all references to $(UCLIBC_DIR)/.config as dependencies to references to $(UCLIBC_DIR)/.stamp_config_fixup_done as a dependency, and $(UCLIBC_DIR)/.stamp_config_fixup_done is the only target left having a dependency on $(UCLIBC_DIR)/.config. So what prevents from doing:

$(UCLIBC_DIR)/.config: $(UCLIBC_CONFIG_FILE) | uclibc-patch

	$(INSTALL) -m 0644 $(UCLIBC_CONFIG_FILE) $(UCLIBC_DIR)/.config
	$(UCLIBC_FIXUP_DOT_CONFIG)

I'm probably missing one use case, but I find
this .stamp_config_fixup_done stamp file not so nice, so I'd like to really understand why it's needed.

Thanks!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Thomas De Schampheleire

author

2014-07-22 11:11 UTC #5 lore raw

PATCH v3 1/1 uclibc: update-config: preserve freshly configured settings

Hi Thomas,

Quoted 47 lines · Thomas Petazzoni

Dear Thomas De Schampheleire,
Quoted 25 lines · Thomas De Schampheleire
diff -r 29ba03b43b12 -r f8c110cf3b83 package/uclibc/uclibc.mk
package/uclibc/uclibc.mk Mon Jun 16 20:18:23 2014 +0200 renamed from package/uclibc/uclibc.mk Sun Jun 22 10:37:22 2014 +0200 +5 -2
@@ -432,7 +432,6 @@
 endef
 
 define UCLIBC_CONFIGURE_CMDS
-     $(UCLIBC_FIXUP_DOT_CONFIG)
      $(MAKE1) -C $(UCLIBC_DIR) \
              $(UCLIBC_MAKE_FLAGS) \
              PREFIX=$(STAGING_DIR) \
@@ -537,7 +536,11 @@
 $(UCLIBC_DIR)/.config: $(UCLIBC_CONFIG_FILE) | uclibc-patch
      $(INSTALL) -m 0644 $(UCLIBC_CONFIG_FILE) $(UCLIBC_DIR)/.config
 
-$(UCLIBC_TARGET_CONFIGURE): $(UCLIBC_DIR)/.config
+$(UCLIBC_DIR)/.stamp_config_fixup_done: $(UCLIBC_DIR)/.config
+     $(UCLIBC_FIXUP_DOT_CONFIG)
+     $(Q)touch $@
+
+$(UCLIBC_TARGET_CONFIGURE): $(UCLIBC_DIR)/.stamp_config_fixup_done
 
 uclibc-menuconfig: $(UCLIBC_DIR)/.config
      $(MAKE1) -C $(UCLIBC_DIR) \
package/uclibc/uclibc.mk Mon Jun 16 20:18:23 2014 +0200 line 556
Quoted 10 lines
package/uclibc/uclibc.mk Mon Jun 16 20:18:23 2014 +0200 renamed from package/uclibc/uclibc.mk Sun Jun 22 10:37:22 2014 +0200 +3 -2
@@ -546,9 +549,10 @@
              DEVEL_PREFIX=/usr/ \
              RUNTIME_PREFIX=$(STAGING_DIR)/ \
              menuconfig
-     rm -f $(UCLIBC_DIR)/.stamp_{configured,built,target_installed,staging_installed}
+     rm -f $(UCLIBC_DIR)/.stamp_{config_fixup_done,configured,built}
+     rm -f $(UCLIBC_DIR)/.stamp_{target,staging}_installed
 
-uclibc-update-config: $(UCLIBC_DIR)/.stamp_configured
+uclibc-update-config: $(UCLIBC_DIR)/.stamp_config_fixup_done
      cp -f $(UCLIBC_DIR)/.config $(UCLIBC_CONFIG_FILE)
Is a separate stamp file actually needed here? This patch changes all references to $(UCLIBC_DIR)/.config as dependencies to references to $(UCLIBC_DIR)/.stamp_config_fixup_done as a dependency, and $(UCLIBC_DIR)/.stamp_config_fixup_done is the only target left having a dependency on $(UCLIBC_DIR)/.config. So what prevents from doing:
$(UCLIBC_DIR)/.config: $(UCLIBC_CONFIG_FILE) | uclibc-patch

Quoted 8 lines

        $(INSTALL) -m 0644 $(UCLIBC_CONFIG_FILE) $(UCLIBC_DIR)/.config
        $(UCLIBC_FIXUP_DOT_CONFIG)
?
I'm probably missing one use case, but I find
this .stamp_config_fixup_done stamp file not so nice, so I'd like to really understand why it's needed.

Suppose you go with your suggestion above, then if the user uses menuconfig to change the configuration, the .config file is updated by the menuconfig rule, but the fixup itself does no longer happen. This means that the user could enter a corrupted configuration without it being fixed. Same holds when the user manually changes .config.
I think the fixup should run after menuconfig, to ensure a valid configuration.

Note that you can avoid the extra stamp file by repeating the fixup call in several places, but this is not easily extractable in a kconfig-package infrastructure.
The rules as they are can almost be moved verbatim to a pkg-kconfig.mk with almost no changes in the uclibc.mk file itself.

Is this convincing to you? Do you see a better way?

Thanks,
Thomas

Thomas De Schampheleire

author

2014-07-22 18:24 UTC #6 lore raw

PATCH v3 1/1 uclibc: update-config: preserve freshly configured settings

Thomas De Schampheleire <[email protected]> schreef:

Quoted 67 lines

Hi Thomas,
Quoted 47 lines · Thomas Petazzoni
Dear Thomas De Schampheleire,
Quoted 25 lines · Thomas De Schampheleire
diff -r 29ba03b43b12 -r f8c110cf3b83 package/uclibc/uclibc.mk
package/uclibc/uclibc.mk Mon Jun 16 20:18:23 2014 +0200 renamed from package/uclibc/uclibc.mk Sun Jun 22 10:37:22 2014 +0200 +5 -2
@@ -432,7 +432,6 @@
 endef
 
 define UCLIBC_CONFIGURE_CMDS
-     $(UCLIBC_FIXUP_DOT_CONFIG)
      $(MAKE1) -C $(UCLIBC_DIR) \
              $(UCLIBC_MAKE_FLAGS) \
              PREFIX=$(STAGING_DIR) \
@@ -537,7 +536,11 @@
 $(UCLIBC_DIR)/.config: $(UCLIBC_CONFIG_FILE) | uclibc-patch
      $(INSTALL) -m 0644 $(UCLIBC_CONFIG_FILE) $(UCLIBC_DIR)/.config
 
-$(UCLIBC_TARGET_CONFIGURE): $(UCLIBC_DIR)/.config
+$(UCLIBC_DIR)/.stamp_config_fixup_done: $(UCLIBC_DIR)/.config
+     $(UCLIBC_FIXUP_DOT_CONFIG)
+     $(Q)touch $@
+
+$(UCLIBC_TARGET_CONFIGURE): $(UCLIBC_DIR)/.stamp_config_fixup_done
 
 uclibc-menuconfig: $(UCLIBC_DIR)/.config
      $(MAKE1) -C $(UCLIBC_DIR) \
package/uclibc/uclibc.mk Mon Jun 16 20:18:23 2014 +0200 line 556
Quoted 10 lines
package/uclibc/uclibc.mk Mon Jun 16 20:18:23 2014 +0200 renamed from package/uclibc/uclibc.mk Sun Jun 22 10:37:22 2014 +0200 +3 -2
@@ -546,9 +549,10 @@
              DEVEL_PREFIX=/usr/ \
              RUNTIME_PREFIX=$(STAGING_DIR)/ \
              menuconfig
-     rm -f $(UCLIBC_DIR)/.stamp_{configured,built,target_installed,staging_installed}
+     rm -f $(UCLIBC_DIR)/.stamp_{config_fixup_done,configured,built}
+     rm -f $(UCLIBC_DIR)/.stamp_{target,staging}_installed
 
-uclibc-update-config: $(UCLIBC_DIR)/.stamp_configured
+uclibc-update-config: $(UCLIBC_DIR)/.stamp_config_fixup_done
      cp -f $(UCLIBC_DIR)/.config $(UCLIBC_CONFIG_FILE)
Is a separate stamp file actually needed here? This patch changes all references to $(UCLIBC_DIR)/.config as dependencies to references to $(UCLIBC_DIR)/.stamp_config_fixup_done as a dependency, and $(UCLIBC_DIR)/.stamp_config_fixup_done is the only target left having a dependency on $(UCLIBC_DIR)/.config. So what prevents from doing:
$(UCLIBC_DIR)/.config: $(UCLIBC_CONFIG_FILE) | uclibc-patch
Quoted 8 lines
        $(INSTALL) -m 0644 $(UCLIBC_CONFIG_FILE) $(UCLIBC_DIR)/.config
        $(UCLIBC_FIXUP_DOT_CONFIG)
?
I'm probably missing one use case, but I find
this .stamp_config_fixup_done stamp file not so nice, so I'd like to really understand why it's needed.
Suppose you go with your suggestion above, then if the user uses menuconfig to change the configuration, the .config file is updated by the menuconfig rule, but the fixup itself does no longer happen. This means that the user could enter a corrupted configuration without it being fixed. Same holds when the user manually changes .config.
I think the fixup should run after menuconfig, to ensure a valid configuration.

Quoted 7 lines

Note that you can avoid the extra stamp file by repeating the fixup call in several places, but this is not easily extractable in a kconfig-package infrastructure.
The rules as they are can almost be moved verbatim to a pkg-kconfig.mk with almost no changes in the uclibc.mk file itself.
Is this convincing to you? Do you see a better way?

Note also that the existence of the extra stamp file will be hidden entirely by the kconfig-package infra that I'm working on...

Thomas De Schampheleire

author

2014-07-26 18:19 UTC #7 lore raw

PATCH v3 1/1 uclibc: update-config: preserve freshly configured settings

Hi Thomas,

Quoted 74 lines · Thomas De Schampheleire

Thomas De Schampheleire <[email protected]> schreef:
Quoted 73 lines
Hi Thomas,
Quoted 47 lines · Thomas Petazzoni
Dear Thomas De Schampheleire,
Quoted 25 lines · Thomas De Schampheleire
diff -r 29ba03b43b12 -r f8c110cf3b83 package/uclibc/uclibc.mk
package/uclibc/uclibc.mk Mon Jun 16 20:18:23 2014 +0200 renamed from package/uclibc/uclibc.mk Sun Jun 22 10:37:22 2014 +0200 +5 -2
@@ -432,7 +432,6 @@
 endef
 
 define UCLIBC_CONFIGURE_CMDS
-     $(UCLIBC_FIXUP_DOT_CONFIG)
      $(MAKE1) -C $(UCLIBC_DIR) \
              $(UCLIBC_MAKE_FLAGS) \
              PREFIX=$(STAGING_DIR) \
@@ -537,7 +536,11 @@
 $(UCLIBC_DIR)/.config: $(UCLIBC_CONFIG_FILE) | uclibc-patch
      $(INSTALL) -m 0644 $(UCLIBC_CONFIG_FILE) $(UCLIBC_DIR)/.config
 
-$(UCLIBC_TARGET_CONFIGURE): $(UCLIBC_DIR)/.config
+$(UCLIBC_DIR)/.stamp_config_fixup_done: $(UCLIBC_DIR)/.config
+     $(UCLIBC_FIXUP_DOT_CONFIG)
+     $(Q)touch $@
+
+$(UCLIBC_TARGET_CONFIGURE): $(UCLIBC_DIR)/.stamp_config_fixup_done
 
 uclibc-menuconfig: $(UCLIBC_DIR)/.config
      $(MAKE1) -C $(UCLIBC_DIR) \
package/uclibc/uclibc.mk Mon Jun 16 20:18:23 2014 +0200 line 556
Quoted 10 lines
package/uclibc/uclibc.mk Mon Jun 16 20:18:23 2014 +0200 renamed from package/uclibc/uclibc.mk Sun Jun 22 10:37:22 2014 +0200 +3 -2
@@ -546,9 +549,10 @@
              DEVEL_PREFIX=/usr/ \
              RUNTIME_PREFIX=$(STAGING_DIR)/ \
              menuconfig
-     rm -f $(UCLIBC_DIR)/.stamp_{configured,built,target_installed,staging_installed}
+     rm -f $(UCLIBC_DIR)/.stamp_{config_fixup_done,configured,built}
+     rm -f $(UCLIBC_DIR)/.stamp_{target,staging}_installed
 
-uclibc-update-config: $(UCLIBC_DIR)/.stamp_configured
+uclibc-update-config: $(UCLIBC_DIR)/.stamp_config_fixup_done
      cp -f $(UCLIBC_DIR)/.config $(UCLIBC_CONFIG_FILE)
Is a separate stamp file actually needed here? This patch changes all references to $(UCLIBC_DIR)/.config as dependencies to references to $(UCLIBC_DIR)/.stamp_config_fixup_done as a dependency, and $(UCLIBC_DIR)/.stamp_config_fixup_done is the only target left having a dependency on $(UCLIBC_DIR)/.config. So what prevents from doing:
$(UCLIBC_DIR)/.config: $(UCLIBC_CONFIG_FILE) | uclibc-patch
Quoted 8 lines
        $(INSTALL) -m 0644 $(UCLIBC_CONFIG_FILE) $(UCLIBC_DIR)/.config
        $(UCLIBC_FIXUP_DOT_CONFIG)
?
I'm probably missing one use case, but I find
this .stamp_config_fixup_done stamp file not so nice, so I'd like to really understand why it's needed.
Suppose you go with your suggestion above, then if the user uses menuconfig to change the configuration, the .config file is updated by the menuconfig rule, but the fixup itself does no longer happen. This means that the user could enter a corrupted configuration without it being fixed. Same holds when the user manually changes .config.
I think the fixup should run after menuconfig, to ensure a valid configuration.
Note that you can avoid the extra stamp file by repeating the fixup call in several places, but this is not easily extractable in a kconfig-package infrastructure.
The rules as they are can almost be moved verbatim to a pkg-kconfig.mk with almost no changes in the uclibc.mk file itself.

Quoted 5 lines

Quoted 1 line
Is this convincing to you? Do you see a better way?
Note also that the existence of the extra stamp file will be hidden entirely by the kconfig-package infra that I'm working on...

Based on the above, and based on the kconfig-package series that was sent, how do you feel about the extra stamp file? Did you get enough information to understand the reasons for its existence?

If possible, I would like to get this series in 2014.08.
In fact, I also would have liked the kconfig-package series in 2014.08 (the first part that was sent already), but I'm not sure there will be enough feedback to make it acceptable...

Thanks,
Thomas

Thomas Petazzoni

2014-07-29 21:48 UTC #8 lore raw

PATCH v3 1/1 uclibc: fix handling of configuration file

In reply to Thomas De Schampheleire

Dear Thomas De Schampheleire,

Quoted 2 lines · Thomas De Schampheleire

This patch series (part of it poster earlier as RFC) has as main goal to fix the uclibc kconfig handling with following requirements in mind:

I've applied the 3 patches of this series, following our discussion, and after seeing the kconfig-package infra patch series.

Thanks!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Thomas De Schampheleire

author

2014-07-30 05:21 UTC #9 lore raw

PATCH v3 1/1 uclibc: fix handling of configuration file

Quoted 10 lines · Thomas Petazzoni

Dear Thomas De Schampheleire,
Quoted 2 lines · Thomas De Schampheleire
This patch series (part of it poster earlier as RFC) has as main goal to fix the uclibc kconfig handling with following requirements in mind:
I've applied the 3 patches of this series, following our discussion, and after seeing the kconfig-package infra patch series.

Thank you!