Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] Main Makefile Have "make savedefconfig" show where the file is being written to.
@ 2018-02-27 11:28 Christopher McCrory
  2018-02-27 21:02 ` Thomas Petazzoni
  2018-03-01 19:50 ` Arnout Vandecappelle
  0 siblings, 2 replies; 6+ messages in thread
From: Christopher McCrory @ 2018-02-27 11:28 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Christopher McCrory <chrismcc@gmail.com>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index ec39bcdb9c..4ac2435a9c 100644
--- a/Makefile
+++ b/Makefile
@@ -927,6 +927,7 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 		--savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
 		$(CONFIG_CONFIG_IN)
 	@$(SED) '/BR2_DEFCONFIG=/d' $(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig)
+	@echo "Saved to $(DEFCONFIG)"
 
 .PHONY: defconfig savedefconfig
 
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] Main Makefile Have "make savedefconfig" show where the file is being written to.
  2018-02-27 11:28 [Buildroot] [PATCH 1/1] Main Makefile Have "make savedefconfig" show where the file is being written to Christopher McCrory
@ 2018-02-27 21:02 ` Thomas Petazzoni
  2018-03-01 19:50 ` Arnout Vandecappelle
  1 sibling, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2018-02-27 21:02 UTC (permalink / raw)
  To: buildroot

Hello,

On Tue, 27 Feb 2018 03:28:18 -0800, Christopher McCrory wrote:
> Signed-off-by: Christopher McCrory <chrismcc@gmail.com>

The commit title should be shorter, perhaps:

	Makefile: show defconfig file being saved in 'savedefconfig'

and then the commit log should expand on that, to explain why this is
useful.

> ---
>  Makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Makefile b/Makefile
> index ec39bcdb9c..4ac2435a9c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -927,6 +927,7 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
>  		--savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
>  		$(CONFIG_CONFIG_IN)
>  	@$(SED) '/BR2_DEFCONFIG=/d' $(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig)
> +	@echo "Saved to $(DEFCONFIG)"

I'm personally OK with this, and believe it makes sense. I've always
found it weird that we automatically save back to the defconfig we have
started from, but some people liked that. More than once I've been
puzzled after a "make savedefconfig" to not find a "defconfig" file in
the current folder, because the defconfig had been updated in
configs/<something>_defconfig. So to me, your change makes sense.

Could you respin after fixing the commit title, and expanding a bit the
commit log ?

Thanks a lot!

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] Main Makefile Have "make savedefconfig" show where the file is being written to.
  2018-02-27 11:28 [Buildroot] [PATCH 1/1] Main Makefile Have "make savedefconfig" show where the file is being written to Christopher McCrory
  2018-02-27 21:02 ` Thomas Petazzoni
@ 2018-03-01 19:50 ` Arnout Vandecappelle
  2018-03-01 21:21   ` Peter Korsgaard
  1 sibling, 1 reply; 6+ messages in thread
From: Arnout Vandecappelle @ 2018-03-01 19:50 UTC (permalink / raw)
  To: buildroot



On 27-02-18 12:28, Christopher McCrory wrote:
> Signed-off-by: Christopher McCrory <chrismcc@gmail.com>
> ---
>  Makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Makefile b/Makefile
> index ec39bcdb9c..4ac2435a9c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -927,6 +927,7 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
>  		--savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
>  		$(CONFIG_CONFIG_IN)
>  	@$(SED) '/BR2_DEFCONFIG=/d' $(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig)
> +	@echo "Saved to $(DEFCONFIG)"

 If BR2_DEFCONFIG is not set, you'll get "Saved to " which is a bit weird. So
change into:

	@echo "Saved to $(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig)"

 But now $(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) is used 3
times, so perhaps it's worth adding a variable for it. I can't think of a good
name for it unfortunately.

 Regards,
 Arnout

>  
>  .PHONY: defconfig savedefconfig
>  
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] Main Makefile Have "make savedefconfig" show where the file is being written to.
  2018-03-01 19:50 ` Arnout Vandecappelle
