diff mbox

wpa_supplicant: install systemd service files

Message ID 1425411738-3369-1-git-send-email-mike@mikebwilliams.com
State Superseded
Headers show

Commit Message

Mike Williams March 3, 2015, 7:42 p.m. UTC
Signed-off-by: Mike Williams <mike@mikebwilliams.com>
---
 package/wpa_supplicant/wpa_supplicant.mk | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Thomas Petazzoni March 4, 2015, 8:33 p.m. UTC | #1
Dear Mike Williams,

On Tue,  3 Mar 2015 14:42:18 -0500, Mike Williams wrote:

> +define WPA_SUPPLICANT_INSTALL_INIT_SYSTEMD
> +	$(INSTALL) -m 0644 -D $(@D)/$(WPA_SUPPLICANT_SUBDIR)/systemd/wpa_supplicant.service \
> +		$(TARGET_DIR)/lib/systemd/system

When using -D, the destination should be a full path, otherwise your
file will be named $(TARGET_DIR)/lib/systemd/system if
$(TARGET_DIR)/lib/systemd/system isn't an existing directory.

> +	$(INSTALL) -m 0644 -D $(@D)/$(WPA_SUPPLICANT_SUBDIR)/systemd/wpa_supplicant@.service \
> +		$(TARGET_DIR)/lib/systemd/system
> +	$(INSTALL) -m 0644 -D $(@D)/$(WPA_SUPPLICANT_SUBDIR)/systemd/wpa_supplicant-nl80211@.service \
> +		$(TARGET_DIR)/lib/systemd/system
> +	$(INSTALL) -m 0644 -D $(@D)/$(WPA_SUPPLICANT_SUBDIR)/systemd/wpa_supplicant-wired@.service \
> +		$(TARGET_DIR)/lib/systemd/system

Also, and sorry for being so ignorant about systemd, but most of our
other packages set up a bunch of symbolic links when install systemd
unit files. I'm surprised to not see the same thing here. Can you give
some details about this?

Thanks,

Thomas
Mike Williams March 9, 2015, 4:08 p.m. UTC | #2
Thomas,

On Wed, Mar 4, 2015 at 3:33 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Mike Williams,
>
> On Tue,  3 Mar 2015 14:42:18 -0500, Mike Williams wrote:
>
>> +define WPA_SUPPLICANT_INSTALL_INIT_SYSTEMD
>> +     $(INSTALL) -m 0644 -D $(@D)/$(WPA_SUPPLICANT_SUBDIR)/systemd/wpa_supplicant.service \
>> +             $(TARGET_DIR)/lib/systemd/system
>
> When using -D, the destination should be a full path, otherwise your
> file will be named $(TARGET_DIR)/lib/systemd/system if
> $(TARGET_DIR)/lib/systemd/system isn't an existing directory.

Fixed in v2.

>
>> +     $(INSTALL) -m 0644 -D $(@D)/$(WPA_SUPPLICANT_SUBDIR)/systemd/wpa_supplicant@.service \
>> +             $(TARGET_DIR)/lib/systemd/system
>> +     $(INSTALL) -m 0644 -D $(@D)/$(WPA_SUPPLICANT_SUBDIR)/systemd/wpa_supplicant-nl80211@.service \
>> +             $(TARGET_DIR)/lib/systemd/system
>> +     $(INSTALL) -m 0644 -D $(@D)/$(WPA_SUPPLICANT_SUBDIR)/systemd/wpa_supplicant-wired@.service \
>> +             $(TARGET_DIR)/lib/systemd/system
>
> Also, and sorry for being so ignorant about systemd, but most of our
> other packages set up a bunch of symbolic links when install systemd
> unit files. I'm surprised to not see the same thing here. Can you give
> some details about this?

Those symlinks enable starting those services during boot. These
services shouldn't be enabled by default, so no symlinks are
generated.

As an aside, I commented on Gustavo's discussion of our packages
enabling startup by default in this thread:
http://lists.busybox.net/pipermail/buildroot/2015-March/121448.html

>
> Thanks,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
Thomas Petazzoni March 9, 2015, 4:39 p.m. UTC | #3
Dear Mike Williams,

On Mon, 9 Mar 2015 12:08:48 -0400, Mike Williams wrote:

> > Also, and sorry for being so ignorant about systemd, but most of our
> > other packages set up a bunch of symbolic links when install systemd
> > unit files. I'm surprised to not see the same thing here. Can you give
> > some details about this?
> 
> Those symlinks enable starting those services during boot. These
> services shouldn't be enabled by default, so no symlinks are
> generated.

Ok, but right now our policy is that services select in the Buildroot
configuration should be enabled at startup by default.

I know this is not the proposal you made in
http://lists.busybox.net/pipermail/buildroot/2015-March/121448.html,
but the proposal you made on this specific topic is going completely in
the opposite direction of what Buildroot is doing right now.

The reason for the current policy is that we want a simple "out of the
box" experience: you enable lighttpd or minidlna, you build, you boot
the system, and those services start by default, with a sane minimal
configuration. It's kind of the principle of "least surprise" I would
say.

Then if you want some services to have a more complex configuration, or
to not start at boot time, it's up to you to do what's needed in your
post-build script or rootfs overlay.

Best regards,

Thomas
diff mbox

Patch

diff --git a/package/wpa_supplicant/wpa_supplicant.mk b/package/wpa_supplicant/wpa_supplicant.mk
index 682cb4c..185c646 100644
--- a/package/wpa_supplicant/wpa_supplicant.mk
+++ b/package/wpa_supplicant/wpa_supplicant.mk
@@ -164,4 +164,15 @@  define WPA_SUPPLICANT_INSTALL_TARGET_CMDS
 	$(WPA_SUPPLICANT_INSTALL_DBUS)
 endef
 
+define WPA_SUPPLICANT_INSTALL_INIT_SYSTEMD
+	$(INSTALL) -m 0644 -D $(@D)/$(WPA_SUPPLICANT_SUBDIR)/systemd/wpa_supplicant.service \
+		$(TARGET_DIR)/lib/systemd/system
+	$(INSTALL) -m 0644 -D $(@D)/$(WPA_SUPPLICANT_SUBDIR)/systemd/wpa_supplicant@.service \
+		$(TARGET_DIR)/lib/systemd/system
+	$(INSTALL) -m 0644 -D $(@D)/$(WPA_SUPPLICANT_SUBDIR)/systemd/wpa_supplicant-nl80211@.service \
+		$(TARGET_DIR)/lib/systemd/system
+	$(INSTALL) -m 0644 -D $(@D)/$(WPA_SUPPLICANT_SUBDIR)/systemd/wpa_supplicant-wired@.service \
+		$(TARGET_DIR)/lib/systemd/system
+endef
+
 $(eval $(generic-package))