| Message ID | 20180110221402.19743-1-thomas.petazzoni@free-electrons.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | system/Config.in: hide BR2_TARGET_GENERIC_GETTY_{TERM, OPTIONS} with systemd | expand |
On 2018-01-10 23:14 +0100, Thomas Petazzoni spake thusly: > When systemd is used as the init system, the > BR2_TARGET_GENERIC_GETTY_TERM and BR2_TARGET_GENERIC_GETTY_OPTIONS are > completely ignored, which might confuse the user, as reported in bug > > For now, let's simply make those options visible only with busybox > init and sysv init, as suggested by Yann E. Morin in the discussion of > bug #10301. > > Fixes #10301. > > Reported-by: Michael Heinemann <posted@heine.so> > Suggested-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > system/Config.in | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/system/Config.in b/system/Config.in > index b9714923c4..0fe0fb60b4 100644 > --- a/system/Config.in > +++ b/system/Config.in > @@ -355,12 +355,16 @@ config BR2_TARGET_GENERIC_GETTY_BAUDRATE > config BR2_TARGET_GENERIC_GETTY_TERM > string "TERM environment variable" > default "vt100" > + # currently not observed by systemd > + depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV Why not: depends on !BR2_INIT_SYSTEMD because this is really want we want to express: this is not compatible with systemd... > help > Specify a TERM type. > > config BR2_TARGET_GENERIC_GETTY_OPTIONS > string "other options to pass to getty" > default "" > + # currently not observed by systemd > + depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV Ditto... Regards, Yann E. MORIN. > help > Any other flags you want to pass to getty, > Refer to getty --help for details. > -- > 2.14.3 >
Hello, On Thu, 11 Jan 2018 18:31:04 +0100, Yann E. MORIN wrote: > > + # currently not observed by systemd > > + depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV > > Why not: > depends on !BR2_INIT_SYSTEMD > > because this is really want we want to express: this is not compatible > with systemd... No, what we really want to express is that BR2_TARGET_GENERIC_GETTY_{TERM,OPTIONS} are only handled in the busybox code and sysvinit code. It's not so much that they are not compatible with systemd, it is that only busybox and sysvinit observe those options. Best regards, Thomas
Thomas, All, On 2018-01-12 10:02 +0100, Thomas Petazzoni spake thusly: > On Thu, 11 Jan 2018 18:31:04 +0100, Yann E. MORIN wrote: > > > + # currently not observed by systemd > > > + depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV > > > > Why not: > > depends on !BR2_INIT_SYSTEMD > > > > because this is really want we want to express: this is not compatible > > with systemd... > > No, what we really want to express is that > BR2_TARGET_GENERIC_GETTY_{TERM,OPTIONS} are only handled in the busybox > code and sysvinit code. It's not so much that they are not compatible > with systemd, it is that only busybox and sysvinit observe those > options. Then change the title and commit to something like: system: only expose getty option for busybox and sysvinit Only busybox and sysvinit handle those options; the other init systems do not. So, protect those options behind appropriate dependencies on busybox or sysvinit. Fixes #10301. And then you can add my: Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Regards, Yann E. MORIN.
Hello, On Fri, 12 Jan 2018 18:39:53 +0100, Yann E. MORIN wrote: > Then change the title and commit to something like: > > system: only expose getty option for busybox and sysvinit > > Only busybox and sysvinit handle those options; the other init > systems do not. > > So, protect those options behind appropriate dependencies on > busybox or sysvinit. > > Fixes #10301. > > And then you can add my: > > Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> ACK, thanks for the feedback! Thomas
Hello, On Wed, 10 Jan 2018 23:14:02 +0100, Thomas Petazzoni wrote: > When systemd is used as the init system, the > BR2_TARGET_GENERIC_GETTY_TERM and BR2_TARGET_GENERIC_GETTY_OPTIONS are > completely ignored, which might confuse the user, as reported in bug > > For now, let's simply make those options visible only with busybox > init and sysv init, as suggested by Yann E. Morin in the discussion of > bug #10301. > > Fixes #10301. > > Reported-by: Michael Heinemann <posted@heine.so> > Suggested-by: "Yann E. MORIN" <yann.morin.1998@free.fr> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > system/Config.in | 4 ++++ > 1 file changed, 4 insertions(+) I've applied, after taking into account the feedback from Yann about the wording of the commit title and commit log. Thanks! Thomas
diff --git a/system/Config.in b/system/Config.in index b9714923c4..0fe0fb60b4 100644 --- a/system/Config.in +++ b/system/Config.in @@ -355,12 +355,16 @@ config BR2_TARGET_GENERIC_GETTY_BAUDRATE config BR2_TARGET_GENERIC_GETTY_TERM string "TERM environment variable" default "vt100" + # currently not observed by systemd + depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV help Specify a TERM type. config BR2_TARGET_GENERIC_GETTY_OPTIONS string "other options to pass to getty" default "" + # currently not observed by systemd + depends on BR2_INIT_BUSYBOX || BR2_INIT_SYSV help Any other flags you want to pass to getty, Refer to getty --help for details.
When systemd is used as the init system, the BR2_TARGET_GENERIC_GETTY_TERM and BR2_TARGET_GENERIC_GETTY_OPTIONS are completely ignored, which might confuse the user, as reported in bug For now, let's simply make those options visible only with busybox init and sysv init, as suggested by Yann E. Morin in the discussion of bug #10301. Fixes #10301. Reported-by: Michael Heinemann <posted@heine.so> Suggested-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- system/Config.in | 4 ++++ 1 file changed, 4 insertions(+)