@ 2018-03-01 21:21   ` Peter Korsgaard
  2018-03-02  8:05     ` Arnout Vandecappelle
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Korsgaard @ 2018-03-01 21:21 UTC (permalink / raw)
  To: buildroot

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

 > On 27-02-18 12:28, Christopher McCrory wrote:
 >> Signed-off-by: Christopher McCrory <chrismcc@gmail.com>
 >> ---
 >> Makefile | 1 +
 >> 1 file changed, 1 insertion(+)
 >> 
 >> diff --git a/Makefile b/Makefile
 >> index ec39bcdb9c..4ac2435a9c 100644
 >> --- a/Makefile
 >> +++ b/Makefile
 >> @@ -927,6 +927,7 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 >> --savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
 >> $(CONFIG_CONFIG_IN)
 >> @$(SED) '/BR2_DEFCONFIG=/d' $(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig)
 >> +	@echo "Saved to $(DEFCONFIG)"

 >  If BR2_DEFCONFIG is not set, you'll get "Saved to " which is a bit weird. So
 > change into:

 > 	@echo "Saved to $(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig)"

 >  But now $(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) is used 3
 > times, so perhaps it's worth adding a variable for it. I can't think of a good
 > name for it unfortunately.

Perhaps we could just set DEFCONFIG to $(CONFIG_DIR)/defconfig if empty?

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] Main Makefile Have "make savedefconfig" show where the file is being written to.
  2018-03-01 21:21   ` Peter Korsgaard
@ 2018-03-02  8:05     ` Arnout Vandecappelle
  2018-03-02  8:17       ` Peter Korsgaard
  0 siblings, 1 reply; 6+ messages in thread
From: Arnout Vandecappelle @ 2018-03-02  8:05 UTC (permalink / raw)
  To: buildroot



On 01-03-18 22:21, Peter Korsgaard wrote:
>>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:
> 
>  > On 27-02-18 12:28, Christopher McCrory wrote:
>  >> Signed-off-by: Christopher McCrory <chrismcc@gmail.com>
>  >> ---
>  >> Makefile | 1 +
>  >> 1 file changed, 1 insertion(+)
>  >> 
>  >> diff --git a/Makefile b/Makefile
>  >> index ec39bcdb9c..4ac2435a9c 100644
>  >> --- a/Makefile
>  >> +++ b/Makefile
>  >> @@ -927,6 +927,7 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
>  >> --savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
>  >> $(CONFIG_CONFIG_IN)
>  >> @$(SED) '/BR2_DEFCONFIG=/d' $(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig)
>  >> +	@echo "Saved to $(DEFCONFIG)"
> 
>  >  If BR2_DEFCONFIG is not set, you'll get "Saved to " which is a bit weird. So
>  > change into:
> 
>  > 	@echo "Saved to $(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig)"
> 
>  >  But now $(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) is used 3
>  > times, so perhaps it's worth adding a variable for it. I can't think of a good
>  > name for it unfortunately.
> 
> Perhaps we could just set DEFCONFIG to $(CONFIG_DIR)/defconfig if empty?

 I looked at that, but in the 'make defconfig' rule we have to be able to
distinguish between BR2_DEFCONFIG is set or not. Since BR2_DEFCONFIG may come
out of the .config file, it may or may not be quoted. That's why we need to use
the qstrip'ed version DEFCONFIG in the condition.

 This stuff is complicated :-) It's one of these examples where, when I started
adding the feature, I thought "this is trivial" but then the real world kicks in.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] Main Makefile Have "make savedefconfig" show where the file is being written to.
  2018-03-02  8:05     ` Arnout Vandecappelle
@ 2018-03-02  8:17       ` Peter Korsgaard
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Korsgaard @ 2018-03-02  8:17 UTC (permalink / raw)
  To: buildroot

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

 >> Perhaps we could just set DEFCONFIG to $(CONFIG_DIR)/defconfig if empty?

 >  I looked at that, but in the 'make defconfig' rule we have to be able to
 > distinguish between BR2_DEFCONFIG is set or not. Since BR2_DEFCONFIG may come
 > out of the .config file, it may or may not be quoted. That's why we need to use
 > the qstrip'ed version DEFCONFIG in the condition.

Ahh yes.

 >  This stuff is complicated :-) It's one of these examples where, when I started
 > adding the feature, I thought "this is trivial" but then the real world kicks in.

I know the feeling ;)

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-03-02  8:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-27 11:28 [Buildroot] [PATCH 1/1] Main Makefile Have "make savedefconfig" show where the file is being written to Christopher McCrory
2018-02-27 21:02 ` Thomas Petazzoni
2018-03-01 19:50 ` Arnout Vandecappelle
2018-03-01 21:21   ` Peter Korsgaard
2018-03-02  8:05     ` Arnout Vandecappelle
2018-03-02  8:17       ` Peter Korsgaard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox