Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [RFC/PATCH] fs/common.mk: Fix wrong double dollar usage
@ 2013-03-09 21:04 Ezequiel Garcia
  2013-03-12  8:30 ` Ezequiel Garcia
  2013-03-12  9:05 ` Thomas De Schampheleire
  0 siblings, 2 replies; 8+ messages in thread
From: Ezequiel Garcia @ 2013-03-09 21:04 UTC (permalink / raw)
  To: buildroot

Double dollar sign is used to put an explicit dollar sign,
for instance, when writing a makefile rule.

In this case, there are some makefile conditionals where
makefile variables are evaluated using double dollar signs
instead of single dollar, which is wrong.

In particular, this fixes a buildroot 'make' stall
when building with an empty device table (empty BR2_ROOTFS_DEVICE_TABLE).

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
I'm sending this as RFC because I'm new to buildroot
and because I'm not a makefile wizard.
AFAIK, this fixes a real bug in my compilation,
as explained in the commit message.

 fs/common.mk |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/common.mk b/fs/common.mk
index 8b5b2f2..700dc00 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -47,9 +47,9 @@ $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES)
 	rm -f $$(FAKEROOT_SCRIPT)
 	rm -f $$(TARGET_DIR_WARNING_FILE)
 	echo "chown -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
-ifneq ($$(ROOTFS_DEVICE_TABLES),)
+ifneq ($(ROOTFS_DEVICE_TABLES),)
 	cat $$(ROOTFS_DEVICE_TABLES) > $$(FULL_DEVICE_TABLE)
-ifeq ($$(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
+ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
 	printf '$$(subst $$(sep),\n,$$(PACKAGES_DEVICES_TABLE))' >> $$(FULL_DEVICE_TABLE)
 endif
 	printf '$$(subst $$(sep),\n,$$(PACKAGES_PERMISSIONS_TABLE))' >> $$(FULL_DEVICE_TABLE)
@@ -61,13 +61,13 @@ endif
 	cp support/misc/target-dir-warning.txt $$(TARGET_DIR_WARNING_FILE)
 	- at rm -f $$(FAKEROOT_SCRIPT) $$(FULL_DEVICE_TABLE)
 	$$(foreach hook,$$(ROOTFS_$(2)_POST_GEN_HOOKS),$$(call $$(hook))$$(sep))
-ifeq ($$(BR2_TARGET_ROOTFS_$(2)_GZIP),y)
+ifeq ($(BR2_TARGET_ROOTFS_$(2)_GZIP),y)
 	gzip -9 -c $$@ > $$@.gz
 endif
-ifeq ($$(BR2_TARGET_ROOTFS_$(2)_BZIP2),y)
+ifeq ($(BR2_TARGET_ROOTFS_$(2)_BZIP2),y)
 	bzip2 -9 -c $$@ > $$@.bz2
 endif
-ifeq ($$(BR2_TARGET_ROOTFS_$(2)_LZMA),y)
+ifeq ($(BR2_TARGET_ROOTFS_$(2)_LZMA),y)
 	$$(LZMA) -9 -c $$@ > $$@.lzma
 endif
 
@@ -76,7 +76,7 @@ rootfs-$(1)-show-depends:
 
 rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) $$(ROOTFS_$(2)_POST_TARGETS)
 
-ifeq ($$(BR2_TARGET_ROOTFS_$(2)),y)
+ifeq ($(BR2_TARGET_ROOTFS_$(2)),y)
 TARGETS += rootfs-$(1)
 endif
 endef
-- 
1.7.8.6

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

* [Buildroot] [RFC/PATCH] fs/common.mk: Fix wrong double dollar usage
  2013-03-09 21:04 [Buildroot] [RFC/PATCH] fs/common.mk: Fix wrong double dollar usage Ezequiel Garcia
@ 2013-03-12  8:30 ` Ezequiel Garcia
  2013-03-12  9:05 ` Thomas De Schampheleire
  1 sibling, 0 replies; 8+ messages in thread
From: Ezequiel Garcia @ 2013-03-12  8:30 UTC (permalink / raw)
  To: buildroot

On Sat, Mar 09, 2013 at 06:04:12PM -0300, Ezequiel Garcia wrote:
> Double dollar sign is used to put an explicit dollar sign,
> for instance, when writing a makefile rule.
> 
> In this case, there are some makefile conditionals where
> makefile variables are evaluated using double dollar signs
> instead of single dollar, which is wrong.
> 
> In particular, this fixes a buildroot 'make' stall
> when building with an empty device table (empty BR2_ROOTFS_DEVICE_TABLE).
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> I'm sending this as RFC because I'm new to buildroot
> and because I'm not a makefile wizard.
> AFAIK, this fixes a real bug in my compilation,
> as explained in the commit message.
> 
>  fs/common.mk |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 

Any comments? How does this look like?

-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

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

* [Buildroot] [RFC/PATCH] fs/common.mk: Fix wrong double dollar usage
  2013-03-09 21:04 [Buildroot] [RFC/PATCH] fs/common.mk: Fix wrong double dollar usage Ezequiel Garcia
  2013-03-12  8:30 ` Ezequiel Garcia
@ 2013-03-12  9:05 ` Thomas De Schampheleire
  2013-03-12 10:06   ` Ezequiel Garcia
  1 sibling, 1 reply; 8+ messages in thread
From: Thomas De Schampheleire @ 2013-03-12  9:05 UTC (permalink / raw)
  To: buildroot

Hi,

On Sat, Mar 9, 2013 at 10:04 PM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> Double dollar sign is used to put an explicit dollar sign,
> for instance, when writing a makefile rule.
>
> In this case, there are some makefile conditionals where
> makefile variables are evaluated using double dollar signs
> instead of single dollar, which is wrong.
>
> In particular, this fixes a buildroot 'make' stall
> when building with an empty device table (empty BR2_ROOTFS_DEVICE_TABLE).
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> I'm sending this as RFC because I'm new to buildroot
> and because I'm not a makefile wizard.
> AFAIK, this fixes a real bug in my compilation,
> as explained in the commit message.

In fact you are reverting changes made by Arnout a while back, see git
commit 847895d29524d81b64afb059b8649a77802a469b
http://git.buildroot.org/buildroot/commit/?id=847895d29524d81b64afb059b8649a77802a469b
There were specific reasons to make these changes, so I don't think we
should revert them like this.

Be aware that these make targets are inside a 'define
ROOTFS_TARGET_INTERNAL' statement, and later in the file this
'function' is executed with 'call'. This makes the rules for $ or $$ a
little different than in standard make recipes.

You mention a 'stall', can you elaborate more? What happens exactly?
Can you debug this further?

Best regards,
Thomas

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

* [Buildroot] [RFC/PATCH] fs/common.mk: Fix wrong double dollar usage
  2013-03-12  9:05 ` Thomas De Schampheleire
@ 2013-03-12 10:06   ` Ezequiel Garcia
  2013-03-12 11:28     ` Thomas De Schampheleire
  0 siblings, 1 reply; 8+ messages in thread
From: Ezequiel Garcia @ 2013-03-12 10:06 UTC (permalink / raw)
  To: buildroot

Hi Thomas,

Thanks for your answer.

On Tue, Mar 12, 2013 at 10:05:04AM +0100, Thomas De Schampheleire wrote:
> Hi,
> 
> On Sat, Mar 9, 2013 at 10:04 PM, Ezequiel Garcia
> <ezequiel.garcia@free-electrons.com> wrote:
> > Double dollar sign is used to put an explicit dollar sign,
> > for instance, when writing a makefile rule.
> >
> > In this case, there are some makefile conditionals where
> > makefile variables are evaluated using double dollar signs
> > instead of single dollar, which is wrong.
> >
> > In particular, this fixes a buildroot 'make' stall
> > when building with an empty device table (empty BR2_ROOTFS_DEVICE_TABLE).
> >
> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> > ---
> > I'm sending this as RFC because I'm new to buildroot
> > and because I'm not a makefile wizard.
> > AFAIK, this fixes a real bug in my compilation,
> > as explained in the commit message.
> 
> In fact you are reverting changes made by Arnout a while back, see git
> commit 847895d29524d81b64afb059b8649a77802a469b
> http://git.buildroot.org/buildroot/commit/?id=847895d29524d81b64afb059b8649a77802a469b
> There were specific reasons to make these changes, so I don't think we
> should revert them like this.
> 
> Be aware that these make targets are inside a 'define
> ROOTFS_TARGET_INTERNAL' statement, and later in the file this
> 'function' is executed with 'call'. This makes the rules for $ or $$ a
> little different than in standard make recipes.

Mmmm, I see. So it's not as easy as it seemed!
However, please note I'm *only* fixing the conditionals,
not the targets or the rules. See below.

> 
> You mention a 'stall', can you elaborate more? What happens exactly?
> Can you debug this further?
> 

Yes, I have. The problem is that this conditional

  ifneq ($$(ROOTFS_DEVICE_TABLES),)
         cat $$(ROOTFS_DEVICE_TABLES) > $$(FULL_DEVICE_TABLE)

is *never* false. However, if ROOTFS_DEVICE_TABLES is empty
the command will execute like this:

  $ cat > some_file

which simply stalls (well it doesn't stall, it's waiting for EOF at stdin).
I think this is easily reproducible, just set an empty value in your
BR2_ROOTFS_DEVICE_TABLE option.

As I said in the patch I'm far from a makefile wizard, but it seems
to me that $$(ROOTFS_DEVICE_TABLES) (double dollar) is not equivalent
to $(ROOTFS_DEVICE_TABLES) (single dollar) from the perspective of the
conditional.

Thanks,
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

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

* [Buildroot] [RFC/PATCH] fs/common.mk: Fix wrong double dollar usage
  2013-03-12 10:06   ` Ezequiel Garcia
@ 2013-03-12 11:28     ` Thomas De Schampheleire
  2013-03-12 12:44       ` Ezequiel Garcia
  2013-03-12 22:22       ` Arnout Vandecappelle
  0 siblings, 2 replies; 8+ messages in thread
From: Thomas De Schampheleire @ 2013-03-12 11:28 UTC (permalink / raw)
  To: buildroot

Hi Ezequiel,

On Tue, Mar 12, 2013 at 11:06 AM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> Hi Thomas,
>
> Thanks for your answer.
>
> On Tue, Mar 12, 2013 at 10:05:04AM +0100, Thomas De Schampheleire wrote:
>> Hi,
>>
>> On Sat, Mar 9, 2013 at 10:04 PM, Ezequiel Garcia
>> <ezequiel.garcia@free-electrons.com> wrote:
>> > Double dollar sign is used to put an explicit dollar sign,
>> > for instance, when writing a makefile rule.
>> >
>> > In this case, there are some makefile conditionals where
>> > makefile variables are evaluated using double dollar signs
>> > instead of single dollar, which is wrong.
>> >
>> > In particular, this fixes a buildroot 'make' stall
>> > when building with an empty device table (empty BR2_ROOTFS_DEVICE_TABLE).
>> >
>> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>> > ---
>> > I'm sending this as RFC because I'm new to buildroot
>> > and because I'm not a makefile wizard.
>> > AFAIK, this fixes a real bug in my compilation,
>> > as explained in the commit message.
>>
>> In fact you are reverting changes made by Arnout a while back, see git
>> commit 847895d29524d81b64afb059b8649a77802a469b
>> http://git.buildroot.org/buildroot/commit/?id=847895d29524d81b64afb059b8649a77802a469b
>> There were specific reasons to make these changes, so I don't think we
>> should revert them like this.
>>
>> Be aware that these make targets are inside a 'define
>> ROOTFS_TARGET_INTERNAL' statement, and later in the file this
>> 'function' is executed with 'call'. This makes the rules for $ or $$ a
>> little different than in standard make recipes.
>
> Mmmm, I see. So it's not as easy as it seemed!
> However, please note I'm *only* fixing the conditionals,
> not the targets or the rules. See below.
>
>>
>> You mention a 'stall', can you elaborate more? What happens exactly?
>> Can you debug this further?
>>
>
> Yes, I have. The problem is that this conditional
>
>   ifneq ($$(ROOTFS_DEVICE_TABLES),)
>          cat $$(ROOTFS_DEVICE_TABLES) > $$(FULL_DEVICE_TABLE)
>
> is *never* false. However, if ROOTFS_DEVICE_TABLES is empty
> the command will execute like this:
>
>   $ cat > some_file
>
> which simply stalls (well it doesn't stall, it's waiting for EOF at stdin).
> I think this is easily reproducible, just set an empty value in your
> BR2_ROOTFS_DEVICE_TABLE option.
>
> As I said in the patch I'm far from a makefile wizard, but it seems
> to me that $$(ROOTFS_DEVICE_TABLES) (double dollar) is not equivalent
> to $(ROOTFS_DEVICE_TABLES) (single dollar) from the perspective of the
> conditional.

I can reproduce the problem you mention.
The main problem is that the 'ifneq' never evaluates to false, as you
mentioned, even if its contents are 'empty'. They are never really
empty, because of this line:

ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) \
        $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE))

Because it is a concatenation of two strings separated by spaces,
there will always be a space in the final variable, which means it's
not empty. We need to strip it.
The following change fixes your problem, it runs the qstrip on the
overal combination of the variables, causing the space to be removed
if it's the only thing left.

diff --git a/fs/common.mk b/fs/common.mk
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -33,8 +33,8 @@

 FAKEROOT_SCRIPT = $(BUILD_DIR)/_fakeroot.fs
 FULL_DEVICE_TABLE = $(BUILD_DIR)/_device_table.txt
-ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) \
-       $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE))
+ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \
+       $(BR2_ROOTFS_STATIC_DEVICE_TABLE))

 define ROOTFS_TARGET_INTERNAL


Best regards,
Thomas

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

* [Buildroot] [RFC/PATCH] fs/common.mk: Fix wrong double dollar usage
  2013-03-12 11:28     ` Thomas De Schampheleire
@ 2013-03-12 12:44       ` Ezequiel Garcia
  2013-03-12 13:34         ` Thomas De Schampheleire
  2013-03-12 22:22       ` Arnout Vandecappelle
  1 sibling, 1 reply; 8+ messages in thread
From: Ezequiel Garcia @ 2013-03-12 12:44 UTC (permalink / raw)
  To: buildroot

On Tue, Mar 12, 2013 at 12:28:15PM +0100, Thomas De Schampheleire wrote:
> Hi Ezequiel,
> 
> On Tue, Mar 12, 2013 at 11:06 AM, Ezequiel Garcia
> <ezequiel.garcia@free-electrons.com> wrote:
> > Hi Thomas,
> >
> > Thanks for your answer.
> >
> > On Tue, Mar 12, 2013 at 10:05:04AM +0100, Thomas De Schampheleire wrote:
> >> Hi,
> >>
> >> On Sat, Mar 9, 2013 at 10:04 PM, Ezequiel Garcia
> >> <ezequiel.garcia@free-electrons.com> wrote:
> >> > Double dollar sign is used to put an explicit dollar sign,
> >> > for instance, when writing a makefile rule.
> >> >
> >> > In this case, there are some makefile conditionals where
> >> > makefile variables are evaluated using double dollar signs
> >> > instead of single dollar, which is wrong.
> >> >
> >> > In particular, this fixes a buildroot 'make' stall
> >> > when building with an empty device table (empty BR2_ROOTFS_DEVICE_TABLE).
> >> >
> >> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> >> > ---
> >> > I'm sending this as RFC because I'm new to buildroot
> >> > and because I'm not a makefile wizard.
> >> > AFAIK, this fixes a real bug in my compilation,
> >> > as explained in the commit message.
> >>
> >> In fact you are reverting changes made by Arnout a while back, see git
> >> commit 847895d29524d81b64afb059b8649a77802a469b
> >> http://git.buildroot.org/buildroot/commit/?id=847895d29524d81b64afb059b8649a77802a469b
> >> There were specific reasons to make these changes, so I don't think we
> >> should revert them like this.
> >>
> >> Be aware that these make targets are inside a 'define
> >> ROOTFS_TARGET_INTERNAL' statement, and later in the file this
> >> 'function' is executed with 'call'. This makes the rules for $ or $$ a
> >> little different than in standard make recipes.
> >
> > Mmmm, I see. So it's not as easy as it seemed!
> > However, please note I'm *only* fixing the conditionals,
> > not the targets or the rules. See below.
> >
> >>
> >> You mention a 'stall', can you elaborate more? What happens exactly?
> >> Can you debug this further?
> >>
> >
> > Yes, I have. The problem is that this conditional
> >
> >   ifneq ($$(ROOTFS_DEVICE_TABLES),)
> >          cat $$(ROOTFS_DEVICE_TABLES) > $$(FULL_DEVICE_TABLE)
> >
> > is *never* false. However, if ROOTFS_DEVICE_TABLES is empty
> > the command will execute like this:
> >
> >   $ cat > some_file
> >
> > which simply stalls (well it doesn't stall, it's waiting for EOF at stdin).
> > I think this is easily reproducible, just set an empty value in your
> > BR2_ROOTFS_DEVICE_TABLE option.
> >
> > As I said in the patch I'm far from a makefile wizard, but it seems
> > to me that $$(ROOTFS_DEVICE_TABLES) (double dollar) is not equivalent
> > to $(ROOTFS_DEVICE_TABLES) (single dollar) from the perspective of the
> > conditional.
> 
> I can reproduce the problem you mention.
> The main problem is that the 'ifneq' never evaluates to false, as you
> mentioned, even if its contents are 'empty'. They are never really
> empty, because of this line:
> 
> ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) \
>         $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE))
> 
> Because it is a concatenation of two strings separated by spaces,
> there will always be a space in the final variable, which means it's
> not empty. We need to strip it.

