Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit

PATCH New Michal Sojka 5 participants 10 messages Oct 21, 2014 – Oct 24, 2014

Share & tools

Michal Sojka

author

2014-10-21 08:42 UTC #0 lore raw

PATCH 1/1 Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit

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 <[email protected]>

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

package/pkg-generic.mk +5 -0

index 259ee02..46d92aa 100644
@@ -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)

Signature · 1 line

2.1.1

Yann E. MORIN

2014-10-21 16:58 UTC #1 lore raw

PATCH 1/1 Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit

Michal, All,

On 2014-10-21 10:42 +0200, Michal Sojka spake thusly:

Quoted 4 lines

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... :-/

Quoted 4 lines

it is better to fail and give a user a message that BR2_GLOBAL_PATCH_DIR is wrongly configured.
Signed-off-by Michal Sojka <[email protected]>

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

Regards,
Yann E. MORIN.

Quoted 27 lines

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

package/pkg-generic.mk +5 -0

index 259ee02..46d92aa 100644
@@ -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)

Signature · 6 lines

2.1.1

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

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.  |
'------------------------------^-------^------------------^--------------------'

Maxime Hadjinlian

2014-10-22 11:26 UTC #2 lore raw

PATCH 1/1 Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit

Hi Michal, Yann, all

Quoted 19 lines · Yann E. MORIN

Michal, All,
On 2014-10-21 10:42 +0200, Michal Sojka spake thusly:
Quoted 16 lines
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(+)
package/pkg-generic.mk +0 -0
index 259ee02..46d92aa 100644
@@ -162,6 +162,11 @@ $(BUILD_DIR)/%/.stamp_patched:
      $(Q)touch $@
      @$(call step_end,patch)
 

Quoted 5 lines

package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_patched: · line 169

Quoted 5 lines

package/pkg-generic.mk +5 -0

+# 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 ?

Quoted 5 lines

package/pkg-generic.mk +0 -1

 # Configure
 $(BUILD_DIR)/%/.stamp_configured:
      @$(call step_start,configure)
--

2.1.1

Michal Sojka

author

2014-10-22 11:43 UTC #3 lore raw

PATCH 1/1 Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit

Quoted 22 lines · Maxime Hadjinlian

Hi Michal, Yann, all
Quoted 19 lines · Yann E. MORIN
Michal, All,
On 2014-10-21 10:42 +0200, Michal Sojka spake thusly:
Quoted 16 lines
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(+)
package/pkg-generic.mk +0 -0
index 259ee02..46d92aa 100644
@@ -162,6 +162,11 @@ $(BUILD_DIR)/%/.stamp_patched:
      $(Q)touch $@
      @$(call step_end,patch)
 

Quoted 7 lines

Quoted 5 lines
package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_patched: · line 169
Quoted 5 lines
package/pkg-generic.mk +5 -0
+# 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

Maxime Hadjinlian

2014-10-22 11:54 UTC #4 lore raw

PATCH 1/1 Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit

Quoted 31 lines · Michal Sojka

Quoted 22 lines · Maxime Hadjinlian
Hi Michal, Yann, all
Quoted 19 lines · Yann E. MORIN
Michal, All,
On 2014-10-21 10:42 +0200, Michal Sojka spake thusly:
Quoted 16 lines
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(+)
package/pkg-generic.mk +0 -0
index 259ee02..46d92aa 100644
@@ -162,6 +162,11 @@ $(BUILD_DIR)/%/.stamp_patched:
      $(Q)touch $@
      @$(call step_end,patch)
 
Quoted 7 lines
Quoted 5 lines
package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_patched: · line 169
Quoted 5 lines
package/pkg-generic.mk +5 -0
+# 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 ?

Quoted 2 lines

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 ?

Quoted 3 lines

Best regards,
-Michal

Michal Sojka

author

2014-10-22 14:01 UTC #5 lore raw

PATCH 1/1 Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit

Quoted 32 lines · Maxime Hadjinlian

Quoted 31 lines · Michal Sojka
Quoted 29 lines · Maxime Hadjinlian
Hi Michal, Yann, all
Quoted 19 lines · Yann E. MORIN
Michal, All,
On 2014-10-21 10:42 +0200, Michal Sojka spake thusly:
Quoted 16 lines
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(+)
package/pkg-generic.mk +0 -0
index 259ee02..46d92aa 100644
@@ -162,6 +162,11 @@ $(BUILD_DIR)/%/.stamp_patched:
      $(Q)touch $@
      @$(call step_end,patch)
 
Quoted 5 lines
package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_patched: · line 169
Quoted 5 lines
package/pkg-generic.mk +5 -0
+# 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 ?

Quoted 5 lines

Quoted 2 lines
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

Maxime Hadjinlian

2014-10-22 14:26 UTC #6 lore raw

PATCH 1/1 Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit

Quoted 33 lines · Michal Sojka

Quoted 32 lines · Maxime Hadjinlian
Quoted 31 lines · Michal Sojka
Quoted 29 lines · Maxime Hadjinlian
Hi Michal, Yann, all
Quoted 19 lines · Yann E. MORIN
Michal, All,
On 2014-10-21 10:42 +0200, Michal Sojka spake thusly:
Quoted 16 lines
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(+)
package/pkg-generic.mk +0 -0
index 259ee02..46d92aa 100644
@@ -162,6 +162,11 @@ $(BUILD_DIR)/%/.stamp_patched:
      $(Q)touch $@
      @$(call step_end,patch)
 
Quoted 5 lines
package/pkg-generic.mk $(BUILD_DIR)/%/.stamp_patched: · line 169
Quoted 5 lines
package/pkg-generic.mk +5 -0
+# 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 ?

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
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.

Quoted 2 lines

-Michal

Arnout Vandecappelle

2014-10-22 16:35 UTC #7 lore raw

PATCH 1/1 Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit

In reply to Maxime Hadjinlian

Quoted 2 lines · Maxime Hadjinlian

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

Signature · 6 lines

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

Maxime Hadjinlian

2014-10-23 17:41 UTC #8 lore raw

PATCH 1/1 Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit

Hi Arnout, Michal, Yann, all,

Quoted 9 lines · Arnout Vandecappelle

Quoted 2 lines · Maxime Hadjinlian
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.

Quoted 16 lines

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

Peter Korsgaard

2014-10-24 23:58 UTC #9 lore raw

PATCH 1/1 Complain loudly if BR2_GLOBAL_PATCH_DIR does not exit

In reply to Michal Sojka

Quoted 9 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Michal" == Michal Sojka <[email protected]> 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 <[email protected]>

Committed, thanks.

Signature · 1 line

Bye, Peter Korsgaard