Message ID | 20161116195543.6456-2-gael.portay@savoirfairelinux.com |
---|---|
State | Changes Requested |
Headers | show |
On 16-11-16 20:55, Gaël PORTAY wrote: > Since the commit 6dd7bbb59134799ed3d7343f238b3b02592faebf, the label does > not need anymore to be quoted. Even worse it *must* not be simple-quoted, > unless the label will contain the double-quotes from the config variable > BR2_TARGET_ROOTFS_EXT2_LABEL. > > Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com> > --- > fs/ext2/ext2.mk | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk > index 7417f81..49b44e4 100644 > --- a/fs/ext2/ext2.mk > +++ b/fs/ext2/ext2.mk > @@ -20,12 +20,8 @@ ifneq ($(strip $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)),0) > EXT2_OPTS += -r $(BR2_TARGET_ROOTFS_EXT2_RESBLKS) > endif > > -# Not qstrip-ing the variable, because it may contain spaces, but we must > -# qstrip it when checking. Furthermore, we need to further quote it, so > -# that the quotes do not get eaten by the echo statement when creating the > -# fakeroot script > ifneq ($(call qstrip,$(BR2_TARGET_ROOTFS_EXT2_LABEL)),) > -EXT2_OPTS += -l '$(BR2_TARGET_ROOTFS_EXT2_LABEL)' > +EXT2_OPTS += -l $(BR2_TARGET_ROOTFS_EXT2_LABEL) We usually don't rely on the quotes added by Kconfig, because it's possible that the variable is overridden on the command line like make BR2_TARGET_ROOTFS_EXT2_LABEL="foo bar" So it should be EXT2_OPTS += -l '$(call qstrip,$(BR2_TARGET_ROOTFS_EXT2_LABEL))' Regards, Arnout > endif > > ROOTFS_EXT2_DEPENDENCIES = host-mke2img >
Hi Arnout, I am sorry, I saw your email shortly afterwards sending the v2... On Wed, Nov 16, 2016 at 11:58:23PM +0100, Arnout Vandecappelle wrote: > > > On 16-11-16 20:55, Gaël PORTAY wrote: > > Since the commit 6dd7bbb59134799ed3d7343f238b3b02592faebf, the label does > > not need anymore to be quoted. Even worse it *must* not be simple-quoted, > > unless the label will contain the double-quotes from the config variable > > BR2_TARGET_ROOTFS_EXT2_LABEL. > > > > Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com> > > --- > > fs/ext2/ext2.mk | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk > > index 7417f81..49b44e4 100644 > > --- a/fs/ext2/ext2.mk > > +++ b/fs/ext2/ext2.mk > > @@ -20,12 +20,8 @@ ifneq ($(strip $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)),0) > > EXT2_OPTS += -r $(BR2_TARGET_ROOTFS_EXT2_RESBLKS) > > endif > > > > -# Not qstrip-ing the variable, because it may contain spaces, but we must > > -# qstrip it when checking. Furthermore, we need to further quote it, so > > -# that the quotes do not get eaten by the echo statement when creating the > > -# fakeroot script > > ifneq ($(call qstrip,$(BR2_TARGET_ROOTFS_EXT2_LABEL)),) > > -EXT2_OPTS += -l '$(BR2_TARGET_ROOTFS_EXT2_LABEL)' > > +EXT2_OPTS += -l $(BR2_TARGET_ROOTFS_EXT2_LABEL) > > We usually don't rely on the quotes added by Kconfig, because it's possible > that the variable is overridden on the command line like > make BR2_TARGET_ROOTFS_EXT2_LABEL="foo bar" > For sure you are right, it was not not appropriate to use the Kconfig variable. > So it should be > EXT2_OPTS += -l '$(call qstrip,$(BR2_TARGET_ROOTFS_EXT2_LABEL))' > As discuss with Thomas and Yann on the channel, we moved to this solution: EXT2_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_EXT2_LABEL)) ifneq ($(EXT2_LABEL),) EXT2_OPTS += -l "$(EXT2_LABEL)" endif ... to not rely on the quotes of Kconfig (as you said) and to preserved the spaces from the Kconfig variable ("BR 2016-11-rc2"). > Regards, > Arnout > > > endif > > > > ROOTFS_EXT2_DEPENDENCIES = host-mke2img > > > > -- > 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 Regards, Gaël
diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk index 7417f81..49b44e4 100644 --- a/fs/ext2/ext2.mk +++ b/fs/ext2/ext2.mk @@ -20,12 +20,8 @@ ifneq ($(strip $(BR2_TARGET_ROOTFS_EXT2_RESBLKS)),0) EXT2_OPTS += -r $(BR2_TARGET_ROOTFS_EXT2_RESBLKS) endif -# Not qstrip-ing the variable, because it may contain spaces, but we must -# qstrip it when checking. Furthermore, we need to further quote it, so -# that the quotes do not get eaten by the echo statement when creating the -# fakeroot script ifneq ($(call qstrip,$(BR2_TARGET_ROOTFS_EXT2_LABEL)),) -EXT2_OPTS += -l '$(BR2_TARGET_ROOTFS_EXT2_LABEL)' +EXT2_OPTS += -l $(BR2_TARGET_ROOTFS_EXT2_LABEL) endif ROOTFS_EXT2_DEPENDENCIES = host-mke2img
Since the commit 6dd7bbb59134799ed3d7343f238b3b02592faebf, the label does not need anymore to be quoted. Even worse it *must* not be simple-quoted, unless the label will contain the double-quotes from the config variable BR2_TARGET_ROOTFS_EXT2_LABEL. Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com> --- fs/ext2/ext2.mk | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)