[Buildroot] [PATCH 1/2] package/pkg-kconfig: hide defconfig targets if defconfig not supported

[Buildroot] [PATCH 1/2] package/pkg-kconfig: hide defconfig targets if defconfig not supported

[Arnout Vandecappelle (Essensium/Mind)]

When the savedefconfig target is not supported by a kconfig package,
(like is the case for busybox) it doesn't make sense to define
busybox-savedefconfig or busybox-update-defconfig. Calling these leads
to an error from busybox itself "No rule to make target
'savedefconfig'.", which may be confusing.

Only define the savedefconfig and update-defconfig target if
$(2)_KCONFIG_SUPPORTS_DEFCONFIG is YES.

Note that we also need to define it as phony in the condition, otherwise
'make busybox-update-defconfig' will just say "Nothing to be done" and
we really want the error "No rule to make target
'busybox-update-defconfig'".

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 package/pkg-kconfig.mk | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index 715c3e04ec..baad2e3baf 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -279,8 +279,11 @@ $(1)-check-configuration-done:
 		exit 1; \
 	fi
 
+ifeq ($$($(2)_KCONFIG_SUPPORTS_DEFCONFIG),YES)
 $(1)-savedefconfig: $(1)-check-configuration-done
 	$$(call kconfig-package-savedefconfig,$(2))
+.PHONY: $(1)-savedefconfig
+endif
 
 # Target to copy back the configuration to the source configuration file
 # Even though we could use 'cp --preserve-timestamps' here, the separate
@@ -289,6 +292,7 @@ $(1)-update-config: PKG=$(2)
 $(1)-update-config: $(1)-check-configuration-done
 	$$(call kconfig-package-update-config,$$($(2)_KCONFIG_DOTCONFIG))
 
+ifeq ($$($(2)_KCONFIG_SUPPORTS_DEFCONFIG),YES)
 # 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
@@ -296,6 +300,8 @@ $(1)-update-config: $(1)-check-configuration-done
 $(1)-update-defconfig: PKG=$(2)
 $(1)-update-defconfig: $(1)-savedefconfig
 	$$(call kconfig-package-update-config,defconfig)
+.PHONY: $(1)-update-defconfig
+endif
 
 # Target to output differences between the configuration obtained via the
 # defconfig + fragments (if any) and the current configuration.
@@ -315,9 +321,7 @@ endif # package enabled
 
 .PHONY: \
 	$(1)-update-config \
-	$(1)-update-defconfig \
 	$(1)-diff-config \
-	$(1)-savedefconfig \
 	$(1)-check-configuration-done \
 	$$($(2)_DIR)/.kconfig_editor_% \
 	$$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS))
-- 
2.31.1

_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

[Buildroot] [PATCH 2/2] package/pkg-kconfig: only define -update-{def, }config if supported

[Arnout Vandecappelle (Essensium/Mind)]

The foo-update-config and foo-update-defconfig targets only work if a
custom config file was defined. Remove those targets entirely if they
don't work to begin with.

This was originally handled with an error condition in the
kconfig-package-update-config macro. However, it makes more sense to
simply remove the target if it anyway can't be used.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 package/pkg-kconfig.mk | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index baad2e3baf..1e18d7dc07 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -12,12 +12,12 @@
 ################################################################################
 
 # Macro to update back the custom (def)config file
+# Must only be called if $(PKG)_KCONFIG_FILE is set and $(PKG)_KCONFIG_DEFCONFIG)
+# is not set.
 # $(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)
 	$(Q)if [ -d $($(PKG)_KCONFIG_FILE) ]; then \
 		echo "Unable to perform $(@) when $($(PKG)_KCONFIG_FILE) is a directory"; \
 		exit 1; \
@@ -285,12 +285,14 @@ $(1)-savedefconfig: $(1)-check-configuration-done
 .PHONY: $(1)-savedefconfig
 endif
 
+ifeq ($$($(2)_KCONFIG_DEFCONFIG),)
 # 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
 	$$(call kconfig-package-update-config,$$($(2)_KCONFIG_DOTCONFIG))
+.PHONY: $(1)-update-config
 
 ifeq ($$($(2)_KCONFIG_SUPPORTS_DEFCONFIG),YES)
 # Note: make sure the timestamp of the stored configuration is not newer than
@@ -303,6 +305,8 @@ $(1)-update-defconfig: $(1)-savedefconfig
 .PHONY: $(1)-update-defconfig
 endif
 
+endif
+
 # Target to output differences between the configuration obtained via the
 # defconfig + fragments (if any) and the current configuration.
 # Note: it preserves the timestamp of the current configuration when moving it
@@ -320,7 +324,6 @@ $(1)-diff-config: $(1)-check-configuration-done
 endif # package enabled
 
 .PHONY: \
-	$(1)-update-config \
 	$(1)-diff-config \
 	$(1)-check-configuration-done \
 	$$($(2)_DIR)/.kconfig_editor_% \
-- 
2.31.1

_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/2] package/pkg-kconfig: hide defconfig targets if defconfig not supported

[Yann E. MORIN]

Arnout, All,

On 2021-07-25 16:11 +0200, Arnout Vandecappelle (Essensium/Mind) spake thusly:
> When the savedefconfig target is not supported by a kconfig package,
> (like is the case for busybox) it doesn't make sense to define
> busybox-savedefconfig or busybox-update-defconfig. Calling these leads
> to an error from busybox itself "No rule to make target
> 'savedefconfig'.", which may be confusing.
> 
> Only define the savedefconfig and update-defconfig target if
> $(2)_KCONFIG_SUPPORTS_DEFCONFIG is YES.
> 
> Note that we also need to define it as phony in the condition, otherwise
> 'make busybox-update-defconfig' will just say "Nothing to be done" and
> we really want the error "No rule to make target
> 'busybox-update-defconfig'".
> 
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Applied to master, thanks. But see a nit below...

> ---
>  package/pkg-kconfig.mk | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> index 715c3e04ec..baad2e3baf 100644
> --- a/package/pkg-kconfig.mk
> +++ b/package/pkg-kconfig.mk
> @@ -279,8 +279,11 @@ $(1)-check-configuration-done:
>  		exit 1; \
>  	fi
>  
> +ifeq ($$($(2)_KCONFIG_SUPPORTS_DEFCONFIG),YES)
>  $(1)-savedefconfig: $(1)-check-configuration-done
>  	$$(call kconfig-package-savedefconfig,$(2))
> +.PHONY: $(1)-savedefconfig
> +endif
>  
>  # Target to copy back the configuration to the source configuration file
>  # Even though we could use 'cp --preserve-timestamps' here, the separate
> @@ -289,6 +292,7 @@ $(1)-update-config: PKG=$(2)
>  $(1)-update-config: $(1)-check-configuration-done
>  	$$(call kconfig-package-update-config,$$($(2)_KCONFIG_DOTCONFIG))
>  
> +ifeq ($$($(2)_KCONFIG_SUPPORTS_DEFCONFIG),YES)
>  # 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
> @@ -296,6 +300,8 @@ $(1)-update-config: $(1)-check-configuration-done
>  $(1)-update-defconfig: PKG=$(2)
>  $(1)-update-defconfig: $(1)-savedefconfig
>  	$$(call kconfig-package-update-config,defconfig)
> +.PHONY: $(1)-update-defconfig

I think that the usual convention is to put the PHONY rule right before
the corresponding rule, not after, so I've just changed that (in both
places in this patch, and in the following patch too).

Thanks!

Regards,
Yann E. MORIN.

> +endif
>  
>  # Target to output differences between the configuration obtained via the
>  # defconfig + fragments (if any) and the current configuration.
> @@ -315,9 +321,7 @@ endif # package enabled
>  
>  .PHONY: \
>  	$(1)-update-config \
> -	$(1)-update-defconfig \
>  	$(1)-diff-config \
> -	$(1)-savedefconfig \
>  	$(1)-check-configuration-done \
>  	$$($(2)_DIR)/.kconfig_editor_% \
>  	$$(addprefix $(1)-,$$($(2)_KCONFIG_EDITORS))
> -- 
> 2.31.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot