[Buildroot] [PATCH v2 1/2] getty: Create specific getty config + cleanups

[Buildroot] [PATCH v2 1/2] getty: Create specific getty config + cleanups

[Assaf Inbal]

This commit introduces a specific BR2_TARGET_GENERIC_GETTY configuration flag.
This eliminates the need for checking if BR2_TARGET_GENERIC_GETTY_PORT is an
empty string or not. It also allows hiding various getty options when getty
isn't enabled.

Signed-off-by: Assaf Inbal <shmuelzon@gmail.com>
---
 system/Config.in |    9 +++++++--
 system/system.mk |   27 ++++++++++++++-------------
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/system/Config.in b/system/Config.in
index d41f184..403b229 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -194,12 +194,16 @@ config BR2_TARGET_GENERIC_ROOT_PASSWD
 	  in the build log! Avoid using a valuable password if either the
 	  .config file or the build log may be distributed!
 
+config BR2_TARGET_GENERIC_GETTY
+	bool "Run a getty (login prompt) after boot"
+	default y
+
+if BR2_TARGET_GENERIC_GETTY
 config BR2_TARGET_GENERIC_GETTY_PORT
 	string "Port to run a getty (login prompt) on"
 	default "ttyS0"
 	help
-	  Specify a port to run a getty (login prompt) on.
-	  Set to the empty string to not run a getty.
+	  Specify a port to run a getty on.
 
 choice
 	prompt "Baudrate to use"
@@ -235,6 +239,7 @@ config BR2_TARGET_GENERIC_GETTY_TERM
 	default "vt100"
 	help
 	  Specify a TERM type.
+endif
 
 config BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW
 	bool "remount root filesystem read-write during boot"
diff --git a/system/system.mk b/system/system.mk
index b4ddc3e..c9e7cf7 100644
--- a/system/system.mk
+++ b/system/system.mk
@@ -1,14 +1,15 @@
-TARGET_GENERIC_HOSTNAME := $(call qstrip,$(BR2_TARGET_GENERIC_HOSTNAME))
-TARGET_GENERIC_ISSUE := $(call qstrip,$(BR2_TARGET_GENERIC_ISSUE))
-TARGET_GENERIC_ROOT_PASSWD := $(call qstrip,$(BR2_TARGET_GENERIC_ROOT_PASSWD))
-TARGET_GENERIC_PASSWD_METHOD := $(call qstrip,$(BR2_TARGET_GENERIC_PASSWD_METHOD))
-TARGET_GENERIC_GETTY := $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_PORT))
-TARGET_GENERIC_GETTY_BAUDRATE := $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE))
-TARGET_GENERIC_GETTY_TERM := $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_TERM))
+TARGET_GENERIC_HOSTNAME = $(call qstrip,$(BR2_TARGET_GENERIC_HOSTNAME))
+TARGET_GENERIC_ISSUE = $(call qstrip,$(BR2_TARGET_GENERIC_ISSUE))
+TARGET_GENERIC_ROOT_PASSWD = $(call qstrip,$(BR2_TARGET_GENERIC_ROOT_PASSWD))
+TARGET_GENERIC_PASSWD_METHOD = $(call qstrip,$(BR2_TARGET_GENERIC_PASSWD_METHOD))
+TARGET_GENERIC_GETTY_PORT = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_PORT))
+TARGET_GENERIC_GETTY_BAUDRATE = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE))
+TARGET_GENERIC_GETTY_TERM = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_TERM))
+TARGET_GENERIC_GETTY_OPTIONS = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_OPTIONS))
 
 target-generic-securetty:
-	grep -q '^$(TARGET_GENERIC_GETTY)$$' $(TARGET_DIR)/etc/securetty || \
-		echo '$(TARGET_GENERIC_GETTY)' >> $(TARGET_DIR)/etc/securetty
+	grep -q '^$(TARGET_GENERIC_GETTY_PORT)$$' $(TARGET_DIR)/etc/securetty || \
+		echo '$(TARGET_GENERIC_GETTY_PORT)' >> $(TARGET_DIR)/etc/securetty
 
 target-generic-hostname:
 	mkdir -p $(TARGET_DIR)/etc
@@ -29,13 +30,13 @@ target-root-passwd:
 	$(SED) "s,^root:[^:]*:,root:$$TARGET_GENERIC_ROOT_PASSWD_HASH:," $(TARGET_DIR)/etc/shadow
 
 target-generic-getty-busybox:
-	$(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(TARGET_GENERIC_GETTY)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
+	$(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(TARGET_GENERIC_GETTY_PORT)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
 		$(TARGET_DIR)/etc/inittab
 
 # In sysvinit inittab, the "id" must not be longer than 4 bytes, so we
 # skip the "tty" part and keep only the remaining.
 target-generic-getty-sysvinit:
-	$(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(shell echo $(TARGET_GENERIC_GETTY) | tail -c+4)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
+	$(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(shell echo $(TARGET_GENERIC_GETTY_PORT) | tail -c+4)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
 		$(TARGET_DIR)/etc/inittab
 
 # Find commented line, if any, and remove leading '#'s
@@ -46,7 +47,7 @@ target-generic-do-remount-rw:
 target-generic-dont-remount-rw:
 	$(SED) '/^[^#].*# REMOUNT_ROOTFS_RW$$/s~^~#~' $(TARGET_DIR)/etc/inittab
 
-ifneq ($(TARGET_GENERIC_GETTY),)
+ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
 TARGETS += target-generic-securetty
 endif
 
@@ -61,7 +62,7 @@ endif
 ifeq ($(BR2_ROOTFS_SKELETON_DEFAULT),y)
 TARGETS += target-root-passwd
 
-ifneq ($(TARGET_GENERIC_GETTY),)
+ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
 TARGETS += target-generic-getty-$(if $(BR2_PACKAGE_SYSVINIT),sysvinit,busybox)
 endif
 
-- 
1.7.9.5

[Buildroot] [PATCH v2 2/2] getty: add the ability to pass options to getty

[Assaf Inbal]

Signed-off-by: Assaf Inbal <shmuelzon@gmail.com>
---
 system/Config.in |    7 +++++++
 system/system.mk |    4 ++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/system/Config.in b/system/Config.in
index 403b229..1a1c34c 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -239,6 +239,13 @@ config BR2_TARGET_GENERIC_GETTY_TERM
 	default "vt100"
 	help
 	  Specify a TERM type.
+
+config BR2_TARGET_GENERIC_GETTY_OPTIONS
+	string "other options to pass to getty"
+	default ""
+	help
+	  Any other flags you want to pass to getty,
+	  Refer to getty --help for details.
 endif
 
 config BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW
diff --git a/system/system.mk b/system/system.mk
index c9e7cf7..01a6c3a 100644
--- a/system/system.mk
+++ b/system/system.mk
@@ -30,13 +30,13 @@ target-root-passwd:
 	$(SED) "s,^root:[^:]*:,root:$$TARGET_GENERIC_ROOT_PASSWD_HASH:," $(TARGET_DIR)/etc/shadow
 
 target-generic-getty-busybox:
-	$(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(TARGET_GENERIC_GETTY_PORT)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
+	$(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(TARGET_GENERIC_GETTY_PORT)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
 		$(TARGET_DIR)/etc/inittab
 
 # In sysvinit inittab, the "id" must not be longer than 4 bytes, so we
 # skip the "tty" part and keep only the remaining.
 target-generic-getty-sysvinit:
-	$(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(shell echo $(TARGET_GENERIC_GETTY_PORT) | tail -c+4)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
+	$(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(shell echo $(TARGET_GENERIC_GETTY_PORT) | tail -c+4)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
 		$(TARGET_DIR)/etc/inittab
 
 # Find commented line, if any, and remove leading '#'s
-- 
1.7.9.5

[Buildroot] [PATCH v2 1/2] getty: Create specific getty config + cleanups

[Thomas De Schampheleire]

Hi Assaf,

Thanks for your patch. Some comments below...

On Sun, Aug 25, 2013 at 1:13 PM, Assaf Inbal <shmuelzon@gmail.com> wrote:
> This commit introduces a specific BR2_TARGET_GENERIC_GETTY configuration flag.
> This eliminates the need for checking if BR2_TARGET_GENERIC_GETTY_PORT is an
> empty string or not. It also allows hiding various getty options when getty
> isn't enabled.
>
> Signed-off-by: Assaf Inbal <shmuelzon@gmail.com>
> ---
>  system/Config.in |    9 +++++++--
>  system/system.mk |   27 ++++++++++++++-------------
>  2 files changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/system/Config.in b/system/Config.in
> index d41f184..403b229 100644
> --- a/system/Config.in
> +++ b/system/Config.in
> @@ -194,12 +194,16 @@ config BR2_TARGET_GENERIC_ROOT_PASSWD
>           in the build log! Avoid using a valuable password if either the
>           .config file or the build log may be distributed!
>
> +config BR2_TARGET_GENERIC_GETTY
> +       bool "Run a getty (login prompt) after boot"
> +       default y
> +
> +if BR2_TARGET_GENERIC_GETTY
>  config BR2_TARGET_GENERIC_GETTY_PORT
>         string "Port to run a getty (login prompt) on"
>         default "ttyS0"
>         help
> -         Specify a port to run a getty (login prompt) on.
> -         Set to the empty string to not run a getty.
> +         Specify a port to run a getty on.
>
>  choice
>         prompt "Baudrate to use"
> @@ -235,6 +239,7 @@ config BR2_TARGET_GENERIC_GETTY_TERM
>         default "vt100"
>         help
>           Specify a TERM type.
> +endif

The text of the suboptions to the new getty top-level option are still
somewhat chaotic:
[*] Run a getty (login prompt) after boot
(ttyS0) Port to run a getty (login prompt) on
Baudrate to use (115200)  --->
(vt100) Value to assign the TERM environment variable
()    other options to pass to getty


My proposal is to simplify the suboptions:
[*] Run a getty (login prompt) after boot
(ttyS0) tty port
baudrate (115200)  --->
(vt100) TERM environment variable
()    other options to pass to getty

>
>  config BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW
>         bool "remount root filesystem read-write during boot"
> diff --git a/system/system.mk b/system/system.mk
> index b4ddc3e..c9e7cf7 100644
> --- a/system/system.mk
> +++ b/system/system.mk
> @@ -1,14 +1,15 @@
> -TARGET_GENERIC_HOSTNAME := $(call qstrip,$(BR2_TARGET_GENERIC_HOSTNAME))
> -TARGET_GENERIC_ISSUE := $(call qstrip,$(BR2_TARGET_GENERIC_ISSUE))
> -TARGET_GENERIC_ROOT_PASSWD := $(call qstrip,$(BR2_TARGET_GENERIC_ROOT_PASSWD))
> -TARGET_GENERIC_PASSWD_METHOD := $(call qstrip,$(BR2_TARGET_GENERIC_PASSWD_METHOD))
> -TARGET_GENERIC_GETTY := $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_PORT))
> -TARGET_GENERIC_GETTY_BAUDRATE := $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE))
> -TARGET_GENERIC_GETTY_TERM := $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_TERM))
> +TARGET_GENERIC_HOSTNAME = $(call qstrip,$(BR2_TARGET_GENERIC_HOSTNAME))
> +TARGET_GENERIC_ISSUE = $(call qstrip,$(BR2_TARGET_GENERIC_ISSUE))
> +TARGET_GENERIC_ROOT_PASSWD = $(call qstrip,$(BR2_TARGET_GENERIC_ROOT_PASSWD))
> +TARGET_GENERIC_PASSWD_METHOD = $(call qstrip,$(BR2_TARGET_GENERIC_PASSWD_METHOD))
> +TARGET_GENERIC_GETTY_PORT = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_PORT))
> +TARGET_GENERIC_GETTY_BAUDRATE = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE))
> +TARGET_GENERIC_GETTY_TERM = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_TERM))
> +TARGET_GENERIC_GETTY_OPTIONS = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_OPTIONS))

