diff mbox series

[1/1] package/busybox: add systemd service for telnetd

Message ID 20210826210241.56201-1-sam.voss@collins.com
State Accepted
Headers show
Series [1/1] package/busybox: add systemd service for telnetd | expand

Commit Message

Sam Voss Aug. 26, 2021, 9:02 p.m. UTC 
Signed-off-by: Sam Voss <sam.voss@collins.com>
---
 package/busybox/busybox.mk      |  7 +++++++
 package/busybox/telnetd.service | 11 +++++++++++
 2 files changed, 18 insertions(+)
 create mode 100644 package/busybox/telnetd.service

Comments

Arnout Vandecappelle Aug. 26, 2021, 10:01 p.m. UTC | #1 
On 26/08/2021 23:02, Sam Voss via buildroot wrote:
> Signed-off-by: Sam Voss <sam.voss@collins.com>
> ---
>  package/busybox/busybox.mk      |  7 +++++++
>  package/busybox/telnetd.service | 11 +++++++++++
>  2 files changed, 18 insertions(+)
>  create mode 100644 package/busybox/telnetd.service
> 
> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
> index cfc06b0d78..fb427faf1c 100644
> --- a/package/busybox/busybox.mk
> +++ b/package/busybox/busybox.mk
> @@ -378,6 +378,13 @@ define BUSYBOX_INSTALL_TARGET_CMDS
>  	$(BUSYBOX_INSTALL_MDEV_CONF)
>  endef
>  
> +define BUSYBOX_INSTALL_INIT_SYSTEMD
> +	if grep -q CONFIG_FEATURE_TELNETD_STANDALONE=y $(@D)/.config; then \
> +		$(INSTALL) -D -m 0644 package/busybox/telnetd.service \
> +			$(TARGET_DIR)/usr/lib/systemd/system/telnetd.service ; \
> +	fi
> +endef
> +
>  # Install the sysvinit scripts, for the moment, but not those that already
>  # have a corresponding one in openrc.
>  define BUSYBOX_INSTALL_INIT_OPENRC
> diff --git a/package/busybox/telnetd.service b/package/busybox/telnetd.service
> new file mode 100644
> index 0000000000..3c1fa457b1
> --- /dev/null
> +++ b/package/busybox/telnetd.service
> @@ -0,0 +1,11 @@
> +[Unit]
> +Description=Telnetd Service
> +After=network.target
> +
> +[Service]
> +# busybox telnet doesn't support pidfile, so run in foreground

 AFAIU Type=simple is anyway the preferred type for services without specific
systemd support, so I don't think this comment is very useful...

 Regardless:

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

 Regards,
 Arnout

> +Type=simple
> +ExecStart=/usr/sbin/telnetd -F
> +
> +[Install]
> +WantedBy=multi-user.target
>
Voss, Samuel M Collins via buildroot Aug. 27, 2021, 3:50 p.m. UTC | #2 
Arnout,

> -----Original Message-----
> From: Arnout Vandecappelle <arnout@mind.be>
> Sent: Thursday, August 26, 2021 5:01 PM
> To: Voss, Samuel M Collins <sam.voss@collins.com>;
> buildroot@buildroot.org
> Subject: [External] Re: [Buildroot] [PATCH 1/1] package/busybox: add
> systemd service for telnetd
> 
> 
> 
> On 26/08/2021 23:02, Sam Voss via buildroot wrote:
> > Signed-off-by: Sam Voss <sam.voss@collins.com>
> > ---
> >  package/busybox/busybox.mk      |  7 +++++++
> >  package/busybox/telnetd.service | 11 +++++++++++
> >  2 files changed, 18 insertions(+)
> >  create mode 100644 package/busybox/telnetd.service
> >
> > diff --git a/package/busybox/busybox.mk
> b/package/busybox/busybox.mk
> > index cfc06b0d78..fb427faf1c 100644
> > --- a/package/busybox/busybox.mk
> > +++ b/package/busybox/busybox.mk
> > @@ -378,6 +378,13 @@ define BUSYBOX_INSTALL_TARGET_CMDS
> >  	$(BUSYBOX_INSTALL_MDEV_CONF)
> >  endef
> >
> > +define BUSYBOX_INSTALL_INIT_SYSTEMD
> > +	if grep -q CONFIG_FEATURE_TELNETD_STANDALONE=y
> $(@D)/.config; then \
> > +		$(INSTALL) -D -m 0644 package/busybox/telnetd.service \
> > +
> 	$(TARGET_DIR)/usr/lib/systemd/system/telnetd.service ; \
> > +	fi
> > +endef
> > +
> >  # Install the sysvinit scripts, for the moment, but not those that
> > already  # have a corresponding one in openrc.
> >  define BUSYBOX_INSTALL_INIT_OPENRC
> > diff --git a/package/busybox/telnetd.service
> > b/package/busybox/telnetd.service new file mode 100644 index
> > 0000000000..3c1fa457b1
> > --- /dev/null
> > +++ b/package/busybox/telnetd.service
> > @@ -0,0 +1,11 @@
> > +[Unit]
> > +Description=Telnetd Service
> > +After=network.target
> > +
> > +[Service]
> > +# busybox telnet doesn't support pidfile, so run in foreground
> 
>  AFAIU Type=simple is anyway the preferred type for services without
> specific systemd support, so I don't think this comment is very useful...