You're right! (I actually verified this before sending the patch,
but I was doing something wrong because I missed the extra space).

> The following change fixes your problem, it runs the qstrip on the
> overal combination of the variables, causing the space to be removed
> if it's the only thing left.
> 
> diff --git a/fs/common.mk b/fs/common.mk
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -33,8 +33,8 @@
> 
>  FAKEROOT_SCRIPT = $(BUILD_DIR)/_fakeroot.fs
>  FULL_DEVICE_TABLE = $(BUILD_DIR)/_device_table.txt
> -ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) \
> -       $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE))
> +ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \
> +       $(BR2_ROOTFS_STATIC_DEVICE_TABLE))
> 
>  define ROOTFS_TARGET_INTERNAL
> 
> 

Indeed, this patch fixes my problem. Do you want me to prepare a patch?

Otherwise, if you plan to submit the patch yourself, feel free to add:
Tested-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

Thanks,
-- 
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

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

* [Buildroot] [RFC/PATCH] fs/common.mk: Fix wrong double dollar usage
  2013-03-12 12:44       ` Ezequiel Garcia
@ 2013-03-12 13:34         ` Thomas De Schampheleire
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas De Schampheleire @ 2013-03-12 13:34 UTC (permalink / raw)
  To: buildroot

Hi Ezequiel,

