| Message ID | 1377148355-21125-1-git-send-email-shmuelzon@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
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 >
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
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
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
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
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
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
Signed-off-by: Assaf Inbal <shmuelzon@gmail.com> --- system/Config.in | 7 +++++++ system/system.mk | 5 +++-- 2 files changed, 10 insertions(+), 2 deletions(-)