Fair enough, I didn't know that was the specific guidance, so I'm okay with dropping that comment when applying.

Thanks for letting me know,

Sam

> 
>  Regardless:
> 
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
>  Regards,
>  Arnout
> 
> > +Type=simple
> > +ExecStart=/usr/sbin/telnetd -F
> > +
> > +[Install]
> > +WantedBy=multi-user.target
> >
Arnout Vandecappelle Aug. 27, 2021, 4:30 p.m. UTC | #3 
On 27/08/2021 17:50, Voss, Samuel M Collins wrote:
> Arnout,
> 
>> -----Original Message-----
>> From: Arnout Vandecappelle <arnout@mind.be>
>> Sent: Thursday, August 26, 2021 5:01 PM
>> To: Voss, Samuel M Collins <sam.voss@collins.com>;
>> buildroot@buildroot.org
>> Subject: [External] Re: [Buildroot] [PATCH 1/1] package/busybox: add
>> systemd service for telnetd
>>
>>
>>
>> On 26/08/2021 23:02, Sam Voss via buildroot wrote:
>>> Signed-off-by: Sam Voss <sam.voss@collins.com>
>>> ---
>>>  package/busybox/busybox.mk      |  7 +++++++
>>>  package/busybox/telnetd.service | 11 +++++++++++
>>>  2 files changed, 18 insertions(+)
>>>  create mode 100644 package/busybox/telnetd.service
>>>
>>> diff --git a/package/busybox/busybox.mk
>> b/package/busybox/busybox.mk
>>> index cfc06b0d78..fb427faf1c 100644
>>> --- a/package/busybox/busybox.mk
>>> +++ b/package/busybox/busybox.mk
>>> @@ -378,6 +378,13 @@ define BUSYBOX_INSTALL_TARGET_CMDS
>>>  	$(BUSYBOX_INSTALL_MDEV_CONF)
>>>  endef
>>>
>>> +define BUSYBOX_INSTALL_INIT_SYSTEMD
>>> +	if grep -q CONFIG_FEATURE_TELNETD_STANDALONE=y
>> $(@D)/.config; then \
>>> +		$(INSTALL) -D -m 0644 package/busybox/telnetd.service \
>>> +
>> 	$(TARGET_DIR)/usr/lib/systemd/system/telnetd.service ; \
>>> +	fi
>>> +endef
>>> +
>>>  # Install the sysvinit scripts, for the moment, but not those that
>>> already  # have a corresponding one in openrc.
>>>  define BUSYBOX_INSTALL_INIT_OPENRC
>>> diff --git a/package/busybox/telnetd.service
>>> b/package/busybox/telnetd.service new file mode 100644 index
>>> 0000000000..3c1fa457b1
>>> --- /dev/null
>>> +++ b/package/busybox/telnetd.service
>>> @@ -0,0 +1,11 @@
>>> +[Unit]
>>> +Description=Telnetd Service
>>> +After=network.target
>>> +
>>> +[Service]
>>> +# busybox telnet doesn't support pidfile, so run in foreground
>>
>>  AFAIU Type=simple is anyway the preferred type for services without
>> specific systemd support, so I don't think this comment is very useful...
> 
> Fair enough, I didn't know that was the specific guidance, so I'm okay with dropping that comment when applying.

 From the systemd.service(5) man page:

It is generally recommended to use Type=simple for long-running services
whenever possible, as it is the simplest and fastest option.


 Regards,
 Arnout

> 
> Thanks for letting me know,
> 
> Sam
> 
>>
>>  Regardless:
>>
>> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>>
>>  Regards,
>>  Arnout
>>
>>> +Type=simple
>>> +ExecStart=/usr/sbin/telnetd -F
>>> +
>>> +[Install]
>>> +WantedBy=multi-user.target
>>>
Yann E. MORIN Aug. 27, 2021, 9:28 p.m. UTC | #4 
Sam, All,

On 2021-08-26 16:02 -0500, Sam Voss via buildroot spake thusly:
> Signed-off-by: Sam Voss <sam.voss@collins.com>
> ---
>  package/busybox/busybox.mk      |  7 +++++++
>  package/busybox/telnetd.service | 11 +++++++++++
>  2 files changed, 18 insertions(+)
>  create mode 100644 package/busybox/telnetd.service
> 
> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
> index cfc06b0d78..fb427faf1c 100644
> --- a/package/busybox/busybox.mk
> +++ b/package/busybox/busybox.mk
> @@ -378,6 +378,13 @@ define BUSYBOX_INSTALL_TARGET_CMDS
>  	$(BUSYBOX_INSTALL_MDEV_CONF)
>  endef
>  
> +define BUSYBOX_INSTALL_INIT_SYSTEMD
> +	if grep -q CONFIG_FEATURE_TELNETD_STANDALONE=y $(@D)/.config; then \
> +		$(INSTALL) -D -m 0644 package/busybox/telnetd.service \
> +			$(TARGET_DIR)/usr/lib/systemd/system/telnetd.service ; \
> +	fi
> +endef

I've changed that to be handled in a fashion similar to the two other
init systems:

  - added a macro right after BUSYBOX_INSTALL_TELNET_SCRIPT, and
    similarly named BUSYBOX_INSTALL_TELNET_SERVICE

  - use thatr macro in the newly introduced BUSYBOX_INSTALL_INIT_SYSTEMD

  - move BUSYBOX_INSTALL_INIT_SYSTEMD alphabetically in-between openrc
    and sysv.

Applied to next, thanks.

Regards,
Yann E. MORIN.

>  # Install the sysvinit scripts, for the moment, but not those that already
>  # have a corresponding one in openrc.
>  define BUSYBOX_INSTALL_INIT_OPENRC
> diff --git a/package/busybox/telnetd.service b/package/busybox/telnetd.service
> new file mode 100644
> index 0000000000..3c1fa457b1
> --- /dev/null
> +++ b/package/busybox/telnetd.service
> @@ -0,0 +1,11 @@
> +[Unit]
> +Description=Telnetd Service
> +After=network.target
> +
> +[Service]
> +# busybox telnet doesn't support pidfile, so run in foreground
> +Type=simple
> +ExecStart=/usr/sbin/telnetd -F
> +
> +[Install]
> +WantedBy=multi-user.target
> -- 
> 2.17.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
diff mbox series

Patch

diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index cfc06b0d78..fb427faf1c 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -378,6 +378,13 @@  define BUSYBOX_INSTALL_TARGET_CMDS
 	$(BUSYBOX_INSTALL_MDEV_CONF)
 endef
 
+define BUSYBOX_INSTALL_INIT_SYSTEMD
+	if grep -q CONFIG_FEATURE_TELNETD_STANDALONE=y $(@D)/.config; then \
+		$(INSTALL) -D -m 0644 package/busybox/telnetd.service \
+			$(TARGET_DIR)/usr/lib/systemd/system/telnetd.service ; \
+	fi
+endef
+
 # Install the sysvinit scripts, for the moment, but not those that already
 # have a corresponding one in openrc.
 define BUSYBOX_INSTALL_INIT_OPENRC
diff --git a/package/busybox/telnetd.service b/package/busybox/telnetd.service
new file mode 100644
index 0000000000..3c1fa457b1
--- /dev/null
+++ b/package/busybox/telnetd.service
@@ -0,0 +1,11 @@ 
+[Unit]
+Description=Telnetd Service
+After=network.target
+
+[Service]
+# busybox telnet doesn't support pidfile, so run in foreground
+Type=simple
+ExecStart=/usr/sbin/telnetd -F
+
+[Install]
+WantedBy=multi-user.target