| Message ID | 20181218005116.17435-5-mmayer@broadcom.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Allow customization of system default PATH | expand |
Markus, All, On 2018-12-17 16:51 -0800, Markus Mayer spake thusly: > We use the configuration option BR2_SYSTEM_DEFAULT_PATH to set the > default PATH in dropbear sessions. Ditto, string already quoted blah-blah-blah... ;-) > Signed-off-by: Markus Mayer <mmayer@broadcom.com> > --- > package/dropbear/dropbear.mk | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/package/dropbear/dropbear.mk b/package/dropbear/dropbear.mk > index 7b1468cfb100..7dc5f46bcc81 100644 > --- a/package/dropbear/dropbear.mk > +++ b/package/dropbear/dropbear.mk > @@ -81,6 +81,14 @@ define DROPBEAR_DISABLE_STANDALONE > echo '#define NON_INETD_MODE 0' >> $(@D)/localoptions.h > endef > > +ifneq ($(BR2_SYSTEM_DEFAULT_PATH),"") > +define DROPBEAR_CUSTOM_PATH > + echo '#define DEFAULT_PATH $(BR2_SYSTEM_DEFAULT_PATH)' >>$(@D)/localoptions.h > +endef Why is it conditional here, and not in the two other patches, then? In fact I think I missed that in the initial patch: we should ensure that it can't be empty (unelss there is a good reason to accept a system-wide empty PATH). Maybe something in system/system.mk: ifeq ($(BR_BUILDLING)$(BR2_SYSTEM_DEFAULT_PATH)),y"") $(error BR2_SYSTEM_DEFAULT_PATH can't be empty) endif (no need to properly finish the sentence, the $(error) will do it for you.) And then we can safely use it as-is, unconditionally, in the three other locations (profile, openssh, dropbear). Of course, if we decide that an empty BR2_SYSTEM_DEFAULT_PATH is valid, then we should also use it as-is in dropbear and openssh, because we should just abide by the user's decision. Regards, Yann E. MORIN. > +DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_CUSTOM_PATH > +endif > + > define DROPBEAR_INSTALL_INIT_SYSTEMD > $(INSTALL) -D -m 644 package/dropbear/dropbear.service \ > $(TARGET_DIR)/usr/lib/systemd/system/dropbear.service > -- > 2.17.1 >
On Tue, 18 Dec 2018 at 14:08, Markus Mayer <markus.mayer@broadcom.com> wrote: > > On Tue, 18 Dec 2018 at 11:47, Yann E. MORIN <yann.morin.1998@free.fr> wrote: > > > > > In fact I think I missed that in the initial patch: we should ensure > > that it can't be empty (unelss there is a good reason to accept a > > system-wide empty PATH). Maybe something in system/system.mk: > > > > ifeq ($(BR_BUILDLING)$(BR2_SYSTEM_DEFAULT_PATH)),y"") > > $(error BR2_SYSTEM_DEFAULT_PATH can't be empty) > > endif > > Makes sense to me. I can't think of a good use-case for an empty > default PATH. I'll add this for now. If it is decided that an empty > path does make sense, I can remove it again from the series. Hm. I can't seem to get this to work. ifeq ($(BR_BUILDING)$(BR2_SYSTEM_DEFAULT_PATH)),y"") #$(error BR2_SYSTEM_DEFAULT_PATH can't be empty) $(error '$(BR_BUILDING)$(BR2_SYSTEM_DEFAULT_PATH)') endif For debug purposes, I replaced the error message with a message that prints the value of the two variables with single quotes around, so potential spaces and such become visible. It never enters the if. If I change it to ifneq, it enters the "if", but prints the exact values that it supposedly does not match. How can it have the value y"" and still enter ifneq? system/system.mk:101: *** 'y""'. Stop. Makefile:25: recipe for target '_all' failed If I keep the ifneq and set the default path so some value: system/system.mk:101: *** 'y"abc"'. Stop. Makefile:25: recipe for target '_all' failed It still matches. This time, it makes sense. Any thoughs why case of y"" doesn't match as I would have anticipated? Thanks, -Markus > > (no need to properly finish the sentence, the $(error) will do it for > > you.) > > > > And then we can safely use it as-is, unconditionally, in the three other > > locations (profile, openssh, dropbear). > > > > Of course, if we decide that an empty BR2_SYSTEM_DEFAULT_PATH is valid, > > then we should also use it as-is in dropbear and openssh, because we > > should just abide by the user's decision. > > > > Regards, > > Yann E. MORIN.
diff --git a/package/dropbear/dropbear.mk b/package/dropbear/dropbear.mk index 7b1468cfb100..7dc5f46bcc81 100644 --- a/package/dropbear/dropbear.mk +++ b/package/dropbear/dropbear.mk @@ -81,6 +81,14 @@ define DROPBEAR_DISABLE_STANDALONE echo '#define NON_INETD_MODE 0' >> $(@D)/localoptions.h endef +ifneq ($(BR2_SYSTEM_DEFAULT_PATH),"") +define DROPBEAR_CUSTOM_PATH + echo '#define DEFAULT_PATH $(BR2_SYSTEM_DEFAULT_PATH)' >>$(@D)/localoptions.h +endef + +DROPBEAR_POST_EXTRACT_HOOKS += DROPBEAR_CUSTOM_PATH +endif + define DROPBEAR_INSTALL_INIT_SYSTEMD $(INSTALL) -D -m 644 package/dropbear/dropbear.service \ $(TARGET_DIR)/usr/lib/systemd/system/dropbear.service
We use the configuration option BR2_SYSTEM_DEFAULT_PATH to set the default PATH in dropbear sessions. Signed-off-by: Markus Mayer <mmayer@broadcom.com> --- package/dropbear/dropbear.mk | 8 ++++++++ 1 file changed, 8 insertions(+)