| Message ID | b68b4ce8c5017701344c6f32a993de7d3e26802b.1390744549.git.yann.morin.1998@free.fr |
|---|---|
| State | Changes Requested |
| Headers | show |
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: > From: "Yann E. MORIN" <yann.morin.1998@free.fr> > We so far have no mean to get the value from a Kconfig option from the > .config file of a package (eg. linux, busybox...). > Add a new function that returns the unmangled value of an option. > It expect two arguments: > - the Kconfig option name (complete, with leading CONFIG if necessary) > - the .config file to get it from > Note that, if the Kconfig option is a string, the returned value will > contain the leading and trailing double-quotes. > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > --- > package/pkg-utils.mk | 6 ++++++ > 1 file changed, 6 insertions(+) > diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk > index 851575c..2f70acc 100644 > --- a/package/pkg-utils.mk > +++ b/package/pkg-utils.mk > @@ -52,6 +52,12 @@ define KCONFIG_DISABLE_OPT > echo "# $(1) is not set" >> $(2) > endef > +# Note: we do not indent this, since we want to avoid any leading > +# space or tabs when calling this function > +define KCONFIG_GET_OPT > +$(shell sed -e "/\\<$(1)\\>=\\(.*\\)$$/!d; s//\\1/" $(2)) Should this perhaps use $(SED)? Sorry, I'm probably missing something, but I don't right away see why we don't just use: $(SED) -n 's/^$(1)=//p' $(2)
On 2014-01-26 21:02 +0100, Peter Korsgaard spake thusly: > >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: > > > From: "Yann E. MORIN" <yann.morin.1998@free.fr> > > We so far have no mean to get the value from a Kconfig option from the > > .config file of a package (eg. linux, busybox...). > > > Add a new function that returns the unmangled value of an option. > > It expect two arguments: > > - the Kconfig option name (complete, with leading CONFIG if necessary) > > - the .config file to get it from > > > Note that, if the Kconfig option is a string, the returned value will > > contain the leading and trailing double-quotes. > > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > --- > > package/pkg-utils.mk | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk > > index 851575c..2f70acc 100644 > > --- a/package/pkg-utils.mk > > +++ b/package/pkg-utils.mk > > @@ -52,6 +52,12 @@ define KCONFIG_DISABLE_OPT > > echo "# $(1) is not set" >> $(2) > > endef > > > +# Note: we do not indent this, since we want to avoid any leading > > +# space or tabs when calling this function > > +define KCONFIG_GET_OPT > > +$(shell sed -e "/\\<$(1)\\>=\\(.*\\)$$/!d; s//\\1/" $(2)) > > Should this perhaps use $(SED)? $(SED) is 'sed -i -e' And we don't want to replace in-place! BTW, that $(SED) is 'sed -i -e' is really bugging me everytime I need to use sed. I thionk we should introduce: SED := sed -e SEDI := sed -i -e We unfortunately have that many calls to $(SED) here and there... We'd need to audit and replace all of them. > Sorry, I'm probably missing something, but I don't right away see why we > don't just use: > > $(SED) -n 's/^$(1)=//p' $(2) Oh, I just copied and adapted the commands in the lines above, from KCONFIG_SET_OPT or KCONFIG_ENABLE_OPT. Regards, Yann E. MORIN.
Peter, All, On 2014-01-26 21:02 +0100, Peter Korsgaard spake thusly: > > From: "Yann E. MORIN" <yann.morin.1998@free.fr> > > We so far have no mean to get the value from a Kconfig option from the > > .config file of a package (eg. linux, busybox...). > > > Add a new function that returns the unmangled value of an option. > > It expect two arguments: > > - the Kconfig option name (complete, with leading CONFIG if necessary) > > - the .config file to get it from > > > Note that, if the Kconfig option is a string, the returned value will > > contain the leading and trailing double-quotes. > > > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > > --- > > package/pkg-utils.mk | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk > > index 851575c..2f70acc 100644 > > --- a/package/pkg-utils.mk > > +++ b/package/pkg-utils.mk > > @@ -52,6 +52,12 @@ define KCONFIG_DISABLE_OPT > > echo "# $(1) is not set" >> $(2) > > endef > > > +# Note: we do not indent this, since we want to avoid any leading > > +# space or tabs when calling this function > > +define KCONFIG_GET_OPT > > +$(shell sed -e "/\\<$(1)\\>=\\(.*\\)$$/!d; s//\\1/" $(2)) > > Should this perhaps use $(SED)? > > Sorry, I'm probably missing something, but I don't right away see why we > don't just use: > > $(SED) -n 's/^$(1)=//p' $(2) Oh, I also forgot: we use $(shell ...) because we want to call this from inside make conditionals, like I did in the followup patch to check if CONFIG_KERNEL_LZO was set or not. If we do not use $(shell ...), then the expansion of KCONFIG_GET_OPT is used, and we'd get something like: ifeq (sed 'blabla' config-file,y) ... endif which is not really of any help, is it? ;-) (Not usre if your question was about use of $(shell ...) or about the sed expression itself. I can certinly use your alternate sed expression if you prefer, of course.) Regards, Yann E. MORIN.
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: Hi, >> Should this perhaps use $(SED)? > $(SED) is 'sed -i -e' > And we don't want to replace in-place! > BTW, that $(SED) is 'sed -i -e' is really bugging me everytime I need > to use sed. > I thionk we should introduce: > SED := sed -e > SEDI := sed -i -e > We unfortunately have that many calls to $(SED) here and there... We'd > need to audit and replace all of them. Hmm, yes - Agreed (even though I don't think we really need a SEDI). I don't really know why we added -i to SED, but I agree that it isn't really nice (it dates back to the 2d523c23 mega commit). >> Sorry, I'm probably missing something, but I don't right away see why we >> don't just use: >> >> $(SED) -n 's/^$(1)=//p' $(2) > Oh, I just copied and adapted the commands in the lines above, from > KCONFIG_SET_OPT or KCONFIG_ENABLE_OPT. Ahh, ok - Then I prefer the simple variant then - I believe it behaves identical but is directly obvious (to me).
On 26/01/14 14:56, Yann E. MORIN wrote: > +# Note: we do not indent this, since we want to avoid any leading > +# space or tabs when calling this function > +define KCONFIG_GET_OPT > +$(shell sed -e "/\\<$(1)\\>=\\(.*\\)$$/!d; s//\\1/" $(2)) > +endef There's no need to define this with 'define', you can do with a normal assignment, like we do for the UPPERCASE macro. KCONFIG_GET_OPT = $(shell sed -e ...) Regards, Arnout
diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk index 851575c..2f70acc 100644 --- a/package/pkg-utils.mk +++ b/package/pkg-utils.mk @@ -52,6 +52,12 @@ define KCONFIG_DISABLE_OPT echo "# $(1) is not set" >> $(2) endef +# Note: we do not indent this, since we want to avoid any leading +# space or tabs when calling this function +define KCONFIG_GET_OPT +$(shell sed -e "/\\<$(1)\\>=\\(.*\\)$$/!d; s//\\1/" $(2)) +endef + # Helper functions to determine the name of a package and its # directory from its makefile directory, using the $(MAKEFILE_LIST) # variable provided by make. This is used by the *TARGETS macros to