On Tue, Mar 12, 2013 at 1:44 PM, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> On Tue, Mar 12, 2013 at 12:28:15PM +0100, Thomas De Schampheleire wrote:
>> Hi Ezequiel,
>>
>> On Tue, Mar 12, 2013 at 11:06 AM, Ezequiel Garcia
>> <ezequiel.garcia@free-electrons.com> wrote:
>> > Hi Thomas,
>> >
>> > Thanks for your answer.
>> >
>> > On Tue, Mar 12, 2013 at 10:05:04AM +0100, Thomas De Schampheleire wrote:
>> >> Hi,
>> >>
>> >> On Sat, Mar 9, 2013 at 10:04 PM, Ezequiel Garcia
>> >> <ezequiel.garcia@free-electrons.com> wrote:
>> >> > Double dollar sign is used to put an explicit dollar sign,
>> >> > for instance, when writing a makefile rule.
>> >> >
>> >> > In this case, there are some makefile conditionals where
>> >> > makefile variables are evaluated using double dollar signs
>> >> > instead of single dollar, which is wrong.
>> >> >
>> >> > In particular, this fixes a buildroot 'make' stall
>> >> > when building with an empty device table (empty BR2_ROOTFS_DEVICE_TABLE).
>> >> >
>> >> > Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>> >> > ---
>> >> > I'm sending this as RFC because I'm new to buildroot
>> >> > and because I'm not a makefile wizard.
>> >> > AFAIK, this fixes a real bug in my compilation,
>> >> > as explained in the commit message.
>> >>
>> >> In fact you are reverting changes made by Arnout a while back, see git
>> >> commit 847895d29524d81b64afb059b8649a77802a469b
>> >> http://git.buildroot.org/buildroot/commit/?id=847895d29524d81b64afb059b8649a77802a469b
>> >> There were specific reasons to make these changes, so I don't think we
>> >> should revert them like this.
>> >>
>> >> Be aware that these make targets are inside a 'define
>> >> ROOTFS_TARGET_INTERNAL' statement, and later in the file this
>> >> 'function' is executed with 'call'. This makes the rules for $ or $$ a
>> >> little different than in standard make recipes.
>> >
>> > Mmmm, I see. So it's not as easy as it seemed!
>> > However, please note I'm *only* fixing the conditionals,
>> > not the targets or the rules. See below.
>> >
>> >>
>> >> You mention a 'stall', can you elaborate more? What happens exactly?
>> >> Can you debug this further?
>> >>
>> >
>> > Yes, I have. The problem is that this conditional
>> >
>> >   ifneq ($$(ROOTFS_DEVICE_TABLES),)
>> >          cat $$(ROOTFS_DEVICE_TABLES) > $$(FULL_DEVICE_TABLE)
>> >
>> > is *never* false. However, if ROOTFS_DEVICE_TABLES is empty
>> > the command will execute like this:
>> >
>> >   $ cat > some_file
>> >
>> > which simply stalls (well it doesn't stall, it's waiting for EOF at stdin).
>> > I think this is easily reproducible, just set an empty value in your
>> > BR2_ROOTFS_DEVICE_TABLE option.
>> >
>> > As I said in the patch I'm far from a makefile wizard, but it seems
>> > to me that $$(ROOTFS_DEVICE_TABLES) (double dollar) is not equivalent
>> > to $(ROOTFS_DEVICE_TABLES) (single dollar) from the perspective of the
>> > conditional.
>>
>> I can reproduce the problem you mention.
>> The main problem is that the 'ifneq' never evaluates to false, as you
>> mentioned, even if its contents are 'empty'. They are never really
>> empty, because of this line:
>>
>> ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) \
>>         $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE))
>>
>> Because it is a concatenation of two strings separated by spaces,
>> there will always be a space in the final variable, which means it's
>> not empty. We need to strip it.
>
> You're right! (I actually verified this before sending the patch,
> but I was doing something wrong because I missed the extra space).
>
>> The following change fixes your problem, it runs the qstrip on the
>> overal combination of the variables, causing the space to be removed
>> if it's the only thing left.
>>
>> diff --git a/fs/common.mk b/fs/common.mk
>> --- a/fs/common.mk
>> +++ b/fs/common.mk
>> @@ -33,8 +33,8 @@
>>
>>  FAKEROOT_SCRIPT = $(BUILD_DIR)/_fakeroot.fs
>>  FULL_DEVICE_TABLE = $(BUILD_DIR)/_device_table.txt
>> -ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) \
>> -       $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE))
>> +ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \
>> +       $(BR2_ROOTFS_STATIC_DEVICE_TABLE))
>>
>>  define ROOTFS_TARGET_INTERNAL
>>
>>
>
> Indeed, this patch fixes my problem. Do you want me to prepare a patch?
>
> Otherwise, if you plan to submit the patch yourself, feel free to add:
> Tested-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>

Feel free to send the patch yourself, I don't mind. It will be quicker :)

Best regards,
Thomas

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

* [Buildroot] [RFC/PATCH] fs/common.mk: Fix wrong double dollar usage
  2013-03-12 11:28     ` Thomas De Schampheleire
  2013-03-12 12:44       ` Ezequiel Garcia
@ 2013-03-12 22:22       ` Arnout Vandecappelle
  1 sibling, 0 replies; 8+ messages in thread
From: Arnout Vandecappelle @ 2013-03-12 22:22 UTC (permalink / raw)
  To: buildroot

On 03/12/13 12:28, Thomas De Schampheleire wrote:
> The main problem is that the 'ifneq' never evaluates to false, as you
> mentioned, even if its contents are 'empty'. They are never really
> empty, because of this line:
>
> ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) \
>          $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE))
>
> Because it is a concatenation of two strings separated by spaces,
> there will always be a space in the final variable, which means it's
> not empty. We need to strip it.
> The following change fixes your problem, it runs the qstrip on the
> overal combination of the variables, causing the space to be removed
> if it's the only thing left.

  Alternatively, you could remove the space before the backslash. But I 
think I prefer the overall qstrip.

  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] 8+ messages in thread

end of thread, other threads:[~2013-03-12 22:22 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-09 21:04 [Buildroot] [RFC/PATCH] fs/common.mk: Fix wrong double dollar usage Ezequiel Garcia
2013-03-12  8:30 ` Ezequiel Garcia
2013-03-12  9:05 ` Thomas De Schampheleire
2013-03-12 10:06   ` Ezequiel Garcia
2013-03-12 11:28     ` Thomas De Schampheleire
2013-03-12 12:44       ` Ezequiel Garcia
2013-03-12 13:34         ` Thomas De Schampheleire
2013-03-12 22:22       ` Arnout Vandecappelle

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