All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 0/4] Allow customization of system default PATH
@ 2018-12-18  0:51 Markus Mayer
  2018-12-18  0:51 ` [Buildroot] [PATCH 1/4] system cfg: introduce option BR2_SYSTEM_DEFAULT_PATH Markus Mayer
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Markus Mayer @ 2018-12-18  0:51 UTC (permalink / raw)
  To: buildroot

Following an earlier patch submission[1] and a subsequent discussion[2],
here is the resulting patch series to handle the default PATH on the system
in a more generic manner.

What do you think of this initial round of patches?

Thanks,
-Markus

[1] http://lists.busybox.net/pipermail/buildroot/2018-March/215466.html
[2] http://lists.busybox.net/pipermail/buildroot/2018-December/238879.html

Markus Mayer (4):
  system cfg: introduce option BR2_SYSTEM_DEFAULT_PATH
  skeleton: use BR2_SYSTEM_DEFAULT_PATH as default PATH
  openssh: use BR2_SYSTEM_DEFAULT_PATH as default PATH
  dropbear: use BR2_SYSTEM_DEFAULT_PATH as default PATH

 package/dropbear/dropbear.mk                         |  8 ++++++++
 package/openssh/openssh.mk                           |  1 +
 package/skeleton-init-common/skeleton-init-common.mk |  3 +++
 system/Config.in                                     | 10 ++++++++++
 system/skeleton/etc/profile                          |  1 +
 5 files changed, 23 insertions(+)

-- 
2.17.1

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

* [Buildroot] [PATCH 1/4] system cfg: introduce option BR2_SYSTEM_DEFAULT_PATH
  2018-12-18  0:51 [Buildroot] [PATCH 0/4] Allow customization of system default PATH Markus Mayer
@ 2018-12-18  0:51 ` Markus Mayer
  2018-12-18 19:27   ` Yann E. MORIN
  2018-12-18  0:51 ` [Buildroot] [PATCH 2/4] skeleton: use BR2_SYSTEM_DEFAULT_PATH as default PATH Markus Mayer
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Markus Mayer @ 2018-12-18  0:51 UTC (permalink / raw)
  To: buildroot

The configuration option BR2_SYSTEM_DEFAULT_PATH allows the user to
override the default path, which can be used by /etc/profile and some
system daemons.

It defaults to the value previously hard-coded in /etc/profile. This
default should be suitable for most users.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 system/Config.in | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/system/Config.in b/system/Config.in
index 0f77b9b6721a..93458c0011b7 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -386,6 +386,16 @@ comment "automatic network configuration via DHCP needs ifupdown or busybox or n
 
 endif # BR2_ROOTFS_SKELETON_DEFAULT
 
+config BR2_SYSTEM_DEFAULT_PATH
+	string "Set the system's default PATH"
+	default "/bin:/sbin:/usr/bin:/usr/sbin" if !BR2_ROOTFS_MERGED_USR
+	default "/bin:/sbin" if BR2_ROOTFS_MERGED_USR
+	help
+	  Sets the system's default PATH. It is being used in /etc/profile
+	  in the skeleton-init-common package and by some daemons.
+
+	  The default should work in most cases.
+
 config BR2_ENABLE_LOCALE_PURGE
 	bool "Purge unwanted locales"
 	default y
-- 
2.17.1

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

* [Buildroot] [PATCH 2/4] skeleton: use BR2_SYSTEM_DEFAULT_PATH as default PATH
  2018-12-18  0:51 [Buildroot] [PATCH 0/4] Allow customization of system default PATH Markus Mayer
  2018-12-18  0:51 ` [Buildroot] [PATCH 1/4] system cfg: introduce option BR2_SYSTEM_DEFAULT_PATH Markus Mayer
@ 2018-12-18  0:51 ` Markus Mayer
  2018-12-18 19:32   ` Yann E. MORIN
  2018-12-18 19:33   ` Yann E. MORIN
  2018-12-18  0:51 ` [Buildroot] [PATCH 3/4] openssh: " Markus Mayer
  2018-12-18  0:51 ` [Buildroot] [PATCH 4/4] dropbear: " Markus Mayer
  3 siblings, 2 replies; 12+ messages in thread
From: Markus Mayer @ 2018-12-18  0:51 UTC (permalink / raw)
  To: buildroot

We substitute the path specified in system/skeleton/etc/profile with
the path specified in the configuration varialbe
BR2_SYSTEM_DEFAULT_PATH.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 package/skeleton-init-common/skeleton-init-common.mk | 3 +++
 system/skeleton/etc/profile                          | 1 +
 2 files changed, 4 insertions(+)

diff --git a/package/skeleton-init-common/skeleton-init-common.mk b/package/skeleton-init-common/skeleton-init-common.mk
index e8a052205268..358336599389 100644
--- a/package/skeleton-init-common/skeleton-init-common.mk
+++ b/package/skeleton-init-common/skeleton-init-common.mk
@@ -22,6 +22,9 @@ define SKELETON_INIT_COMMON_INSTALL_TARGET_CMDS
 	$(call SYSTEM_LIB_SYMLINK,$(TARGET_DIR))
 	$(INSTALL) -m 0644 support/misc/target-dir-warning.txt \
 		$(TARGET_DIR_WARNING_FILE)
+	$(SED) '/.*the PATH below may be replaced.*/d' $(TARGET_DIR)/etc/profile
+	$(SED) 's|PATH=.*|PATH=$(BR2_SYSTEM_DEFAULT_PATH)|' \
+		$(TARGET_DIR)/etc/profile
 endef
 
 # We don't care much about what goes in staging, as long as it is
diff --git a/system/skeleton/etc/profile b/system/skeleton/etc/profile
index 1255d23ff40d..5ed47743d8f4 100644
--- a/system/skeleton/etc/profile
+++ b/system/skeleton/etc/profile
@@ -1,3 +1,4 @@
+# At install, the PATH below may be replaced with BR2_SYSTEM_DEFAULT_PATH.
 export PATH=/bin:/sbin:/usr/bin:/usr/sbin
 
 if [ "$PS1" ]; then
-- 
2.17.1

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

* [Buildroot] [PATCH 3/4] openssh: use BR2_SYSTEM_DEFAULT_PATH as default PATH
  2018-12-18  0:51 [Buildroot] [PATCH 0/4] Allow customization of system default PATH Markus Mayer
  2018-12-18  0:51 ` [Buildroot] [PATCH 1/4] system cfg: introduce option BR2_SYSTEM_DEFAULT_PATH Markus Mayer
  2018-12-18  0:51 ` [Buildroot] [PATCH 2/4] skeleton: use BR2_SYSTEM_DEFAULT_PATH as default PATH Markus Mayer
@ 2018-12-18  0:51 ` Markus Mayer
  2018-12-18 19:38   ` Yann E. MORIN
  2018-12-18  0:51 ` [Buildroot] [PATCH 4/4] dropbear: " Markus Mayer
  3 siblings, 1 reply; 12+ messages in thread
From: Markus Mayer @ 2018-12-18  0:51 UTC (permalink / raw)
  To: buildroot

We use the configuration option BR2_SYSTEM_DEFAULT_PATH to set the
default PATH in OpenSSH sessions.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 package/openssh/openssh.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/openssh/openssh.mk b/package/openssh/openssh.mk
index 07f3e0d663f6..4ee7727fb9e8 100644
--- a/package/openssh/openssh.mk
+++ b/package/openssh/openssh.mk
@@ -11,6 +11,7 @@ OPENSSH_LICENSE_FILES = LICENCE
 OPENSSH_CONF_ENV = LD="$(TARGET_CC)" LDFLAGS="$(TARGET_CFLAGS)"
 OPENSSH_CONF_OPTS = \
 	--sysconfdir=/etc/ssh \
+	--with-default-path=$(BR2_SYSTEM_DEFAULT_PATH) \
 	--disable-lastlog \
 	--disable-utmp \
 	--disable-utmpx \
-- 
2.17.1

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

* [Buildroot] [PATCH 4/4] dropbear: use BR2_SYSTEM_DEFAULT_PATH as default PATH
  2018-12-18  0:51 [Buildroot] [PATCH 0/4] Allow customization of system default PATH Markus Mayer
                   ` (2 preceding siblings ...)
  2018-12-18  0:51 ` [Buildroot] [PATCH 3/4] openssh: " Markus Mayer
@ 2018-12-18  0:51 ` Markus Mayer
  2018-12-18 19:47   ` Yann E. MORIN
  3 siblings, 1 reply; 12+ messages in thread
From: Markus Mayer @ 2018-12-18  0:51 UTC (permalink / raw)
  To: buildroot

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(+)

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
-- 
2.17.1

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

* [Buildroot] [PATCH 1/4] system cfg: introduce option BR2_SYSTEM_DEFAULT_PATH
  2018-12-18  0:51 ` [Buildroot] [PATCH 1/4] system cfg: introduce option BR2_SYSTEM_DEFAULT_PATH Markus Mayer
@ 2018-12-18 19:27   ` Yann E. MORIN
  2018-12-18 21:22     ` Markus Mayer
  0 siblings, 1 reply; 12+ messages in thread
From: Yann E. MORIN @ 2018-12-18 19:27 UTC (permalink / raw)
  To: buildroot

Markus, All,

On 2018-12-17 16:51 -0800, Markus Mayer spake thusly:
> The configuration option BR2_SYSTEM_DEFAULT_PATH allows the user to
> override the default path, which can be used by /etc/profile and some
> system daemons.
> 
> It defaults to the value previously hard-coded in /etc/profile. This
> default should be suitable for most users.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
>  system/Config.in | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/system/Config.in b/system/Config.in
> index 0f77b9b6721a..93458c0011b7 100644
> --- a/system/Config.in
> +++ b/system/Config.in
> @@ -386,6 +386,16 @@ comment "automatic network configuration via DHCP needs ifupdown or busybox or n
>  
>  endif # BR2_ROOTFS_SKELETON_DEFAULT
>  
> +config BR2_SYSTEM_DEFAULT_PATH
> +	string "Set the system's default PATH"
> +	default "/bin:/sbin:/usr/bin:/usr/sbin" if !BR2_ROOTFS_MERGED_USR
> +	default "/bin:/sbin" if BR2_ROOTFS_MERGED_USR

Do we really care to have a different value at all?

In the merged-/usr case: if the command exists, it is found in either
/bin or /sbin, and the lookup stops there. If the command does not
exist, it is looked for in the four locations, instead of only two.

This does not look like too much of an overhead, does it?

Anyway, if we really want different values, then:
  - we can simplify it a bit,
  - and use the realy locations rahter than the  symlinks:

    default "/usr/bin:/usr/sbin" if BR2_ROOTFS_MERGED_USR
    default "/bin:/sbin:/usr/bin:/usr/sbin"

(kconfig will stop on the fiurst default that is valid.)

But for my part, I think a single default as is currently used is
enough (i.e. /bin:/sbin:/usr/bin:/usr/sbin).

Regards,
Yann E. MORIN.

> +	help
> +	  Sets the system's default PATH. It is being used in /etc/profile
> +	  in the skeleton-init-common package and by some daemons.
> +
> +	  The default should work in most cases.
> +
>  config BR2_ENABLE_LOCALE_PURGE
>  	bool "Purge unwanted locales"
>  	default y
> -- 
> 2.17.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 2/4] skeleton: use BR2_SYSTEM_DEFAULT_PATH as default PATH
  2018-12-18  0:51 ` [Buildroot] [PATCH 2/4] skeleton: use BR2_SYSTEM_DEFAULT_PATH as default PATH Markus Mayer
@ 2018-12-18 19:32   ` Yann E. MORIN
  2018-12-18 19:33   ` Yann E. MORIN
  1 sibling, 0 replies; 12+ messages in thread
From: Yann E. MORIN @ 2018-12-18 19:32 UTC (permalink / raw)
  To: buildroot

On 2018-12-17 16:51 -0800, Markus Mayer spake thusly:
> We substitute the path specified in system/skeleton/etc/profile with
> the path specified in the configuration varialbe
> BR2_SYSTEM_DEFAULT_PATH.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
>  package/skeleton-init-common/skeleton-init-common.mk | 3 +++
>  system/skeleton/etc/profile                          | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/package/skeleton-init-common/skeleton-init-common.mk b/package/skeleton-init-common/skeleton-init-common.mk
> index e8a052205268..358336599389 100644
> --- a/package/skeleton-init-common/skeleton-init-common.mk
> +++ b/package/skeleton-init-common/skeleton-init-common.mk
> @@ -22,6 +22,9 @@ define SKELETON_INIT_COMMON_INSTALL_TARGET_CMDS
>  	$(call SYSTEM_LIB_SYMLINK,$(TARGET_DIR))
>  	$(INSTALL) -m 0644 support/misc/target-dir-warning.txt \
>  		$(TARGET_DIR_WARNING_FILE)
> +	$(SED) '/.*the PATH below may be replaced.*/d' $(TARGET_DIR)/etc/profile
> +	$(SED) 's|PATH=.*|PATH=$(BR2_SYSTEM_DEFAULT_PATH)|' \
> +		$(TARGET_DIR)/etc/profile
>  endef
>  
>  # We don't care much about what goes in staging, as long as it is
> diff --git a/system/skeleton/etc/profile b/system/skeleton/etc/profile
> index 1255d23ff40d..5ed47743d8f4 100644
> --- a/system/skeleton/etc/profile
> +++ b/system/skeleton/etc/profile
> @@ -1,3 +1,4 @@
> +# At install, the PATH below may be replaced with BR2_SYSTEM_DEFAULT_PATH.

No need for the boilerplate, just:

    export PATH=@PATH@

This is enough to understand the value is actually replaced at some
point in the build process. This is so very similar to what the
autotools do. And thus, the sed expression becomes:

    $(SED) 's, at PATH@,$(BR2_SYSTEM_DEFAULT_PATH),' \
        $(TARGET_DIR)/etc/profile

(yeah, I see you went for the code snippet I initially provided, but
you should learn that you should not always do take what I say to
the letter! ;-] )

Regards,
Yann E. MORIN.

>  export PATH=/bin:/sbin:/usr/bin:/usr/sbin
>  
>  if [ "$PS1" ]; then
> -- 
> 2.17.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 2/4] skeleton: use BR2_SYSTEM_DEFAULT_PATH as default PATH
  2018-12-18  0:51 ` [Buildroot] [PATCH 2/4] skeleton: use BR2_SYSTEM_DEFAULT_PATH as default PATH Markus Mayer
  2018-12-18 19:32   ` Yann E. MORIN
@ 2018-12-18 19:33   ` Yann E. MORIN
  1 sibling, 0 replies; 12+ messages in thread
From: Yann E. MORIN @ 2018-12-18 19:33 UTC (permalink / raw)
  To: buildroot

Markus, All,

On 2018-12-17 16:51 -0800, Markus Mayer spake thusly:
> We substitute the path specified in system/skeleton/etc/profile with
> the path specified in the configuration varialbe
> BR2_SYSTEM_DEFAULT_PATH.

You forgot to state in the commit log that $(BR2_SYSTEM_DEFAULT_PATH) is
a kconfig string, so it is already quoted, and that's what we want it to
be in the file.

Regards,
Yann E. MORIN.

> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
>  package/skeleton-init-common/skeleton-init-common.mk | 3 +++
>  system/skeleton/etc/profile                          | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/package/skeleton-init-common/skeleton-init-common.mk b/package/skeleton-init-common/skeleton-init-common.mk
> index e8a052205268..358336599389 100644
> --- a/package/skeleton-init-common/skeleton-init-common.mk
> +++ b/package/skeleton-init-common/skeleton-init-common.mk
> @@ -22,6 +22,9 @@ define SKELETON_INIT_COMMON_INSTALL_TARGET_CMDS
>  	$(call SYSTEM_LIB_SYMLINK,$(TARGET_DIR))
>  	$(INSTALL) -m 0644 support/misc/target-dir-warning.txt \
>  		$(TARGET_DIR_WARNING_FILE)
> +	$(SED) '/.*the PATH below may be replaced.*/d' $(TARGET_DIR)/etc/profile
> +	$(SED) 's|PATH=.*|PATH=$(BR2_SYSTEM_DEFAULT_PATH)|' \
> +		$(TARGET_DIR)/etc/profile
>  endef
>  
>  # We don't care much about what goes in staging, as long as it is
> diff --git a/system/skeleton/etc/profile b/system/skeleton/etc/profile
> index 1255d23ff40d..5ed47743d8f4 100644
> --- a/system/skeleton/etc/profile
> +++ b/system/skeleton/etc/profile
> @@ -1,3 +1,4 @@
> +# At install, the PATH below may be replaced with BR2_SYSTEM_DEFAULT_PATH.
>  export PATH=/bin:/sbin:/usr/bin:/usr/sbin
>  
>  if [ "$PS1" ]; then
> -- 
> 2.17.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 3/4] openssh: use BR2_SYSTEM_DEFAULT_PATH as default PATH
  2018-12-18  0:51 ` [Buildroot] [PATCH 3/4] openssh: " Markus Mayer
@ 2018-12-18 19:38   ` Yann E. MORIN
  0 siblings, 0 replies; 12+ messages in thread
From: Yann E. MORIN @ 2018-12-18 19:38 UTC (permalink / raw)
  To: buildroot

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 OpenSSH sessions.

Ditto, add a blurb that the value is already quoted blah-blah-blah... ;-)

Regards,
Yann E. MORIN.

> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
>  package/openssh/openssh.mk | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/package/openssh/openssh.mk b/package/openssh/openssh.mk
> index 07f3e0d663f6..4ee7727fb9e8 100644
> --- a/package/openssh/openssh.mk
> +++ b/package/openssh/openssh.mk
> @@ -11,6 +11,7 @@ OPENSSH_LICENSE_FILES = LICENCE
>  OPENSSH_CONF_ENV = LD="$(TARGET_CC)" LDFLAGS="$(TARGET_CFLAGS)"
>  OPENSSH_CONF_OPTS = \
>  	--sysconfdir=/etc/ssh \
> +	--with-default-path=$(BR2_SYSTEM_DEFAULT_PATH) \
>  	--disable-lastlog \
>  	--disable-utmp \
>  	--disable-utmpx \
> -- 
> 2.17.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 4/4] dropbear: use BR2_SYSTEM_DEFAULT_PATH as default PATH
  2018-12-18  0:51 ` [Buildroot] [PATCH 4/4] dropbear: " Markus Mayer
@ 2018-12-18 19:47   ` Yann E. MORIN
       [not found]     ` <CAGt4E5uXpteKpCCt_XmKG2HONQ3PQQADHjSyjeBKRPhphxhWhQ@mail.gmail.com>
  0 siblings, 1 reply; 12+ messages in thread
From: Yann E. MORIN @ 2018-12-18 19:47 UTC (permalink / raw)
  To: buildroot

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
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 1/4] system cfg: introduce option BR2_SYSTEM_DEFAULT_PATH
  2018-12-18 19:27   ` Yann E. MORIN
@ 2018-12-18 21:22     ` Markus Mayer
  0 siblings, 0 replies; 12+ messages in thread
From: Markus Mayer @ 2018-12-18 21:22 UTC (permalink / raw)
  To: buildroot

On Tue, 18 Dec 2018 at 11:27, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> Markus, All,
>
> On 2018-12-17 16:51 -0800, Markus Mayer spake thusly:
> > The configuration option BR2_SYSTEM_DEFAULT_PATH allows the user to
> > override the default path, which can be used by /etc/profile and some
> > system daemons.
> >
> > It defaults to the value previously hard-coded in /etc/profile. This
> > default should be suitable for most users.
> >
> > Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> > ---
> >  system/Config.in | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/system/Config.in b/system/Config.in
> > index 0f77b9b6721a..93458c0011b7 100644
> > --- a/system/Config.in
> > +++ b/system/Config.in
> > @@ -386,6 +386,16 @@ comment "automatic network configuration via DHCP needs ifupdown or busybox or n
> >
> >  endif # BR2_ROOTFS_SKELETON_DEFAULT
> >
> > +config BR2_SYSTEM_DEFAULT_PATH
> > +     string "Set the system's default PATH"
> > +     default "/bin:/sbin:/usr/bin:/usr/sbin" if !BR2_ROOTFS_MERGED_USR
> > +     default "/bin:/sbin" if BR2_ROOTFS_MERGED_USR
>
> Do we really care to have a different value at all?
>
> In the merged-/usr case: if the command exists, it is found in either
> /bin or /sbin, and the lookup stops there. If the command does not
> exist, it is looked for in the four locations, instead of only two.
>
> This does not look like too much of an overhead, does it?
>
> Anyway, if we really want different values, then:
>   - we can simplify it a bit,
>   - and use the realy locations rahter than the  symlinks:
>
>     default "/usr/bin:/usr/sbin" if BR2_ROOTFS_MERGED_USR
>     default "/bin:/sbin:/usr/bin:/usr/sbin"
>
> (kconfig will stop on the fiurst default that is valid.)
>
> But for my part, I think a single default as is currently used is
> enough (i.e. /bin:/sbin:/usr/bin:/usr/sbin).

I am perfectly okay with a single value. That's how I had it
originally, until I saw Carlos' e-mail.

I'll let the powers that be make the call on this one. :-) If we do go
with two separate default values, I'll use the above simplification
(with just a single if).

Regards,
-Markus

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

* [Buildroot] [PATCH 4/4] dropbear: use BR2_SYSTEM_DEFAULT_PATH as default PATH
       [not found]     ` <CAGt4E5uXpteKpCCt_XmKG2HONQ3PQQADHjSyjeBKRPhphxhWhQ@mail.gmail.com>
@ 2018-12-18 23:16       ` Markus Mayer
  0 siblings, 0 replies; 12+ messages in thread
From: Markus Mayer @ 2018-12-18 23:16 UTC (permalink / raw)
  To: buildroot

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.

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

end of thread, other threads:[~2018-12-18 23:16 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-18  0:51 [Buildroot] [PATCH 0/4] Allow customization of system default PATH Markus Mayer
2018-12-18  0:51 ` [Buildroot] [PATCH 1/4] system cfg: introduce option BR2_SYSTEM_DEFAULT_PATH Markus Mayer
2018-12-18 19:27   ` Yann E. MORIN
2018-12-18 21:22     ` Markus Mayer
2018-12-18  0:51 ` [Buildroot] [PATCH 2/4] skeleton: use BR2_SYSTEM_DEFAULT_PATH as default PATH Markus Mayer
2018-12-18 19:32   ` Yann E. MORIN
2018-12-18 19:33   ` Yann E. MORIN
2018-12-18  0:51 ` [Buildroot] [PATCH 3/4] openssh: " Markus Mayer
2018-12-18 19:38   ` Yann E. MORIN
2018-12-18  0:51 ` [Buildroot] [PATCH 4/4] dropbear: " Markus Mayer
2018-12-18 19:47   ` Yann E. MORIN
     [not found]     ` <CAGt4E5uXpteKpCCt_XmKG2HONQ3PQQADHjSyjeBKRPhphxhWhQ@mail.gmail.com>
2018-12-18 23:16       ` Markus Mayer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.