Although in general = is favored over := in buildroot, I'm now
wondering if this is the right choice here: variable
TARGET_GENERIC_GETTY_PORT is used several times in .mk. If = is used,
the qstrip expansion is done each time, while := will do it once.

Arnout: isn't this ($(call), $(shell), ...) a case where we should
rather use := iso = ?

>
>  target-generic-securetty:
> -       grep -q '^$(TARGET_GENERIC_GETTY)$$' $(TARGET_DIR)/etc/securetty || \
> -               echo '$(TARGET_GENERIC_GETTY)' >> $(TARGET_DIR)/etc/securetty
> +       grep -q '^$(TARGET_GENERIC_GETTY_PORT)$$' $(TARGET_DIR)/etc/securetty || \
> +               echo '$(TARGET_GENERIC_GETTY_PORT)' >> $(TARGET_DIR)/etc/securetty
>
>  target-generic-hostname:
>         mkdir -p $(TARGET_DIR)/etc
> @@ -29,13 +30,13 @@ target-root-passwd:
>         $(SED) "s,^root:[^:]*:,root:$$TARGET_GENERIC_ROOT_PASSWD_HASH:," $(TARGET_DIR)/etc/shadow
>
>  target-generic-getty-busybox:
> -       $(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(TARGET_GENERIC_GETTY)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
> +       $(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(TARGET_GENERIC_GETTY_PORT)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
>                 $(TARGET_DIR)/etc/inittab
>
>  # In sysvinit inittab, the "id" must not be longer than 4 bytes, so we
>  # skip the "tty" part and keep only the remaining.
>  target-generic-getty-sysvinit:
> -       $(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(shell echo $(TARGET_GENERIC_GETTY) | tail -c+4)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
> +       $(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(shell echo $(TARGET_GENERIC_GETTY_PORT) | tail -c+4)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
>                 $(TARGET_DIR)/etc/inittab
>
>  # Find commented line, if any, and remove leading '#'s
> @@ -46,7 +47,7 @@ target-generic-do-remount-rw:
>  target-generic-dont-remount-rw:
>         $(SED) '/^[^#].*# REMOUNT_ROOTFS_RW$$/s~^~#~' $(TARGET_DIR)/etc/inittab
>
> -ifneq ($(TARGET_GENERIC_GETTY),)
> +ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
>  TARGETS += target-generic-securetty
>  endif
>
> @@ -61,7 +62,7 @@ endif
>  ifeq ($(BR2_ROOTFS_SKELETON_DEFAULT),y)
>  TARGETS += target-root-passwd
>
> -ifneq ($(TARGET_GENERIC_GETTY),)
> +ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
>  TARGETS += target-generic-getty-$(if $(BR2_PACKAGE_SYSVINIT),sysvinit,busybox)
>  endif
>
> --
> 1.7.9.5

Best regards,
Thomas

[Buildroot] [PATCH v2 2/2] getty: add the ability to pass options to getty

[Thomas De Schampheleire]

On Sun, Aug 25, 2013 at 1:13 PM, Assaf Inbal <shmuelzon@gmail.com> wrote:
> Signed-off-by: Assaf Inbal <shmuelzon@gmail.com>
> ---
>  system/Config.in |    7 +++++++
>  system/system.mk |    4 ++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/system/Config.in b/system/Config.in
> index 403b229..1a1c34c 100644
> --- a/system/Config.in
> +++ b/system/Config.in
> @@ -239,6 +239,13 @@ config BR2_TARGET_GENERIC_GETTY_TERM
>         default "vt100"
>         help
>           Specify a TERM type.
> +
> +config BR2_TARGET_GENERIC_GETTY_OPTIONS
> +       string "other options to pass to getty"
> +       default ""
> +       help
> +         Any other flags you want to pass to getty,
> +         Refer to getty --help for details.
>  endif
>
>  config BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW
> diff --git a/system/system.mk b/system/system.mk
> index c9e7cf7..01a6c3a 100644
> --- a/system/system.mk
> +++ b/system/system.mk
> @@ -30,13 +30,13 @@ target-root-passwd:
>         $(SED) "s,^root:[^:]*:,root:$$TARGET_GENERIC_ROOT_PASSWD_HASH:," $(TARGET_DIR)/etc/shadow
>
>  target-generic-getty-busybox:
> -       $(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(TARGET_GENERIC_GETTY_PORT)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
> +       $(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(TARGET_GENERIC_GETTY_PORT)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
>                 $(TARGET_DIR)/etc/inittab
>
>  # In sysvinit inittab, the "id" must not be longer than 4 bytes, so we
>  # skip the "tty" part and keep only the remaining.
>  target-generic-getty-sysvinit:
> -       $(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(shell echo $(TARGET_GENERIC_GETTY_PORT) | tail -c+4)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
> +       $(SED) '/# GENERIC_SERIAL$$/s~^.*#~$(shell echo $(TARGET_GENERIC_GETTY_PORT) | tail -c+4)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY_PORT) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
>                 $(TARGET_DIR)/etc/inittab
>
>  # Find commented line, if any, and remove leading '#'s

Acked-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

[Buildroot] [PATCH v2 1/2] getty: Create specific getty config + cleanups

[Arnout Vandecappelle]

On 08/27/13 12:33, Thomas De Schampheleire wrote:
>> >+TARGET_GENERIC_HOSTNAME = $(call qstrip,$(BR2_TARGET_GENERIC_HOSTNAME))
>> >+TARGET_GENERIC_ISSUE = $(call qstrip,$(BR2_TARGET_GENERIC_ISSUE))
>> >+TARGET_GENERIC_ROOT_PASSWD = $(call qstrip,$(BR2_TARGET_GENERIC_ROOT_PASSWD))
>> >+TARGET_GENERIC_PASSWD_METHOD = $(call qstrip,$(BR2_TARGET_GENERIC_PASSWD_METHOD))
>> >+TARGET_GENERIC_GETTY_PORT = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_PORT))
>> >+TARGET_GENERIC_GETTY_BAUDRATE = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE))
>> >+TARGET_GENERIC_GETTY_TERM = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_TERM))
>> >+TARGET_GENERIC_GETTY_OPTIONS = $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_OPTIONS))
> Although in general = is favored over := in buildroot, I'm now
> wondering if this is the right choice here: variable
> TARGET_GENERIC_GETTY_PORT is used several times in .mk. If = is used,
> the qstrip expansion is done each time, while := will do it once.

  Remember that := will expand once, even if it is never used. So using = 
is sometimes even more efficient.


> Arnout: isn't this ($(call), $(shell), ...) a case where we should
> rather use := iso = ?

  For $(shell), yes (on condition that it is _always_ evaluated). But the 
overhead of $(call) isn't very high. These variables are used a couple of 
times, but every time it is within a separate sub-shell - the overhead of 
starting that sub-shell is way higher than the overhead of running a $(call).

  So no, this is not a case where := would be more appropriate. It's not 
even a case where := is necessarily faster.

  Regards,
  Arnout


-- 
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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2 1/2] getty: Create specific getty config + cleanups

[Assaf Inbal]

Hey Thomas,

The text of the suboptions to the new getty top-level option are still
> somewhat chaotic:
> [*] Run a getty (login prompt) after boot
> (ttyS0) Port to run a getty (login prompt) on
> Baudrate to use (115200)  --->
> (vt100) Value to assign the TERM environment variable
> ()    other options to pass to getty
>
>
> My proposal is to simplify the suboptions:
> [*] Run a getty (login prompt) after boot
> (ttyS0) tty port
> baudrate (115200)  --->
> (vt100) TERM environment variable
> ()    other options to pass to getty
>

Would you like to me submit another version of the patch with your wording,
or will you do this yourself when applying the patch?

Good day,
Assaf
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20130828/3ef3326e/attachment.html>

