Patchwork getty: add the ability to pass options to getty

login
register
mail settings
Submitter Assaf Inbal
Date Aug. 22, 2013, 5:12 a.m.
Message ID <1377148355-21125-1-git-send-email-shmuelzon@gmail.com>
Download mbox | patch
Permalink /patch/268954/
State Superseded
Headers show

Comments

Assaf Inbal - Aug. 22, 2013, 5:12 a.m.
Signed-off-by: Assaf Inbal <shmuelzon@gmail.com>
---
 system/Config.in |    7 +++++++
 system/system.mk |    5 +++--
 2 files changed, 10 insertions(+), 2 deletions(-)
Arnout Vandecappelle - Aug. 22, 2013, 6:02 a.m.
On 22/08/13 07:12, Assaf Inbal wrote:
> Signed-off-by: Assaf Inbal <shmuelzon@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
  (for -next)

  With some minor fixes, though:

> ---
>   system/Config.in |    7 +++++++
>   system/system.mk |    5 +++--
>   2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/system/Config.in b/system/Config.in
> index d41f184..16558bf 100644
> --- a/system/Config.in
> +++ b/system/Config.in
> @@ -230,6 +230,13 @@ config BR2_TARGET_GENERIC_GETTY_BAUDRATE
>   	default "57600"		if BR2_TARGET_GENERIC_GETTY_BAUDRATE_57600
>   	default "115200"	if BR2_TARGET_GENERIC_GETTY_BAUDRATE_115200
>
> +config BR2_TARGET_GENERIC_GETTY_OPTIONS
> +	string "other random options to pass to getty"
> +	default ""
> +	help
> +	  Any other flags you want to pass to gettey,

  gettey -> getty

> +	  Refer to getty --help for details.
> +
>   config BR2_TARGET_GENERIC_GETTY_TERM
>   	string "Value to assign the TERM environment variable"
>   	default "vt100"
> diff --git a/system/system.mk b/system/system.mk
> index b4ddc3e..925f3f4 100644
> --- a/system/system.mk
> +++ b/system/system.mk
> @@ -5,6 +5,7 @@ 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_GETTY_OPTIONS:=$(call qstrip,$(BR2_TARGET_GENERIC_GETTY_OPTIONS))

  Don't copy the incorrect coding style :-) Use = instead of := and put 
spaces around it.

>
>   target-generic-securetty:
>   	grep -q '^$(TARGET_GENERIC_GETTY)$$' $(TARGET_DIR)/etc/securetty || \
> @@ -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)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \

  This line could really use some splitting... But maybe that's not the 
responsibility of this patch.


  Regards,
  Arnout

>   		$(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) | tail -c+4)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
>   		$(TARGET_DIR)/etc/inittab
>
>   # Find commented line, if any, and remove leading '#'s
>
Assaf Inbal - Aug. 22, 2013, 7:28 a.m.
Hey,

 Don't copy the incorrect coding style :-) Use = instead of := and put
> spaces around it.


Just out of curiosity, why would you rather I use =? Is it because that
macro won't always be evaluated, or is there something else I'm missing?

BTW - Would you like me to submit an updated patch?

Good day,
Assaf
Thomas De Schampheleire - Aug. 22, 2013, 10:58 a.m.
On Thu, Aug 22, 2013 at 7:12 AM, Assaf Inbal <shmuelzon@gmail.com> wrote:
> Signed-off-by: Assaf Inbal <shmuelzon@gmail.com>
> ---
>  system/Config.in |    7 +++++++
>  system/system.mk |    5 +++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/system/Config.in b/system/Config.in
> index d41f184..16558bf 100644
> --- a/system/Config.in
> +++ b/system/Config.in
> @@ -230,6 +230,13 @@ config BR2_TARGET_GENERIC_GETTY_BAUDRATE
>         default "57600"         if BR2_TARGET_GENERIC_GETTY_BAUDRATE_57600
>         default "115200"        if BR2_TARGET_GENERIC_GETTY_BAUDRATE_115200
>
> +config BR2_TARGET_GENERIC_GETTY_OPTIONS
> +       string "other random options to pass to getty"

I would remove the 'random': options you pass here are not random but
probably well-chosen.

> +       default ""
> +       help
> +         Any other flags you want to pass to gettey,
> +         Refer to getty --help for details.
> +
>  config BR2_TARGET_GENERIC_GETTY_TERM
>         string "Value to assign the TERM environment variable"
>         default "vt100"


There are now a number of getty-related config options in
system/Config.in, but without any structure.
When the getty port is empty, no action is taken.
What about introducing a new bool option to determine if a getty needs
to be started, with suboptions for port, baudrate, term, and
user-configurable options?
Something like:

config BR2_TARGET_GENERIC_GETTY
        bool "Run a getty (login prompt) after boot"

config BR2_TARGET_GENERIC_GETTY_PORT
        bool "tty port"
        depends on BR2_TARGET_GENERIC_GETTY

choice
        prompt "baudrate"
        depends on BR2_TARGET_GENERIC_GETTY

[baudrate options]
endchoice

config BR2_TARGET_GENERIC_GETTY_TERM
        bool "TERM environment variable"
        depends on BR2_TARGET_GENERIC_GETTY

config BR2_TARGET_GENERIC_GETTY_OPTIONS
        string "other options to pass to getty"
        depends on BR2_TARGET_GENERIC_GETTY


This creates some structure, and hides all non-relevant options when
the user does not want a getty.
In this case, system/system.mk will need a few adaptations to check
the new symbol instead of for the emptiness of _PORT.

Best regards,
Thomas


> diff --git a/system/system.mk b/system/system.mk
> index b4ddc3e..925f3f4 100644
> --- a/system/system.mk
> +++ b/system/system.mk
> @@ -5,6 +5,7 @@ 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_GETTY_OPTIONS:=$(call qstrip,$(BR2_TARGET_GENERIC_GETTY_OPTIONS))
>
>  target-generic-securetty:
>         grep -q '^$(TARGET_GENERIC_GETTY)$$' $(TARGET_DIR)/etc/securetty || \
> @@ -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)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY) $(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) | tail -c+4)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY) $(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 mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Thomas De Schampheleire - Aug. 22, 2013, 11:01 a.m.
Hi Assaf,

On Thu, Aug 22, 2013 at 9:28 AM, Assaf Inbal <shmuelzon@gmail.com> wrote:
> Hey,
>
>>  Don't copy the incorrect coding style :-) Use = instead of := and put
>> spaces around it.
>
>
> Just out of curiosity, why would you rather I use =? Is it because that
> macro won't always be evaluated, or is there something else I'm missing?

Technically, in this case, both := and = are fine, and both produce
the same result because the right-hand side is fixed and does not
reference any other make variable.
The more unusual := is thus not really needed, and therefore it has
become buildroot policy to use the simple = in all cases, with spaces
as delimiter. This is also visually the most attractive.

>
> BTW - Would you like me to submit an updated patch?

Yes please. Review the comments provided and send an updated patch
marked as 'PATCH v2'.

Best regards,
Thomas
Arnout Vandecappelle - Aug. 22, 2013, 4:06 p.m.
On 22/08/13 12:58, Thomas De Schampheleire wrote:
> There are now a number of getty-related config options in
> system/Config.in, but without any structure.
> When the getty port is empty, no action is taken.
> What about introducing a new bool option to determine if a getty needs
> to be started, with suboptions for port, baudrate, term, and
> user-configurable options?
> Something like:
>
> config BR2_TARGET_GENERIC_GETTY
>          bool "Run a getty (login prompt) after boot"
>
> config BR2_TARGET_GENERIC_GETTY_PORT
>          bool "tty port"
>          depends on BR2_TARGET_GENERIC_GETTY
>
> choice
>          prompt "baudrate"
>          depends on BR2_TARGET_GENERIC_GETTY
>
> [baudrate options]
> endchoice
>
> config BR2_TARGET_GENERIC_GETTY_TERM
>          bool "TERM environment variable"
>          depends on BR2_TARGET_GENERIC_GETTY
>
> config BR2_TARGET_GENERIC_GETTY_OPTIONS
>          string "other options to pass to getty"
>          depends on BR2_TARGET_GENERIC_GETTY

  Sounds like a good plan. Except that the sub-options should be wrapped 
in a if...endif construct instead of repeating 'depends on'.

  Regards,
  Arnout
Assaf Inbal - Aug. 25, 2013, 11:18 a.m.
Hey,

Yes please. Review the comments provided and send an updated patch
> marked as 'PATCH v2'.
>

Sadly, it seems that my git-send-email-foo isn't that good since it didn't
attach my new patches to this thread.
In any case, I sent out two patches, one for cleaning up getty a bit as
discussed, and the other for adding the original command line options that
started this thread.

Good day,
Assaf

Patch

diff --git a/system/Config.in b/system/Config.in
index d41f184..16558bf 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -230,6 +230,13 @@  config BR2_TARGET_GENERIC_GETTY_BAUDRATE
 	default "57600"		if BR2_TARGET_GENERIC_GETTY_BAUDRATE_57600
 	default "115200"	if BR2_TARGET_GENERIC_GETTY_BAUDRATE_115200
 
+config BR2_TARGET_GENERIC_GETTY_OPTIONS
+	string "other random options to pass to getty"
+	default ""
+	help
+	  Any other flags you want to pass to gettey,
+	  Refer to getty --help for details.
+
 config BR2_TARGET_GENERIC_GETTY_TERM
 	string "Value to assign the TERM environment variable"
 	default "vt100"
diff --git a/system/system.mk b/system/system.mk
index b4ddc3e..925f3f4 100644
--- a/system/system.mk
+++ b/system/system.mk
@@ -5,6 +5,7 @@  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_GETTY_OPTIONS:=$(call qstrip,$(BR2_TARGET_GENERIC_GETTY_OPTIONS))
 
 target-generic-securetty:
 	grep -q '^$(TARGET_GENERIC_GETTY)$$' $(TARGET_DIR)/etc/securetty || \
@@ -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)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY) $(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) | tail -c+4)::respawn:/sbin/getty -L $(TARGET_GENERIC_GETTY_OPTIONS) $(TARGET_GENERIC_GETTY) $(TARGET_GENERIC_GETTY_BAUDRATE) $(TARGET_GENERIC_GETTY_TERM) #~' \
 		$(TARGET_DIR)/etc/inittab
 
 # Find commented line, if any, and remove leading '#'s