diff mbox

[OpenWrt-Devel,3/4] ar71xx: fix giving extra arguments to Build/mktplinkfw

Message ID 0bb71f7724182cb86d3d67cf399776b811c6219b.1461220406.git.mschiffer@universe-factory.net
State Changes Requested
Headers show

Commit Message

Matthias Schiffer April 21, 2016, 6:33 a.m. UTC 
The build command will always get the whole argument string in $(1),
regardless of whitespace. We need to use word/wordlist to split the string
after the first word.

Whitespace and quotation will be given to the command verbatim (make will
ignore it), so to give multiple arguments, no quotation marks may be used.

Fixes: r47174 ("ar71xx/image: add options argument to mktplinkfw step")
Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
---
 target/linux/ar71xx/image/Makefile | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

Comments

Rafał Miłecki April 21, 2016, 9:30 a.m. UTC | #1 
On 21 April 2016 at 08:33, Matthias Schiffer
<mschiffer@universe-factory.net> wrote:
> @@ -967,8 +968,8 @@ define Device/tl-wr2543-v1
>      BOARDNAME := TL-WR2543N
>      DEVICE_PROFILE := TLWR2543
>      TPLINK_HWID := 0x25430001
> -    IMAGE/sysupgrade.bin := append-rootfs | mktplinkfw sysupgrade "-v 3.13.99"
> -    IMAGE/factory.bin := append-rootfs | mktplinkfw factory "-v 3.13.99"
> +    IMAGE/sysupgrade.bin := append-rootfs | mktplinkfw sysupgrade -v 3.13.99
> +    IMAGE/factory.bin := append-rootfs | mktplinkfw factory -v 3.13.99
>  endef
>  TARGET_DEVICES += tl-wr2543-v1

Any idea why we don't use variables in the first place? E.g.
VERSION := 3.13.99
Matthias Schiffer April 21, 2016, 9:40 a.m. UTC | #2 
On 04/21/2016 11:30 AM, Rafał Miłecki wrote:
> On 21 April 2016 at 08:33, Matthias Schiffer
> <mschiffer@universe-factory.net> wrote:
>> @@ -967,8 +968,8 @@ define Device/tl-wr2543-v1
>>      BOARDNAME := TL-WR2543N
>>      DEVICE_PROFILE := TLWR2543
>>      TPLINK_HWID := 0x25430001
>> -    IMAGE/sysupgrade.bin := append-rootfs | mktplinkfw sysupgrade "-v 3.13.99"
>> -    IMAGE/factory.bin := append-rootfs | mktplinkfw factory "-v 3.13.99"
>> +    IMAGE/sysupgrade.bin := append-rootfs | mktplinkfw sysupgrade -v 3.13.99
>> +    IMAGE/factory.bin := append-rootfs | mktplinkfw factory -v 3.13.99
>>  endef
>>  TARGET_DEVICES += tl-wr2543-v1
> 
> Any idea why we don't use variables in the first place? E.g.
> VERSION := 3.13.99
> 

For the version, this would make sense, but can be done in a separate
patch. For the region code, we will eventually need to build multiple
factory images with different arguments, so in that case, using additional
arguments for the mktplinkfw build step makes sense.

Matthias
diff mbox

Patch

diff --git a/target/linux/ar71xx/image/Makefile b/target/linux/ar71xx/image/Makefile
index e4e2fcf..2699b64 100644
--- a/target/linux/ar71xx/image/Makefile
+++ b/target/linux/ar71xx/image/Makefile
@@ -49,14 +49,15 @@  endef
 # -X reserve <size> bytes in the firmware image (hexval prefixed with 0x)
 define Build/mktplinkfw
 	-$(STAGING_DIR_HOST)/bin/mktplinkfw \
-		-H $(TPLINK_HWID) -W $(TPLINK_HWREV) -F $(TPLINK_FLASHLAYOUT) -N OpenWrt -V $(REVISION) $2 \
+		-H $(TPLINK_HWID) -W $(TPLINK_HWREV) -F $(TPLINK_FLASHLAYOUT) -N OpenWrt -V $(REVISION) \
 		-m $(TPLINK_HEADER_VERSION) \
 		-k $(word 1,$^) \
 		-r $@ \
 		-o $@.new \
 		-j -X 0x40000 \
 		-a $(call rootfs_align,$(FILESYSTEM)) \
-		$(if $(findstring sysupgrade,$1),-s) && mv $@.new $@ || rm -f $@
+		$(wordlist 2,$(words $(1)),$(1)) \
+		$(if $(findstring sysupgrade,$(word 1,$(1))),-s) && mv $@.new $@ || rm -f $@
 endef
 
 # mktplinkfw-initramfs <optional extra arguments to mktplinkfw binary>
@@ -64,7 +65,7 @@  endef
 # -c combined image
 define Build/mktplinkfw-initramfs
 	$(STAGING_DIR_HOST)/bin/mktplinkfw \
-		-H $(TPLINK_HWID) -W $(TPLINK_HWREV) -F $(TPLINK_FLASHLAYOUT) -N OpenWrt -V $(REVISION) $2 \
+		-H $(TPLINK_HWID) -W $(TPLINK_HWREV) -F $(TPLINK_FLASHLAYOUT) -N OpenWrt -V $(REVISION) $(1) \
 		-m $(TPLINK_HEADER_VERSION) \
 		-k $@ \
 		-o $@.new \
@@ -75,14 +76,14 @@  endef
 
 define Build/tplink-safeloader
        -$(STAGING_DIR_HOST)/bin/tplink-safeloader \
-               -B $(TPLINK_BOARD_NAME) \
-               -V $(REVISION) \
-               -k $(word 1,$^) \
-               -r $@ \
-               -o $@.new \
-               $2 \
-               -j \
-               $(if $(findstring sysupgrade,$1),-S) && mv $@.new $@ || rm -f $@
+		-B $(TPLINK_BOARD_NAME) \
+		-V $(REVISION) \
+		-k $(word 1,$^) \
+		-r $@ \
+		-o $@.new \
+		-j \
+		$(wordlist 2,$(words $(1)),$(1)) \
+		$(if $(findstring sysupgrade,$(word 1,$(1))),-S) && mv $@.new $@ || rm -f $@
 endef
 
 define Build/loader-common
@@ -967,8 +968,8 @@  define Device/tl-wr2543-v1
     BOARDNAME := TL-WR2543N
     DEVICE_PROFILE := TLWR2543
     TPLINK_HWID := 0x25430001
-    IMAGE/sysupgrade.bin := append-rootfs | mktplinkfw sysupgrade "-v 3.13.99"
-    IMAGE/factory.bin := append-rootfs | mktplinkfw factory "-v 3.13.99"
+    IMAGE/sysupgrade.bin := append-rootfs | mktplinkfw sysupgrade -v 3.13.99
+    IMAGE/factory.bin := append-rootfs | mktplinkfw factory -v 3.13.99
 endef
 TARGET_DEVICES += tl-wr2543-v1