| Message ID | 1416775163-20215-14-git-send-email-thomas.petazzoni@free-electrons.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Thomas, On Sun, Nov 23, 2014 at 09:38:45PM +0100, Thomas Petazzoni wrote: > +define SAMBA_INSTALL_INIT_SYSV > + # install start/stop script > + @if [ ! -f $(TARGET_DIR)/etc/init.d/S91smb ]; then \ > + $(INSTALL) -m 0755 -D package/samba/S91smb $(TARGET_DIR)/etc/init.d/S91smb; \ > + fi > +endef Why not install the init script unconditionally like you did in vsftpd? baruch
Hi Thomas, On Mon, Nov 24, 2014 at 06:49:22AM +0200, Baruch Siach wrote: > On Sun, Nov 23, 2014 at 09:38:45PM +0100, Thomas Petazzoni wrote: > > +define SAMBA_INSTALL_INIT_SYSV > > + # install start/stop script > > + @if [ ! -f $(TARGET_DIR)/etc/init.d/S91smb ]; then \ > > + $(INSTALL) -m 0755 -D package/samba/S91smb $(TARGET_DIR)/etc/init.d/S91smb; \ > > + fi > > +endef > > Why not install the init script unconditionally like you did in vsftpd? The same comment goes for the input-event-daemon, netatalk, and gpsd patches in this series. baruch
Dear Baruch Siach, On Mon, 24 Nov 2014 06:49:22 +0200, Baruch Siach wrote: > Hi Thomas, > > On Sun, Nov 23, 2014 at 09:38:45PM +0100, Thomas Petazzoni wrote: > > +define SAMBA_INSTALL_INIT_SYSV > > + # install start/stop script > > + @if [ ! -f $(TARGET_DIR)/etc/init.d/S91smb ]; then \ > > + $(INSTALL) -m 0755 -D package/samba/S91smb $(TARGET_DIR)/etc/init.d/S91smb; \ > > + fi > > +endef > > Why not install the init script unconditionally like you did in vsftpd? Well, when I saw how many packages were still installing init script (and other files) conditionally, I kind of changed my mind mid-way through the patch series, and therefore for the remaining patches decided to keep things as they were: keep the condition when it was there, and keep the direct installation with no condition when it was done this way. We certainly need a decision about whether we want those conditions everywhere. Their role is to potentially allow the users to provide a custom version of those scripts (or configuration files) in the filesystem skeleton. I personally believe it's not a good idea, for two reasons: * People should use a filesystem overlay and/or post-build script instead of a custom skeleton. * Why would we install our own stuff conditionally, while the build system of all packages install things unconditionally, making it anyway impossible for a target skeleton to customize all files. Peter, your call? Thomas
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: > Dear Baruch Siach, > On Mon, 24 Nov 2014 06:49:22 +0200, Baruch Siach wrote: >> Hi Thomas, >> >> On Sun, Nov 23, 2014 at 09:38:45PM +0100, Thomas Petazzoni wrote: >> > +define SAMBA_INSTALL_INIT_SYSV >> > + # install start/stop script >> > + @if [ ! -f $(TARGET_DIR)/etc/init.d/S91smb ]; then \ >> > + $(INSTALL) -m 0755 -D package/samba/S91smb $(TARGET_DIR)/etc/init.d/S91smb; \ >> > + fi >> > +endef >> >> Why not install the init script unconditionally like you did in vsftpd? > Well, when I saw how many packages were still installing init script > (and other files) conditionally, I kind of changed my mind mid-way > through the patch series, and therefore for the remaining patches > decided to keep things as they were: keep the condition when it was > there, and keep the direct installation with no condition when it was > done this way. > We certainly need a decision about whether we want those conditions > everywhere. Their role is to potentially allow the users to provide a > custom version of those scripts (or configuration files) in the > filesystem skeleton. I personally believe it's not a good idea, for two > reasons: > * People should use a filesystem overlay and/or post-build script > instead of a custom skeleton. > * Why would we install our own stuff conditionally, while the > build system of all packages install things unconditionally, making > it anyway impossible for a target skeleton to customize all files. > Peter, your call? Yes, I agree - Lets get rid of these legacy checks. Using a custom rootfs overlay / post-build script is a lot more flexible and robust.
diff --git a/package/samba/samba.mk b/package/samba/samba.mk index 1957997..13d2cb7 100644 --- a/package/samba/samba.mk +++ b/package/samba/samba.mk @@ -160,17 +160,20 @@ ifeq ($(BR2_PACKAGE_SAMBA_SWAT),y) SAMBA_POST_INSTALL_TARGET_HOOKS += SAMBA_REMOVE_SWAT_DOCUMENTATION endif -define SAMBA_INSTALL_INITSCRIPTS_CONFIG - # install start/stop script - @if [ ! -f $(TARGET_DIR)/etc/init.d/S91smb ]; then \ - $(INSTALL) -m 0755 -D package/samba/S91smb $(TARGET_DIR)/etc/init.d/S91smb; \ - fi +define SAMBA_INSTALL_CONFIG # install config @if [ ! -f $(TARGET_DIR)/etc/samba/smb.conf ]; then \ $(INSTALL) -m 0644 -D package/samba/simple.conf $(TARGET_DIR)/etc/samba/smb.conf; \ fi endef -SAMBA_POST_INSTALL_TARGET_HOOKS += SAMBA_INSTALL_INITSCRIPTS_CONFIG +SAMBA_POST_INSTALL_TARGET_HOOKS += SAMBA_INSTALL_CONFIG + +define SAMBA_INSTALL_INIT_SYSV + # install start/stop script + @if [ ! -f $(TARGET_DIR)/etc/init.d/S91smb ]; then \ + $(INSTALL) -m 0755 -D package/samba/S91smb $(TARGET_DIR)/etc/init.d/S91smb; \ + fi +endef $(eval $(autotools-package))
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- package/samba/samba.mk | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)