[Buildroot] [PATCH v2 1/2] getty: Create specific getty config + cleanups

[Thomas De Schampheleire]

Hi Assaf,

On Wed, Aug 28, 2013 at 6:41 AM, Assaf Inbal <shmuelzon@gmail.com> wrote:
> Hey Thomas,
>
>> The text of the suboptions to the new getty top-level option are still
>> somewhat chaotic:
>> [*] Run a getty (login prompt) after boot
>> (ttyS0) Port to run a getty (login prompt) on
>> Baudrate to use (115200)  --->
>> (vt100) Value to assign the TERM environment variable
>> ()    other options to pass to getty
>>
>>
>> My proposal is to simplify the suboptions:
>> [*] Run a getty (login prompt) after boot
>> (ttyS0) tty port
>> baudrate (115200)  --->
>> (vt100) TERM environment variable
>> ()    other options to pass to getty
>
>
> Would you like to me submit another version of the patch with your wording,
> or will you do this yourself when applying the patch?

I'm just a 'regular' developer like you, I cannot apply the patch to
the buildroot repo. Only the maintainer Peter Korsgaard can do that
(or recently the maintainer-ad-interim Thomas Petazzoni)
I think it is best if you submit a new version, to minimize the work
he has to do.

Thanks again for your contributions.

Best regards,
Thomas

[Buildroot] [PATCH v2 1/2] getty: Create specific getty config + cleanups

[Thomas De Schampheleire]

Hi Assaf,

On Wed, Aug 28, 2013 at 8:43 AM, Assaf Inbal <shmuelzon@gmail.com> wrote:
> Hey,
>
>> I'm just a 'regular' developer like you,
>
>
> Must have gotten my Thomas's mixed up :)
> Sorry about that.
> I'll send in a new patch shortly.

I haven't seen a new version yet. When you have some time, could you
do it? I think the patch series is a nice improvement so I don't want
to loose this...

Thanks,
Thomas

[Buildroot] [PATCH v2 1/2] getty: Create specific getty config + cleanups

[Assaf Inbal]

Hey,

> I haven't seen a new version yet. When you have some time, could you
> do it? I think the patch series is a nice improvement so I don't want
> to loose this...

Sorry, got caught up by my actual day job :)
Two quick questions:
- Would it be better to create a sub menu called "getty options"? I
think that otherwise it would be clear what these options refer to. It
would also clean up the main "system" menu.
- In order to avoid the new thread that started with the 2nd version
of the patch, how should I configure git send-email to reply to this
thread? I altered the "reply-to" field but that didn't seem to work.

Good day,
Assaf

[Buildroot] [PATCH v2 1/2] getty: Create specific getty config + cleanups

[Thomas De Schampheleire]

On Fri, Sep 6, 2013 at 8:59 AM, Assaf Inbal <shmuelzon@gmail.com> wrote:
> Hey,
>
>> I haven't seen a new version yet. When you have some time, could you
>> do it? I think the patch series is a nice improvement so I don't want
>> to loose this...
>
> Sorry, got caught up by my actual day job :)
> Two quick questions:
> - Would it be better to create a sub menu called "getty options"? I
> think that otherwise it would be clear what these options refer to. It
> would also clean up the main "system" menu.

We don't usually have real submenus in buildroot, but I don't have a
very strong opinion on this.

> - In order to avoid the new thread that started with the 2nd version
> of the patch, how should I configure git send-email to reply to this
> thread? I altered the "reply-to" field but that didn't seem to work.

There are different opinions in the buildroot community regarding the
desirability of such in-reply-to patches. I'm personally not an
opponent of it, for the following reasons:

1. from the list of e-mails (in gmail for example) it is unclear if
the new reply is just a comment on the current version of the patch,
or if it is a new version. You really have to check the reply.

2. it doesn't work well for patch series: the first patch would indeed
be a reply to the previous thread, but all other patches would not.
This makes it very annoying to read all patches in a series.

Therefore, I always send new versions as new standalone e-mails, with
a marker vX to indicate the version. Several other buildroot
contributors do the same.

Best regards,
Thomas