package/dropbear: Respect user specific configurations
PATCH 1/2 package/dropbear: Respect user specific configurations
systemd .service file should respect /etc/default/dropbear
--- package/dropbear/dropbear.service | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
package/dropbear/dropbear.service +2 -1
@@ -19,7 +19,8 @@ if [ -L /etc/dropbear \ mkdir -p "$(readlink /etc/dropbear)"; \ fi; \ fi' -ExecStart=/usr/sbin/dropbear -F -R +EnvironmentFile=/etc/default/dropbear +ExecStart=/usr/sbin/dropbear -F -R $DROPBEAR_ARGS ExecReload=/bin/kill -HUP $MAINPID [Install]
Signature · 1 line
2.6.2
PATCH 1/2 package/dropbear: Respect user specific configurations
On Thu, 29 Oct 2015 17:11:41 +1100
Cyril Bur <[email protected]> wrote:
Apologies, missing sign off, don't try to rush patches out!
Quoted 1 line
systemd .service file should respect
/etc/default/dropbear
Quoted 18 lines
--- package/dropbear/dropbear.service | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)package/dropbear/dropbear.service +2
-1@@ -19,7 +19,8 @@ if [ -L /etc/dropbear \ mkdir -p "$(readlink /etc/dropbear)"; \ fi; \ fi' -ExecStart=/usr/sbin/dropbear -F -R +EnvironmentFile=/etc/default/dropbear +ExecStart=/usr/sbin/dropbear -F -R $DROPBEAR_ARGS ExecReload=/bin/kill -HUP $MAINPID [Install]
PATCH 2/2 package/systemd: Respect BR2_TARGET_GENERIC_GETTY_BAUDRATE
In reply to Cyril BurCurrently systemd getty services ignore baudrates set in buildroot in favour of a hardcoded 115200. This patch SEDs out that hardcoded value with what is selected.
--- package/systemd/systemd.mk | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
package/systemd/systemd.mk +7 -2
@@ -183,6 +183,7 @@ endef ifneq ($(call qstrip,$(BR2_TARGET_GENERIC_GETTY_PORT)),) # systemd needs getty.service for VTs and serial-getty.service for serial ttys +# also patch the file to use the correct baud-rate, the default baudrate is 115200 so look for that define SYSTEMD_INSTALL_SERVICE_TTY if echo $(BR2_TARGET_GENERIC_GETTY_PORT) | egrep -q 'tty[0-9]*$$'; \ then \
@@ -190,8 +191,12 @@ define SYSTEMD_INSTALL_SERVICE_TTY else \ SERVICE="serial-getty"; \ fi; \ - ln -fs ../../../../lib/systemd/system/$${SERVICE}@.service \ - $(TARGET_DIR)/etc/systemd/system/getty.target.wants/$${SERVICE}@$(BR2_TARGET_GENERIC_GETTY_PORT).service + ln -fs $(TARGET_DIR)/lib/systemd/system/$${SERVICE}@.service \ + $(TARGET_DIR)/etc/systemd/system/getty.target.wants/$${SERVICE}@$(BR2_TARGET_GENERIC_GETTY_PORT).service; \ + if [ $(BR2_TARGET_GENERIC_GETTY_BAUDRATE) -gt 0 ] ; \ + then \ + $(SED) 's,115200,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE),' $(TARGET_DIR)/lib/systemd/system/$${SERVICE}@.service; \ + fi endef endif
Signature · 1 line
2.6.2
PATCH 2/2 package/systemd: Respect BR2_TARGET_GENERIC_GETTY_BAUDRATE
Quoted 3 lines · On Thu, 29 Oct 2015 17:11:42 +1100 Cyril Bur <[email protected]> wrote:
Currently systemd getty services ignore baudrates set in buildroot in favour of a hardcoded 115200. This patch SEDs out that hardcoded value with what is selected.
Quoted 31 lines
--- package/systemd/systemd.mk | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)package/systemd/systemd.mk +7
-2@@ -183,6 +183,7 @@ endef ifneq ($(call qstrip,$(BR2_TARGET_GENERIC_GETTY_PORT)),) # systemd needs getty.service for VTs and serial-getty.service for serial ttys +# also patch the file to use the correct baud-rate, the default baudrate is 115200 so look for that define SYSTEMD_INSTALL_SERVICE_TTY if echo $(BR2_TARGET_GENERIC_GETTY_PORT) | egrep -q 'tty[0-9]*$$'; \ then \@@ -190,8 +191,12 @@ define SYSTEMD_INSTALL_SERVICE_TTY else \ SERVICE="serial-getty"; \ fi; \ - ln -fs ../../../../lib/systemd/system/$${SERVICE}@.service \ - $(TARGET_DIR)/etc/systemd/system/getty.target.wants/$${SERVICE}@$(BR2_TARGET_GENERIC_GETTY_PORT).service + ln -fs $(TARGET_DIR)/lib/systemd/system/$${SERVICE}@.service \ + $(TARGET_DIR)/etc/systemd/system/getty.target.wants/$${SERVICE}@$(BR2_TARGET_GENERIC_GETTY_PORT).service; \ + if [ $(BR2_TARGET_GENERIC_GETTY_BAUDRATE) -gt 0 ] ; \ + then \ + $(SED) 's,115200,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE),' $(TARGET_DIR)/lib/systemd/system/$${SERVICE}@.service; \ + fi endef endif
PATCH 2/2 package/systemd: Respect BR2_TARGET_GENERIC_GETTY_BAUDRATE
Cyril, All,
Some comment inline below
Quoted 19 lines · Cyril Bur
Quoted 3 lines · On Thu, 29 Oct 2015 17:11:42 +1100 Cyril Bur <[email protected]> wrote:
Currently systemd getty services ignore baudrates set in buildroot in favour of a hardcoded 115200. This patch SEDs out that hardcoded value with what is selected.
Signed-off-by Cyril Bur <[email protected]>Quoted 10 lines
--- package/systemd/systemd.mk | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)package/systemd/systemd.mk +0
-0@@ -183,6 +183,7 @@ endef
Quoted 14 lines
Quoted 7 lines
package/systemd/systemd.mk +1
-0ifneq ($(call qstrip,$(BR2_TARGET_GENERIC_GETTY_PORT)),) # systemd needs getty.service for VTs and serial-getty.service for serial ttys +# also patch the file to use the correct baud-rate, the default baudrate is 115200 so look for that define SYSTEMD_INSTALL_SERVICE_TTY if echo $(BR2_TARGET_GENERIC_GETTY_PORT) | egrep -q 'tty[0-9]*$$'; \ then \package/systemd/systemd.mk define SYSTEMD_INSTALL_SERVICE_TTY · line 195
Quoted 7 lines
package/systemd/systemd.mk +2
-2@@ -190,8 +191,12 @@ define SYSTEMD_INSTALL_SERVICE_TTY else \ SERVICE="serial-getty"; \ fi; \ - ln -fs ../../../../lib/systemd/system/$${SERVICE}@.service \ - $(TARGET_DIR)/etc/systemd/system/getty.target.wants/$${SERVICE}@$(BR2_TARGET_GENERIC_GETTY_PORT).service + ln -fs $(TARGET_DIR)/lib/systemd/system/$${SERVICE}@.service \ + $(TARGET_DIR)/etc/systemd/system/getty.target.wants/$${SERVICE}@$(BR2_TARGET_GENERIC_GETTY_PORT).service; \
Changing the symlink above is not necessary for this patch. Also i think the the change is wrong and will cause the service to symlink an invalid file when run on the target.
Quoted 4 lines
package/systemd/systemd.mk define SYSTEMD_INSTALL_SERVICE_TTY · line 199
Quoted 4 lines
package/systemd/systemd.mk +4
-0+ if [ $(BR2_TARGET_GENERIC_GETTY_BAUDRATE) -gt 0 ] ; \ + then \ + $(SED) 's,115200,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE),' $(TARGET_DIR)/lib/systemd/system/$${SERVICE}@.service; \ + fi
$(BR2_TARGET_GENERIC_GETTY_BAUDRATE) will have double quotes around it which will need to be removed so i don't think the -gt test will ever work. Have a look in package/skeleton/skeleton.mk where it uses $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE)) to strip the double quotes before it uses the value, you'll need to do something similar.
Thanks
Martin
Quoted 8 lines
package/systemd/systemd.mk define SYSTEMD_INSTALL_SERVICE_TTY · line 202
Quoted 3 lines
package/systemd/systemd.mk +0
-0endef endif_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
PATCH 2/2 package/systemd: Respect BR2_TARGET_GENERIC_GETTY_BAUDRATE
Hi Cyril, Martin, all
Quoted 42 lines · Martin Bark
Cyril, All,
Some comment inline below
Quoted 5 lines · Cyril Bur
Quoted 2 lines · On Thu, 29 Oct 2015 17:11:42 +1100 Cyril Bur <[email protected]> wrote:
Currently systemd getty services ignore baudrates set in buildroot in favour of a hardcoded 115200. This patch SEDs out that hardcoded value
with
Quoted 8 lines
Quoted 1 line
what is selected.
Signed-off-by Cyril Bur <[email protected]>Quoted 4 lines
--- package/systemd/systemd.mk | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)Quoted 8 lines
package/systemd/systemd.mk endef · line 185
Quoted 8 lines
package/systemd/systemd.mk +0
-0@@ -183,6 +183,7 @@ endef ifneq ($(call qstrip,$(BR2_TARGET_GENERIC_GETTY_PORT)),) # systemd needs getty.service for VTs and serial-getty.service forserial ttys
Quoted 1 line
package/systemd/systemd.mk endef · line 186
Quoted 1 line
package/systemd/systemd.mk +1
-0+# also patch the file to use the correct baud-rate, the defaultbaudrate is 115200 so look for that
Quoted 2 lines
package/systemd/systemd.mk endef · line 188
Quoted 2 lines
package/systemd/systemd.mk +0
-0define SYSTEMD_INSTALL_SERVICE_TTY if echo $(BR2_TARGET_GENERIC_GETTY_PORT) | egrep -q'tty[0-9]*$$'; \
Quoted 7 lines
package/systemd/systemd.mk define SYSTEMD_INSTALL_SERVICE_TTY · line 194
Quoted 7 lines
package/systemd/systemd.mk +0
-2then \@@ -190,8 +191,12 @@ define SYSTEMD_INSTALL_SERVICE_TTY else \ SERVICE="serial-getty"; \ fi; \ - ln -fs ../../../../lib/systemd/system/$${SERVICE}@.service \ -$(TARGET_DIR)/etc/systemd/system/getty.target.wants/$${SERVICE}@
$(BR2_TARGET_GENERIC_GETTY_PORT).service
Quoted 9 lines
Quoted 2 lines
package/systemd/systemd.mk define SYSTEMD_INSTALL_SERVICE_TTY · line 195
Quoted 2 lines
package/systemd/systemd.mk +2
-0+ ln -fs $(TARGET_DIR)/lib/systemd/system/$${SERVICE}@.service \ +$(TARGET_DIR)/etc/systemd/system/getty.target.wants/$${SERVICE}@$(BR2_TARGET_GENERIC_GETTY_PORT).service;
\Changing the symlink above is not necessary for this patch. Also i think the the change is wrong and will cause the service to symlink an invalid file when run on the target.
Indeed, it's wrong, but not because it'll be wrong on the target itself, but in the target directory. The relative path allow you to use the symlink even in your target directory (otherwise you my stumble upon the files on your hosts and that would start misunderstanding and errors.).
Quoted 22 lines
Quoted 4 lines
package/systemd/systemd.mk define SYSTEMD_INSTALL_SERVICE_TTY
Quoted 4 lines
package/systemd/systemd.mk +3
-0+ if [ $(BR2_TARGET_GENERIC_GETTY_BAUDRATE) -gt 0 ] ; \ + then \ + $(SED) 's,115200,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE),'$(TARGET_DIR)/lib/systemd/system/$${SERVICE}@.service; \
Quoted 1 line
package/systemd/systemd.mk define SYSTEMD_INSTALL_SERVICE_TTY · line 199
Quoted 1 line
package/systemd/systemd.mk +1
-0+ fi$(BR2_TARGET_GENERIC_GETTY_BAUDRATE) will have double quotes around it which will need to be removed so i don't think the -gt test will ever work. Have a look in
package/skeleton/skeleton.mkwhere it uses $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE)) to strip the double quotes before it uses the value, you'll need to do something similar.Thanks
Martin
Quoted 4 lines
Quoted 3 lines
package/systemd/systemd.mk +0
-0endef endif
Quoted 9 lines
Quoted 4 lines
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151104/43f883a2/attachment.html>
PATCH 2/2 package/systemd: Respect BR2_TARGET_GENERIC_GETTY_BAUDRATE
Quoted 51 lines · On Wed, 4 Nov 2015 10:22:51 +0100 Maxime Hadjinlian…
Hi Cyril, Martin, all
Quoted 47 lines · Martin Bark
Cyril, All,
Some comment inline below
Quoted 5 lines · Cyril Bur
Quoted 2 lines · On Thu, 29 Oct 2015 17:11:42 +1100 Cyril Bur <[email protected]> wrote:
Currently systemd getty services ignore baudrates set in buildroot in favour of a hardcoded 115200. This patch SEDs out that hardcoded value
with
Quoted 8 lines
Quoted 1 line
what is selected.
Signed-off-by Cyril Bur <[email protected]>Quoted 4 lines
--- package/systemd/systemd.mk | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)Quoted 8 lines
package/systemd/systemd.mk endef · line 185
Quoted 8 lines
package/systemd/systemd.mk +0
-0@@ -183,6 +183,7 @@ endef ifneq ($(call qstrip,$(BR2_TARGET_GENERIC_GETTY_PORT)),) # systemd needs getty.service for VTs and serial-getty.service forserial ttys
Quoted 1 line
package/systemd/systemd.mk endef · line 186
Quoted 1 line
package/systemd/systemd.mk +1
-0+# also patch the file to use the correct baud-rate, the defaultbaudrate is 115200 so look for that
Quoted 2 lines
package/systemd/systemd.mk endef · line 188
Quoted 2 lines
package/systemd/systemd.mk +0
-0define SYSTEMD_INSTALL_SERVICE_TTY if echo $(BR2_TARGET_GENERIC_GETTY_PORT) | egrep -q'tty[0-9]*$$'; \
Quoted 7 lines
package/systemd/systemd.mk define SYSTEMD_INSTALL_SERVICE_TTY · line 194
Quoted 7 lines
package/systemd/systemd.mk +0
-2then \@@ -190,8 +191,12 @@ define SYSTEMD_INSTALL_SERVICE_TTY else \ SERVICE="serial-getty"; \ fi; \ - ln -fs ../../../../lib/systemd/system/$${SERVICE}@.service \ -$(TARGET_DIR)/etc/systemd/system/getty.target.wants/$${SERVICE}@
$(BR2_TARGET_GENERIC_GETTY_PORT).serviceQuoted 2 lines
package/systemd/systemd.mk define SYSTEMD_INSTALL_SERVICE_TTY · line 195
Quoted 2 lines
package/systemd/systemd.mk +2
-0+ ln -fs $(TARGET_DIR)/lib/systemd/system/$${SERVICE}@.service \ +$(TARGET_DIR)/etc/systemd/system/getty.target.wants/$${SERVICE}@$(BR2_TARGET_GENERIC_GETTY_PORT).service;
\
Quoted 9 lines
Quoted 4 lines
Changing the symlink above is not necessary for this patch. Also i think the the change is wrong and will cause the service to symlink an invalid file when run on the target.
Indeed, it's wrong, but not because it'll be wrong on the target itself, but in the target directory. The relative path allow you to use the symlink even in your target directory (otherwise you my stumble upon the files on your hosts and that would start misunderstanding and errors.).
Ah yes, I'm not sure why I did this anymore, I do agree, I'll resend without this hunk.
Quoted 7 lines
Quoted 7 lines
Quoted 4 lines
package/systemd/systemd.mk define SYSTEMD_INSTALL_SERVICE_TTY
Quoted 4 lines
package/systemd/systemd.mk +3
-0+ if [ $(BR2_TARGET_GENERIC_GETTY_BAUDRATE) -gt 0 ] ; \ + then \ + $(SED) 's,115200,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE),'$(TARGET_DIR)/lib/systemd/system/$${SERVICE}@.service; \
Quoted 1 line
Quoted 1 line
package/systemd/systemd.mk +1
-0+ fi
Quoted 7 lines
Quoted 7 lines
$(BR2_TARGET_GENERIC_GETTY_BAUDRATE) will have double quotes around it which will need to be removed so i don't think the -gt test will ever work. Have a look in
package/skeleton/skeleton.mkwhere it uses $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE)) to strip the double quotes before it uses the value, you'll need to do something similar.
I believe it will work, Nicolas Cavallari provided a good explanation as to why:
This test will work just fine, it will be expanded to e.g.
if [ "38400" -gt 0 ];
Which is a perfectly valid shell condition. However, the sed will introduce the double quotes in the systemd unit file. Which, according to the systemd documentation, is also fine in an ExecStart statement, which somewhat mimic the behavior of the shell.
Quoted 17 lines
Quoted 17 lines
Thanks
Martin
Quoted 8 lines
package/systemd/systemd.mk define SYSTEMD_INSTALL_SERVICE_TTY · line 202
Quoted 3 lines
package/systemd/systemd.mk +0
-0endef endif_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
PATCH 2/2 package/systemd: Respect BR2_TARGET_GENERIC_GETTY_BAUDRATE
Quoted 14 lines · Cyril Bur
Quoted 11 lines · On Wed, 4 Nov 2015 10:22:51 +0100 Maxime Hadjinlian…
Hi Cyril, Martin, all
Quoted 7 lines · Martin Bark
Cyril, All,
Some comment inline below
Quoted 2 lines · Cyril Bur
On Thu, 29 Oct 2015 17:11:42 +1100
Cyril Bur <[email protected]> wrote:
[snip]
Quoted 17 lines
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
package/systemd/systemd.mk define SYSTEMD_INSTALL_SERVICE_TTY
Quoted 3 lines
package/systemd/systemd.mk +3
-0+ if [ $(BR2_TARGET_GENERIC_GETTY_BAUDRATE) -gt 0 ] ; \ + then \ + $(SED) 's,115200,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE),'$(TARGET_DIR)/lib/systemd/system/$${SERVICE}@.service; \
Quoted 1 line
Quoted 1 line
+ fi
Quoted 7 lines
Quoted 7 lines
$(BR2_TARGET_GENERIC_GETTY_BAUDRATE) will have double quotes around it which will need to be removed so i don't think the -gt test will ever work. Have a look in
package/skeleton/skeleton.mkwhere it uses $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE)) to strip the double quotes before it uses the value, you'll need to do something similar.I believe it will work, Nicolas Cavallari provided a good explanation as to why:
Quoted 8 lines
This test will work just fine, it will be expanded to e.g.
if [ "38400" -gt 0 ];
Which is a perfectly valid shell condition. However, the sed will introduce the double quotes in the systemd unit file. Which, according to the systemd documentation, is also fine in an ExecStart statement, which somewhat mimic the behavior of the shell.
Even though it works without the qstrip, I would prefer to use qstrip after all
to make things consistent.
Ideally, by the way, I would like the getty handling to be done together for
systemd and old init. But unfortunately, moving it to skeleton is not going to work since this patch requires systemd to be installed and systemd requires skeleton to be installed. So we'll have to keep it split up like this until we find a better solution.
Regards,
Arnout
[snip]
Signature · 6 lines
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: 7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF
PATCH 2/2 package/systemd: Respect BR2_TARGET_GENERIC_GETTY_BAUDRATE
In reply to Martin BarkQuoted 11 lines · Martin Bark
Quoted 4 lines
package/systemd/systemd.mk define SYSTEMD_INSTALL_SERVICE_TTY · line 199
Quoted 4 lines
package/systemd/systemd.mk +4
-0+ if [ $(BR2_TARGET_GENERIC_GETTY_BAUDRATE) -gt 0 ] ; \ + then \ + $(SED) 's,115200,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE),' $(TARGET_DIR)/lib/systemd/system/$${SERVICE}@.service; \ + fi$(BR2_TARGET_GENERIC_GETTY_BAUDRATE) will have double quotes around it which will need to be removed so i don't think the -gt test will ever work. Have a look in
package/skeleton/skeleton.mkwhere it uses $(call qstrip,$(BR2_TARGET_GENERIC_GETTY_BAUDRATE)) to strip the double quotes before it uses the value, you'll need to do something similar.
This test will work just fine, it will be expanded to e.g.
if [ "38400" -gt 0 ];
Which is a perfectly valid shell condition. However, the sed will introduce the double quotes in the systemd unit file. Which, according to the systemd documentation, is also fine in an ExecStart statement, which somewhat mimic the behavior of the shell.
PATCH 2/2 package/systemd: Respect BR2_TARGET_GENERIC_GETTY_BAUDRATE
In reply to Cyril BurDear Cyril Bur,
Quoted 6 lines · Cyril Bur
Currently systemd getty services ignore baudrates set in buildroot in favour of a hardcoded 115200. This patch SEDs out that hardcoded value with what is selected.
--- package/systemd/systemd.mk | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
Both Martin Bark and Maxime Hadjinlian raised issues about your patches. Therefore, I have for now marked your patch as "Rejected" in our patch tracking system. However, if you don't agree with what Martin and Maxime said, do not hesitate to reply to them, find a solution that works fine for everyone and post an updated patch.
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 2/2 package/systemd: Respect BR2_TARGET_GENERIC_GETTY_BAUDRATE
Quoted 10 lines · On Thu, 5 Nov 2015 13:32:18 +0100 Thomas Petazzoni…
Dear Cyril Bur,
Quoted 6 lines · Cyril Bur
Currently systemd getty services ignore baudrates set in buildroot in favour of a hardcoded 115200. This patch SEDs out that hardcoded value with what is selected.
--- package/systemd/systemd.mk | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
Quoted 6 lines
Both Martin Bark and Maxime Hadjinlian raised issues about your patches. Therefore, I have for now marked your patch as "Rejected" in our patch tracking system. However, if you don't agree with what Martin and Maxime said, do not hesitate to reply to them, find a solution that works fine for everyone and post an updated patch.
Hi Thomas,
Thanks for taking the time to respond, these emails got lost somewhere, somehow. Apologies!
The patches did work for me but one specific case means nothing. I'll incorporate feedback and send you an updated version.
Cyril
Quoted 3 lines
Thanks!
Thomas
Thomas Petazzoni
AppliedPATCH 1/2 package/dropbear: Respect user specific configurations
In reply to Cyril BurDear Cyril Bur,
Quoted 4 lines · Cyril Bur
systemd .service file should respect
/etc/default/dropbear--- package/dropbear/dropbear.service | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Thanks, applied.
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 1/2 package/dropbear: Respect user specific configurations
Hi Cyril, Thomas,
This patch results in dropbear being broken out-of-the-box for systemd users. The EnvironmentFile= line sets a requirement that /etc/default/dropbear exist (it doesn't, by default).
I'm not so sure having an environment file by default is a good practice for systemd units. The general consensus is that things effecting the execution of a service should always be found in the appropriate service unit. Environment= would be better anyway since there's only one variable ($DROPBEAR_ARGS) being used in this particular case.
An even better solution would be to move the dropbear.service symlink from /etc/systemd/system/multi-user.target.wants/ to /usr/lib/systemd/system/multi-user.target.wants/. That would allow users to shadow the unit completely with their own customizations or override individual options through the use of "drop-in" units. The manual explains this in more detail:
http://www.freedesktop.org/software/systemd/man/systemd.unit.html#id-1.10.3
Thanks,
Gabe
Quoted 20 lines · Thomas Petazzoni
Dear Cyril Bur,
Quoted 4 lines · Cyril Bur
systemd .service file should respect
/etc/default/dropbear--- package/dropbear/dropbear.service | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)Thanks, applied.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 5 lines
Gabe Evans | Co-Founder & CTO hashrabbit.co ? angel.co/hashrabbit ? github.com/gevans -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151103/c94dba14/attachment.html>
PATCH 1/2 package/dropbear: Respect user specific configurations
Dear Gabe Evans,
Quoted 9 lines · Gabe Evans
This patch results in dropbear being broken out-of-the-box for systemd users. The EnvironmentFile= line sets a requirement that
/etc/default/dropbearexist (it doesn't, by default).I'm not so sure having an environment file by default is a good practice for systemd units. The general consensus is that things effecting the execution of a service should always be found in the appropriate service unit. Environment= would be better anyway since there's only one variable ($DROPBEAR_ARGS) being used in this particular case.
Right. I'll revert the patch from Cyril then, unless he suggests a better solution. I really don't know much about systemd, so I can only rely on people reviewing patches touching systemd topics. If no-one does, all I can do is a guess that the change looks somewhat reasonable.
So your report is definitely useful. But if I may suggest, it would be even more useful if you could review systemd related patches before they are applied.
Quoted 7 lines
An even better solution would be to move the dropbear.service symlink from
/etc/systemd/system/multi-user.target.wants/to/usr/lib/systemd/system/multi-user.target.wants/. That would allow users to shadow the unit completely with their own customizations or override individual options through the use of "drop-in" units. The manual explains this in more detail:
http://www.freedesktop.org/software/systemd/man/systemd.unit.html#id-1.10.3
*All* our packages are creating the symlink
in /etc/systemd/system/multi-user.target.wants/, so changing that really needs to be discussed with the other people interested in systemd support in Buildroot. I believe we used to have some symlinks in /usr, some in /etc, and we settled on /etc with some ${justification}.
Maxime ?
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 1/2 package/dropbear: Respect user specific configurations
Hi Thomas,
Quoted 3 lines · Thomas Petazzoni
So your report is definitely useful. But if I may suggest, it would be even more useful if you could review systemd related patches before they are applied.
This one slipped past me but I'd be more than happy to review patches in the future.
Quoted 8 lines
Quoted 7 lines
An even better solution would be to move the dropbear.service symlink from
/etc/systemd/system/multi-user.target.wants/to/usr/lib/systemd/system/multi-user.target.wants/. That would allow users to shadow the unit completely with their own customizations or override individual options through the use of "drop-in" units. The manual explains this in more detail:
http://www.freedesktop.org/software/systemd/man/systemd.unit.html#id-1.10.3
Quoted 6 lines
*All* our packages are creating the symlink
in/etc/systemd/system/multi-user.target.wants/, so changing that really needs to be discussed with the other people interested in systemd support in Buildroot. I believe we used to have some symlinks in /usr, some in /etc, and we settled on /etc with some ${justification}.
It might also be worth clarifying that other Linux distributions follow the practice of reserving /etc/systemd for users and /usr/lib/systemd for maintainers.
This is definitely a big change that should be discussed in more detail. I'm still getting familiar with the mailing list so your help in starting this conversation would be appreciated. :)
Thanks,
Gabe
Signature · 2 lines
Gabe Evans | Co-Founder & CTO hashrabbit.co ? angel.co/hashrabbit ? github.com/gevans
PATCH 1/2 package/dropbear: Respect user specific configurations
Hi Gabe, Thomas, all,
Quoted 30 lines · Gabe Evans
Hi Thomas,
Quoted 3 lines · Thomas Petazzoni
So your report is definitely useful. But if I may suggest, it would be even more useful if you could review systemd related patches before they are applied.
This one slipped past me but I'd be more than happy to review patches in the future.
Quoted 1 line
Quoted 1 line
An even better solution would be to move the dropbear.service symlink
from
Quoted 2 lines
Quoted 2 lines
/etc/systemd/system/multi-user.target.wants/to/usr/lib/systemd/system/multi-user.target.wants/. That would allowusers to
Quoted 2 lines
Quoted 2 lines
shadow the unit completely with their own customizations or override individual options through the use of "drop-in" units. The manual
explains
Quoted 2 lines
Quoted 2 lines
this in more detail:
http://www.freedesktop.org/software/systemd/man/systemd.unit.html#id-1.10.3
Quoted 7 lines
*All* our packages are creating the symlink
in/etc/systemd/system/multi-user.target.wants/, so changing that really needs to be discussed with the other people interested in systemd support in Buildroot. I believe we used to have some symlinks in /usr, some in /etc, and we settled on /etc with some ${justification}.
Quoted 4 lines
It might also be worth clarifying that other Linux distributions follow the practice of reserving
/etc/systemdfor users and/usr/lib/systemdfor maintainers.
We organized the files the way they are after what Debian did. They install everything in '/usr/lib/systemd/system' and simply create symlinks in '/etc/systemd/system/....', the way systemctl enable/disable does it (with regards of the targets). It's also easy to test/tweak or the target without loosing the original file.
Regarding the user replacing the unit files, we strongly suggest that people that wants to customize anything to their needs, use a post-build/post-image scripts.
We don't really provide an image as a 'vendor' would do, that you would customize after. You build your own image, so you can customize it the way you want it to at build time.
And if you want to customize only a little, the EnvironmentFile is there to do that (that's the way I see it at least).
The simpler Environment is tricker to use in Buildroot, to customize the env for a specific process, you would have to tweak systemd's configuration file so it would evaluate a file containing all the env variable you would use. What happens if there's collisions in the name ? Environment is good on a distro, if you quickly want to hack something.
'EnvironmentFile' is the way to provide users with a way to customize the process (or using the templates system, but that would requires more effort to maintain, again since the file is used by both init, it's pretty straightforward).
But since you don't want a process to stop functioning because it's lacking options (since they are optionnal, it must work without them), using the '=-' mechanisms is kind of mandatory for us I would say.
So I kind of like the way things are right now but if you see something that doesn't sit right with you, let's discuss it :)
Quoted 5 lines
This is definitely a big change that should be discussed in more detail. I'm still getting familiar with the mailing list so your help in starting this conversation would be appreciated. :)
Quoted 7 lines
Thanks,
Gabe--
Gabe Evans | Co-Founder & CTO
hashrabbit.co ? angel.co/hashrabbit ? github.com/gevans
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151104/77edc9f6/attachment.html>
PATCH 1/2 package/dropbear: Respect user specific configurations
Hi Maxime, Thomas, all,
Quoted 6 lines · Maxime Hadjinlian
We organized the files the way they are after what Debian did. They install everything in '
/usr/lib/systemd/system' and simply create symlinks in '/etc/systemd/system/....', the way systemctl enable/disable does it (with regards of the targets). It's also easy to test/tweak or the target without loosing the original file.
Quoted 6 lines
Regarding the user replacing the unit files, we strongly suggest that people that wants to customize anything to their needs, use a post-build/post-image scripts.
We don't really provide an image as a 'vendor' would do, that you would customize after. You build your own image, so you can customize it the way you want it to at build time.
My reasoning for linking services to targets in '/usr/lib/systemd/system' is more for endusers than it is for developers using Buildroot. Also, being able to link to '../something.service' instead of '../../../../usr/lib/systemd/system/something.service' leaves less room for error. In the long run it offers more flexibility to users who may not have the access or ability to make modifications to the build system. Even though the project isn't much of a 'vendor' the developers using it are in most cases.
Quoted 9 lines
And if you want to customize only a little, the EnvironmentFile is there to do that (that's the way I see it at least).
The simpler Environment is tricker to use in Buildroot, to customize the env for a specific process, you would have to tweak systemd's configuration file so it would evaluate a file containing all the env variable you would use. What happens if there's collisions in the name ?
Environment is good on a distro, if you quickly want to hack something.
Quoted 7 lines
'EnvironmentFile' is the way to provide users with a way to customize the process (or using the templates system, but that would requires more effort to maintain, again since the file is used by both init, it's pretty straightforward).
But since you don't want a process to stop functioning because it's lacking options (since they are optionnal, it must work without them), using the '=-' mechanisms is kind of mandatory for us I would say.
I completely agree with you here. My original suggestion of using 'Environment=' doesn't work as effectively as 'EnvironmentFile=-' if the only way to change a unit is to modify or replace the original file.
Quoted 2 lines
So I kind of like the way things are right now but if you see something that doesn't sit right with you, let's discuss it :)
My only other concern would be making sure other packages follow the pattern of having an optional environment file in a common location and use common arguments. In this particular case, additional arguments are specified as '$DROPBEAR_ARGS' meanwhile, the other proposed patch for rng-tools names this variable '$DAEMON_ARGS'.
That aside, I'm okay with this solution given your reasoning. :)
Thanks,
Gabe
Signature · 2 lines
Gabe Evans | Co-Founder & CTO hashrabbit.co ? angel.co/hashrabbit ? github.com/gevans
PATCH 1/2 package/dropbear: Respect user specific configurations
In reply to Thomas PetazzoniQuoted 15 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Thomas" == Thomas Petazzoni <[email protected]> writes:
Dear Gabe Evans,
Quoted 9 lines · Gabe Evans
This patch results in dropbear being broken out-of-the-box for systemd users. The EnvironmentFile= line sets a requirement that
/etc/default/dropbearexist (it doesn't, by default).I'm not so sure having an environment file by default is a good practice for systemd units. The general consensus is that things effecting the execution of a service should always be found in the appropriate service unit. Environment= would be better anyway since there's only one variable ($DROPBEAR_ARGS) being used in this particular case.
Quoted 4 lines
Right. I'll revert the patch from Cyril then, unless he suggests a better solution. I really don't know much about systemd, so I can only rely on people reviewing patches touching systemd topics. If no-one does, all I can do is a guess that the change looks somewhat reasonable.
I have instead changed it to use =- as suggested by Maxime and Steven and fixed a similar issue with out dhcp package.
Signature · 1 line
Bye, Peter Korsgaard
PATCH 1/2 package/dropbear: Respect user specific configurations
In reply to Gabe EvansQuoted 5 lines · Gabe Evans
Hi Cyril, Thomas,
This patch results in dropbear being broken out-of-the-box for systemd users. The EnvironmentFile= line sets a requirement that
/etc/default/dropbearexist (it doesn't, by default).
Make it "EnvironmentFile=-/some/path" (note the dash) and it will ignore cases where the environment file is not present.
Quoted 51 lines
I'm not so sure having an environment file by default is a good practice for systemd units. The general consensus is that things effecting the execution of a service should always be found in the appropriate service unit. Environment= would be better anyway since there's only one variable ($DROPBEAR_ARGS) being used in this particular case.
An even better solution would be to move the dropbear.service symlink from
/etc/systemd/system/multi-user.target.wants/to/usr/lib/systemd/system/multi-user.target.wants/. That would allow users to shadow the unit completely with their own customizations or override individual options through the use of "drop-in" units. The manual explains this in more detail:
http://www.freedesktop.org/software/systemd/man/systemd.unit.html#id-1.10.3Thanks,
GabeQuoted 11 lines · Thomas Petazzoni
Dear Cyril Bur,
Quoted 4 lines · Cyril Bur
systemd .service file should respect
/etc/default/dropbear--- package/dropbear/dropbear.service | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)Thanks, applied.
Quoted 9 lines
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot--
Gabe Evans | Co-Founder & CTO
hashrabbit.co ? angel.co/hashrabbit ? github.com/gevans_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot