| Message ID | cdb72fa7204d35b88f2b74e79d4d1117b797712d.1433591404.git.yann.morin.1998@free.fr |
|---|---|
| State | Superseded |
| Headers | show |
Dear Yann E. MORIN, On Sat, 6 Jun 2015 13:54:24 +0200, Yann E. MORIN wrote: > # In order to get a usable, consistent configuration, some fixup may be needed. > # The exact rules are specified by the package .mk file. > -$$($(2)_DIR)/.stamp_kconfig_fixup_done: $$($(2)_DIR)/.config > +define $(1)_FIXUP_KCONFIG > $$($(2)_KCONFIG_FIXUP_CMDS) > @yes "" | $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \ We could use $(@D) instead of $$($(2)_DIR) here. > $$($(2)_KCONFIG_OPTS) oldconfig > - $$(Q)touch $$@ > + $$(Q)touch $$($(2)_DIR)/.stamp_kconfig_fixup_done and here. But that's indeed not very important, since they are the same. Thomas
Thomas, All, On 2015-06-12 23:33 +0200, Thomas Petazzoni spake thusly: > On Sat, 6 Jun 2015 13:54:24 +0200, Yann E. MORIN wrote: > > # In order to get a usable, consistent configuration, some fixup may be needed. > > # The exact rules are specified by the package .mk file. > > -$$($(2)_DIR)/.stamp_kconfig_fixup_done: $$($(2)_DIR)/.config > > +define $(1)_FIXUP_KCONFIG > > $$($(2)_KCONFIG_FIXUP_CMDS) > > @yes "" | $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \ > > We could use $(@D) instead of $$($(2)_DIR) here. Yes, and Arnout suggested the same in another series I sent. My reasoning for using $($(2)_DIR) is that we are in the infra, and we do have this variable, while $$(@D) only works when there is an actual file being generated, which is not always the case. See for example the followup patch, for which we have to fake @D because of those package-provided macros that may need it. So, I prefer using $($(2)_DIR) because I /know/ what it is, while $(@D) I don;t know beforehand if it would be set. > > $$($(2)_KCONFIG_OPTS) oldconfig > > - $$(Q)touch $$@ > > + $$(Q)touch $$($(2)_DIR)/.stamp_kconfig_fixup_done > > and here. But that's indeed not very important, since they are the same. Yes, it works with this patch because there is an actual file (and in the correct location) as the target of the current rule. It wiull also work with the next patch, because we are faking @D . So, I'd prefer we stick with $($(2)_DIR) here, to avoid any issue in the future. Regards, Yann E. MORIN.
On 06/13/15 00:17, Yann E. MORIN wrote: > Thomas, All, > > On 2015-06-12 23:33 +0200, Thomas Petazzoni spake thusly: >> On Sat, 6 Jun 2015 13:54:24 +0200, Yann E. MORIN wrote: >>> # In order to get a usable, consistent configuration, some fixup may be needed. >>> # The exact rules are specified by the package .mk file. >>> -$$($(2)_DIR)/.stamp_kconfig_fixup_done: $$($(2)_DIR)/.config >>> +define $(1)_FIXUP_KCONFIG >>> $$($(2)_KCONFIG_FIXUP_CMDS) >>> @yes "" | $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \ >> >> We could use $(@D) instead of $$($(2)_DIR) here. > > Yes, and Arnout suggested the same in another series I sent. > > My reasoning for using $($(2)_DIR) is that we are in the infra, and we > do have this variable, while $$(@D) only works when there is an actual > file being generated, which is not always the case. > > See for example the followup patch, for which we have to fake @D because > of those package-provided macros that may need it. > > So, I prefer using $($(2)_DIR) because I /know/ what it is, while $(@D) > I don;t know beforehand if it would be set. I was going to stick to my guns and say it has to be @D (cfr. the workaround I proposed on the following patch). However, what you say makes complete sense, so let's keep it at $(2)_DIR. It's also more readable anyway. Regards, Arnout [snip]
diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk index 1d69722..d6986c3 100644 --- a/package/pkg-kconfig.mk +++ b/package/pkg-kconfig.mk @@ -64,11 +64,15 @@ $$($(2)_DIR)/.config: $$($(2)_KCONFIG_FILE) $$($(2)_KCONFIG_FRAGMENT_FILES) # In order to get a usable, consistent configuration, some fixup may be needed. # The exact rules are specified by the package .mk file. -$$($(2)_DIR)/.stamp_kconfig_fixup_done: $$($(2)_DIR)/.config +define $(1)_FIXUP_KCONFIG $$($(2)_KCONFIG_FIXUP_CMDS) @yes "" | $$($(2)_MAKE_ENV) $$(MAKE) -C $$($(2)_DIR) \ $$($(2)_KCONFIG_OPTS) oldconfig - $$(Q)touch $$@ + $$(Q)touch $$($(2)_DIR)/.stamp_kconfig_fixup_done +endef + +$$($(2)_DIR)/.stamp_kconfig_fixup_done: $$($(2)_DIR)/.config + $$(call $(1)_FIXUP_KCONFIG) # Before running configure, the configuration file should be present and fixed $$($(2)_TARGET_CONFIGURE): $$($(2)_DIR)/.stamp_kconfig_fixup_done
The same fixups will have to be done after leaving the configurators, so we want to commonalise that code. Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- package/pkg-kconfig.mk | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)