infra/pkg-config: fix updating back (def)config files

PATCH New Yann E. MORIN 2 participants 5 messages Dec 1, 2017 – Mar 31, 2018

lore

Share & tools

lore

Yann E. MORIN

author

2017-12-01 15:09 UTC #0 lore raw

cover infra/pkg-config: fix updating back (def)config files

Hello All!

As reported by a newbie or IRC, saving back the (def)config files is buggy when the path provided by the user points to a diurectory instead of a (existing or not) file.

This small series, after comonalising the updateing code, fixes that bug.

Regards,
Yann E. MORIN.

The following changes since commit 57dcad243e6daefefbe21109e1fc97272053a7a0

Kickoff 2018.02 cycle (2017-12-01 10:31:43 +0100)

are available in the git repository at:

git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to d2b584aa49a8597e975fcc4dfe4b568011798318

infra/pkg-kconfig: do not update-(def)config to a directory (2017-12-01 15:38:56 +0100)

----------------------------------------------------------------
Yann E. MORIN (3):
infra/pkg-kconfig: comonalise update-(def)config code infra/pkg-config: hide away non-critical commands infra/pkg-kconfig: do not update-(def)config to a directory

 package/pkg-kconfig.mk | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Yann E. MORIN

author

2017-12-01 15:09 UTC #1 lore raw

PATCH 1/3 infra/pkg-kconfig: comonalise update-(def)config code

Except for the nice human-friendly reminder of the command that the user was just running, and the name of the file to copu from, those two rules are exactly the same.

Make that a common macro that is shared, so that it's easier to add more checks, and to simplify maintenance.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 package/pkg-kconfig.mk | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

package/pkg-kconfig.mk +15 ~~-12~~

index 0402f81ffe..4bd7161b1a 100644
@@ -11,6 +11,17 @@
 #
 ################################################################################
 
+# Macro to update back the custom (def)config file
+# $(1): file to copy from
+define kconfig-package-update-config
+	@$(if $($(PKG)_KCONFIG_FRAGMENT_FILES), \
+		echo "Unable to perform $(@) when fragment files are set"; exit 1)
+	@$(if $($(PKG)_KCONFIG_DEFCONFIG), \
+		echo "Unable to perform $(@) when using a defconfig rule"; exit 1)
+	cp -f $($(PKG)_DIR)/$(1) $($(PKG)_KCONFIG_FILE)
+	touch --reference $($(PKG)_DIR)/$($(PKG)_KCONFIG_DOTCONFIG) $($(PKG)_KCONFIG_FILE)
+endef
+
 ################################################################################
 # inner-kconfig-package -- generates the make targets needed to support a
 # kconfig package

@@ -204,25 +215,17 @@ $(1)-savedefconfig: $(1)-check-configuration-done
 # Target to copy back the configuration to the source configuration file
 # Even though we could use 'cp --preserve-timestamps' here, the separate
 # cp and 'touch --reference' is used for symmetry with $(1)-update-defconfig.
+$(1)-update-config: PKG=$(2)
 $(1)-update-config: $(1)-check-configuration-done
-	@$$(if $$($(2)_KCONFIG_FRAGMENT_FILES), \
-		echo "Unable to perform $(1)-update-config when fragment files are set"; exit 1)
-	@$$(if $$($(2)_KCONFIG_DEFCONFIG), \
-		echo "Unable to perform $(1)-update-config when using a defconfig rule"; exit 1)
-	cp -f $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) $$($(2)_KCONFIG_FILE)
-	touch --reference $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) $$($(2)_KCONFIG_FILE)
+	$$(call kconfig-package-update-config,$$($(2)_KCONFIG_DOTCONFIG))
 
 # Note: make sure the timestamp of the stored configuration is not newer than
 # the .config to avoid a useless rebuild. Note that, contrary to
 # $(1)-update-config, the reference for 'touch' is _not_ the file from which
 # we copy.
+$(1)-update-defconfig: PKG=$(2)
 $(1)-update-defconfig: $(1)-savedefconfig
-	@$$(if $$($(2)_KCONFIG_FRAGMENT_FILES), \
-		echo "Unable to perform $(1)-update-defconfig when fragment files are set"; exit 1)
-	@$$(if $$($(2)_KCONFIG_DEFCONFIG), \
-		echo "Unable to perform $(1)-update-defconfig when using a defconfig rule"; exit 1)
-	cp -f $$($(2)_DIR)/defconfig $$($(2)_KCONFIG_FILE)
-	touch --reference $$($(2)_DIR)/$$($(2)_KCONFIG_DOTCONFIG) $$($(2)_KCONFIG_FILE)
+	$$(call kconfig-package-update-config,defconfig)
 
 endif # package enabled

Signature · 1 line

2.11.0

Yann E. MORIN

author

2017-12-01 15:09 UTC #2 lore raw

PATCH 2/3 infra/pkg-config: hide away non-critical commands

In reply to Yann E. MORIN

When updating back the (def)config file, the touch command is not very useful by default, so hide it away.

We do not hide away the cp command, as that could confuse the user under some circunmstances. For example, when the toolchain does not yet exist, the uClibc buildsystem will complain multiple times about gcc not being found, like so:

make[2]: /home/ymorin/dev/buildroot/O/host/bin/i686-buildroot-linux-uclibc-gcc: Command not found

(Note that we can not suppress those warnings, as they are on stderr, and we still want to see stderr in case of real errors).

So, if we were to hide the cp command, the user could be left confused, even though we were sucessful in updating back the (def)config file.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 package/pkg-kconfig.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

package/pkg-kconfig.mk +1 -1

index 4bd7161b1a..0da6d0ab4e 100644
@@ -19,7 +19,7 @@ define kconfig-package-update-config
 	@$(if $($(PKG)_KCONFIG_DEFCONFIG), \
 		echo "Unable to perform $(@) when using a defconfig rule"; exit 1)
 	cp -f $($(PKG)_DIR)/$(1) $($(PKG)_KCONFIG_FILE)
-	touch --reference $($(PKG)_DIR)/$($(PKG)_KCONFIG_DOTCONFIG) $($(PKG)_KCONFIG_FILE)
+	$(Q)touch --reference $($(PKG)_DIR)/$($(PKG)_KCONFIG_DOTCONFIG) $($(PKG)_KCONFIG_FILE)
 endef
 
 ################################################################################

Signature · 1 line

2.11.0

Yann E. MORIN

author

2017-12-01 15:09 UTC #3 lore raw

PATCH 3/3 infra/pkg-kconfig: do not update-(def)config to a directory

In reply to Yann E. MORIN

Currently, if the user specifies the path to an existing directory as the path to the custom (def)config file (FOO_KCONFIG_FILE), then we happily create a file in there, either 'defconfig' or '.config' (or whatever the .config is named for that package), depending on whether we're saving a defconfig or a full config.

So, we could save the file, but then the Buildroot defconfig file that contains that path would no longer be reusable as-is, because we interpret that path as a path to a file.

Furthermore, if the directory-portion of FOO_KCONFIG_FILE does not exist yet, the update would fail, because cp does not create missing directory components.

So we fix that by adding an explicit test for the directory-ness of the target file, and then an explicit mkdir to create missing directory components.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 package/pkg-kconfig.mk | 5 +++++
 1 file changed, 5 insertions(+)

package/pkg-kconfig.mk +5 -0

index 0da6d0ab4e..81bba5220c 100644
@@ -18,6 +18,11 @@ define kconfig-package-update-config
 		echo "Unable to perform $(@) when fragment files are set"; exit 1)
 	@$(if $($(PKG)_KCONFIG_DEFCONFIG), \
 		echo "Unable to perform $(@) when using a defconfig rule"; exit 1)
+	$(Q)if [ -d $($(PKG)_KCONFIG_FILE) ]; then \
+		echo "Unable to perform $(@) when $($(PKG)_KCONFIG_FILE) is a directory"; \
+		exit 1; \
+	fi
+	$(Q)mkdir -p $(dir $($(PKG)_KCONFIG_FILE))
 	cp -f $($(PKG)_DIR)/$(1) $($(PKG)_KCONFIG_FILE)
 	$(Q)touch --reference $($(PKG)_DIR)/$($(PKG)_KCONFIG_DOTCONFIG) $($(PKG)_KCONFIG_FILE)
 endef

Signature · 1 line

2.11.0

Thomas Petazzoni

2018-03-31 20:35 UTC #4 lore raw

cover infra/pkg-config: fix updating back (def)config files

In reply to Yann E. MORIN

Hello,

Quoted 4 lines · Yann E. MORIN

Yann E. MORIN (3):
infra/pkg-kconfig: comonalise update-(def)config code infra/pkg-config: hide away non-critical commands infra/pkg-kconfig: do not update-(def)config to a directory

Series applied, thanks!

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com