* [Buildroot] [PATCH] Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit @ 2014-10-21 8:42 Michal Sojka 2014-10-21 16:58 ` Yann E. MORIN 2014-10-24 23:58 ` Peter Korsgaard 0 siblings, 2 replies; 10+ messages in thread From: Michal Sojka @ 2014-10-21 8:42 UTC (permalink / raw) To: buildroot If the BR2_GLOBAL_PATCH_DIR is misspelled in the .config or if somebody renamed the existing directory with patches, buildroot happily builds everything but without the patches. As this can lead to surprising results, it is better to fail and give a user a message that BR2_GLOBAL_PATCH_DIR is wrongly configured. Signed-off-by: Michal Sojka <sojka@merica.cz> --- package/pkg-generic.mk | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index 259ee02..46d92aa 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -162,6 +162,11 @@ $(BUILD_DIR)/%/.stamp_patched: $(Q)touch $@ @$(call step_end,patch) +# Check that all directories specified in BR2_GLOBAL_PATCH_DIR exist. +$(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\ + $(if $(wildcard $(dir)),,\ + $(error BR2_GLOBAL_PATCH_DIR contains nonexistent directory $(dir)))) + # Configure $(BUILD_DIR)/%/.stamp_configured: @$(call step_start,configure) -- 2.1.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH] Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit 2014-10-21 8:42 [Buildroot] [PATCH] Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit Michal Sojka @ 2014-10-21 16:58 ` Yann E. MORIN 2014-10-22 11:26 ` Maxime Hadjinlian 2014-10-24 23:58 ` Peter Korsgaard 1 sibling, 1 reply; 10+ messages in thread From: Yann E. MORIN @ 2014-10-21 16:58 UTC (permalink / raw) To: buildroot Michal, All, On 2014-10-21 10:42 +0200, Michal Sojka spake thusly: > If the BR2_GLOBAL_PATCH_DIR is misspelled in the .config or if somebody > renamed the existing directory with patches, buildroot happily builds > everything but without the patches. As this can lead to surprising > results, Indeed... :-/ > it is better to fail and give a user a message that > BR2_GLOBAL_PATCH_DIR is wrongly configured. > > Signed-off-by: Michal Sojka <sojka@merica.cz> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Regards, Yann E. MORIN. > --- > package/pkg-generic.mk | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index 259ee02..46d92aa 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -162,6 +162,11 @@ $(BUILD_DIR)/%/.stamp_patched: > $(Q)touch $@ > @$(call step_end,patch) > > +# Check that all directories specified in BR2_GLOBAL_PATCH_DIR exist. > +$(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\ > + $(if $(wildcard $(dir)),,\ > + $(error BR2_GLOBAL_PATCH_DIR contains nonexistent directory $(dir)))) > + > # Configure > $(BUILD_DIR)/%/.stamp_configured: > @$(call step_start,configure) > -- > 2.1.1 > > _______________________________________________ > buildroot mailing list > buildroot at 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 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH] Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit 2014-10-21 16:58 ` Yann E. MORIN @ 2014-10-22 11:26 ` Maxime Hadjinlian 2014-10-22 11:43 ` Michal Sojka 0 siblings, 1 reply; 10+ messages in thread From: Maxime Hadjinlian @ 2014-10-22 11:26 UTC (permalink / raw) To: buildroot Hi Michal, Yann, all On Tue, Oct 21, 2014 at 6:58 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > Michal, All, > > On 2014-10-21 10:42 +0200, Michal Sojka spake thusly: >> If the BR2_GLOBAL_PATCH_DIR is misspelled in the .config or if somebody >> renamed the existing directory with patches, buildroot happily builds >> everything but without the patches. As this can lead to surprising >> results, >> --- >> package/pkg-generic.mk | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk >> index 259ee02..46d92aa 100644 >> --- a/package/pkg-generic.mk >> +++ b/package/pkg-generic.mk >> @@ -162,6 +162,11 @@ $(BUILD_DIR)/%/.stamp_patched: >> $(Q)touch $@ >> @$(call step_end,patch) >> >> +# Check that all directories specified in BR2_GLOBAL_PATCH_DIR exist. >> +$(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\ >> + $(if $(wildcard $(dir)),,\ >> + $(error BR2_GLOBAL_PATCH_DIR contains nonexistent directory $(dir)))) >> + On a small note, since we already do a '[...] test -d $${D} [...]', why don't you simply add an else condition with the error message ? >> # Configure >> $(BUILD_DIR)/%/.stamp_configured: >> @$(call step_start,configure) >> -- >> 2.1.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH] Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit 2014-10-22 11:26 ` Maxime Hadjinlian @ 2014-10-22 11:43 ` Michal Sojka 2014-10-22 11:54 ` Maxime Hadjinlian 0 siblings, 1 reply; 10+ messages in thread From: Michal Sojka @ 2014-10-22 11:43 UTC (permalink / raw) To: buildroot On Wed, Oct 22 2014, Maxime Hadjinlian wrote: > Hi Michal, Yann, all > > On Tue, Oct 21, 2014 at 6:58 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote: >> Michal, All, >> >> On 2014-10-21 10:42 +0200, Michal Sojka spake thusly: >>> If the BR2_GLOBAL_PATCH_DIR is misspelled in the .config or if somebody >>> renamed the existing directory with patches, buildroot happily builds >>> everything but without the patches. As this can lead to surprising >>> results, >>> --- >>> package/pkg-generic.mk | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk >>> index 259ee02..46d92aa 100644 >>> --- a/package/pkg-generic.mk >>> +++ b/package/pkg-generic.mk >>> @@ -162,6 +162,11 @@ $(BUILD_DIR)/%/.stamp_patched: >>> $(Q)touch $@ >>> @$(call step_end,patch) >>> >>> +# Check that all directories specified in BR2_GLOBAL_PATCH_DIR exist. >>> +$(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\ >>> + $(if $(wildcard $(dir)),,\ >>> + $(error BR2_GLOBAL_PATCH_DIR contains nonexistent directory $(dir)))) >>> + > On a small note, since we already do a '[...] test -d $${D} [...]', > why don't you simply add an else condition with the error message ? Because $(D) is in fact $(BR2_GLOBAL_PATCH_DIR)/$(RAWNAME), which is not required to exist for every package. Best regards, -Michal ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH] Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit 2014-10-22 11:43 ` Michal Sojka @ 2014-10-22 11:54 ` Maxime Hadjinlian 2014-10-22 14:01 ` Michal Sojka 2014-10-22 16:35 ` Arnout Vandecappelle 0 siblings, 2 replies; 10+ messages in thread From: Maxime Hadjinlian @ 2014-10-22 11:54 UTC (permalink / raw) To: buildroot On Wed, Oct 22, 2014 at 1:43 PM, Michal Sojka <sojka@merica.cz> wrote: > On Wed, Oct 22 2014, Maxime Hadjinlian wrote: >> Hi Michal, Yann, all >> >> On Tue, Oct 21, 2014 at 6:58 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote: >>> Michal, All, >>> >>> On 2014-10-21 10:42 +0200, Michal Sojka spake thusly: >>>> If the BR2_GLOBAL_PATCH_DIR is misspelled in the .config or if somebody >>>> renamed the existing directory with patches, buildroot happily builds >>>> everything but without the patches. As this can lead to surprising >>>> results, >>>> --- >>>> package/pkg-generic.mk | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk >>>> index 259ee02..46d92aa 100644 >>>> --- a/package/pkg-generic.mk >>>> +++ b/package/pkg-generic.mk >>>> @@ -162,6 +162,11 @@ $(BUILD_DIR)/%/.stamp_patched: >>>> $(Q)touch $@ >>>> @$(call step_end,patch) >>>> >>>> +# Check that all directories specified in BR2_GLOBAL_PATCH_DIR exist. >>>> +$(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\ >>>> + $(if $(wildcard $(dir)),,\ >>>> + $(error BR2_GLOBAL_PATCH_DIR contains nonexistent directory $(dir)))) >>>> + >> On a small note, since we already do a '[...] test -d $${D} [...]', >> why don't you simply add an else condition with the error message ? > > Because $(D) is in fact $(BR2_GLOBAL_PATCH_DIR)/$(RAWNAME), which is not > required to exist for every package. Ah yes indeed. Sorry. Other question, why don't put that bit of code at the beginning of the target stamp_patched ? Or at least in the target stamp_patched ? > > Best regards, > -Michal ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH] Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit 2014-10-22 11:54 ` Maxime Hadjinlian @ 2014-10-22 14:01 ` Michal Sojka 2014-10-22 14:26 ` Maxime Hadjinlian 2014-10-22 16:35 ` Arnout Vandecappelle 1 sibling, 1 reply; 10+ messages in thread From: Michal Sojka @ 2014-10-22 14:01 UTC (permalink / raw) To: buildroot On Wed, Oct 22 2014, Maxime Hadjinlian wrote: > On Wed, Oct 22, 2014 at 1:43 PM, Michal Sojka <sojka@merica.cz> wrote: >> On Wed, Oct 22 2014, Maxime Hadjinlian wrote: >>> Hi Michal, Yann, all >>> >>> On Tue, Oct 21, 2014 at 6:58 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote: >>>> Michal, All, >>>> >>>> On 2014-10-21 10:42 +0200, Michal Sojka spake thusly: >>>>> If the BR2_GLOBAL_PATCH_DIR is misspelled in the .config or if somebody >>>>> renamed the existing directory with patches, buildroot happily builds >>>>> everything but without the patches. As this can lead to surprising >>>>> results, >>>>> --- >>>>> package/pkg-generic.mk | 5 +++++ >>>>> 1 file changed, 5 insertions(+) >>>>> >>>>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk >>>>> index 259ee02..46d92aa 100644 >>>>> --- a/package/pkg-generic.mk >>>>> +++ b/package/pkg-generic.mk >>>>> @@ -162,6 +162,11 @@ $(BUILD_DIR)/%/.stamp_patched: >>>>> $(Q)touch $@ >>>>> @$(call step_end,patch) >>>>> >>>>> +# Check that all directories specified in BR2_GLOBAL_PATCH_DIR exist. >>>>> +$(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\ >>>>> + $(if $(wildcard $(dir)),,\ >>>>> + $(error BR2_GLOBAL_PATCH_DIR contains nonexistent directory $(dir)))) >>>>> + >>> On a small note, since we already do a '[...] test -d $${D} [...]', >>> why don't you simply add an else condition with the error message ? >> >> Because $(D) is in fact $(BR2_GLOBAL_PATCH_DIR)/$(RAWNAME), which is not >> required to exist for every package. > Ah yes indeed. Sorry. > Other question, why don't put that bit of code at the beginning of the > target stamp_patched ? Or at least in the target stamp_patched ? Because the same check would be performed many times for every package. This way, things are checked only once, while the Makefile is read. -Michal ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH] Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit 2014-10-22 14:01 ` Michal Sojka @ 2014-10-22 14:26 ` Maxime Hadjinlian 0 siblings, 0 replies; 10+ messages in thread From: Maxime Hadjinlian @ 2014-10-22 14:26 UTC (permalink / raw) To: buildroot On Wed, Oct 22, 2014 at 4:01 PM, Michal Sojka <sojka@merica.cz> wrote: > On Wed, Oct 22 2014, Maxime Hadjinlian wrote: >> On Wed, Oct 22, 2014 at 1:43 PM, Michal Sojka <sojka@merica.cz> wrote: >>> On Wed, Oct 22 2014, Maxime Hadjinlian wrote: >>>> Hi Michal, Yann, all >>>> >>>> On Tue, Oct 21, 2014 at 6:58 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote: >>>>> Michal, All, >>>>> >>>>> On 2014-10-21 10:42 +0200, Michal Sojka spake thusly: >>>>>> If the BR2_GLOBAL_PATCH_DIR is misspelled in the .config or if somebody >>>>>> renamed the existing directory with patches, buildroot happily builds >>>>>> everything but without the patches. As this can lead to surprising >>>>>> results, >>>>>> --- >>>>>> package/pkg-generic.mk | 5 +++++ >>>>>> 1 file changed, 5 insertions(+) >>>>>> >>>>>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk >>>>>> index 259ee02..46d92aa 100644 >>>>>> --- a/package/pkg-generic.mk >>>>>> +++ b/package/pkg-generic.mk >>>>>> @@ -162,6 +162,11 @@ $(BUILD_DIR)/%/.stamp_patched: >>>>>> $(Q)touch $@ >>>>>> @$(call step_end,patch) >>>>>> >>>>>> +# Check that all directories specified in BR2_GLOBAL_PATCH_DIR exist. >>>>>> +$(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\ >>>>>> + $(if $(wildcard $(dir)),,\ >>>>>> + $(error BR2_GLOBAL_PATCH_DIR contains nonexistent directory $(dir)))) >>>>>> + >>>> On a small note, since we already do a '[...] test -d $${D} [...]', >>>> why don't you simply add an else condition with the error message ? >>> >>> Because $(D) is in fact $(BR2_GLOBAL_PATCH_DIR)/$(RAWNAME), which is not >>> required to exist for every package. >> Ah yes indeed. Sorry. >> Other question, why don't put that bit of code at the beginning of the >> target stamp_patched ? Or at least in the target stamp_patched ? > > Because the same check would be performed many times for every package. > This way, things are checked only once, while the Makefile is read. Well. As you may have seen, we already execute the stamp-patched target for every package, we do some action, even call a script. So... I don't think it would matter much to add this check for every package. And IMHO, it would be better suited in that block of code. But I don't know what would be preferred in this case. > > -Michal ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH] Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit 2014-10-22 11:54 ` Maxime Hadjinlian 2014-10-22 14:01 ` Michal Sojka @ 2014-10-22 16:35 ` Arnout Vandecappelle 2014-10-23 17:41 ` Maxime Hadjinlian 1 sibling, 1 reply; 10+ messages in thread From: Arnout Vandecappelle @ 2014-10-22 16:35 UTC (permalink / raw) To: buildroot On 22/10/14 13:54, Maxime Hadjinlian wrote: > Other question, why don't put that bit of code at the beginning of the > target stamp_patched ? Or at least in the target stamp_patched ? Because we prefer to have checks for the validity of the .config immediately, before the build is started. Even a 'make savedefconfig' or 'make graph-depends' will trigger the error. For this particular one it is not so important, but for other checks it is really useful to have it before the build begins and not after half an hour of building. Note that this is currently not always the case, e.g. dependencies.sh, external toolchain validity. But if possible, I prefer .config checks to be done in make. 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: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH] Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit 2014-10-22 16:35 ` Arnout Vandecappelle @ 2014-10-23 17:41 ` Maxime Hadjinlian 0 siblings, 0 replies; 10+ messages in thread From: Maxime Hadjinlian @ 2014-10-23 17:41 UTC (permalink / raw) To: buildroot Hi Arnout, Michal, Yann, all, On Wed, Oct 22, 2014 at 6:35 PM, Arnout Vandecappelle <arnout@mind.be> wrote: > On 22/10/14 13:54, Maxime Hadjinlian wrote: >> Other question, why don't put that bit of code at the beginning of the >> target stamp_patched ? Or at least in the target stamp_patched ? > > Because we prefer to have checks for the validity of the .config immediately, > before the build is started. Even a 'make savedefconfig' or 'make graph-depends' > will trigger the error. For this particular one it is not so important, but for > other checks it is really useful to have it before the build begins and not > after half an hour of building. Understood. Thanks for clearing that up. > > Note that this is currently not always the case, e.g. dependencies.sh, external > toolchain validity. But if possible, I prefer .config checks to be done in make. > > > 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: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Buildroot] [PATCH] Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit 2014-10-21 8:42 [Buildroot] [PATCH] Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit Michal Sojka 2014-10-21 16:58 ` Yann E. MORIN @ 2014-10-24 23:58 ` Peter Korsgaard 1 sibling, 0 replies; 10+ messages in thread From: Peter Korsgaard @ 2014-10-24 23:58 UTC (permalink / raw) To: buildroot >>>>> "Michal" == Michal Sojka <sojka@merica.cz> writes: > If the BR2_GLOBAL_PATCH_DIR is misspelled in the .config or if somebody > renamed the existing directory with patches, buildroot happily builds > everything but without the patches. As this can lead to surprising > results, it is better to fail and give a user a message that > BR2_GLOBAL_PATCH_DIR is wrongly configured. > Signed-off-by: Michal Sojka <sojka@merica.cz> Committed, thanks. -- Bye, Peter Korsgaard ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-10-24 23:58 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-10-21 8:42 [Buildroot] [PATCH] Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit Michal Sojka 2014-10-21 16:58 ` Yann E. MORIN 2014-10-22 11:26 ` Maxime Hadjinlian 2014-10-22 11:43 ` Michal Sojka 2014-10-22 11:54 ` Maxime Hadjinlian 2014-10-22 14:01 ` Michal Sojka 2014-10-22 14:26 ` Maxime Hadjinlian 2014-10-22 16:35 ` Arnout Vandecappelle 2014-10-23 17:41 ` Maxime Hadjinlian 2014-10-24 23:58 ` Peter Korsgaard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox