Re: [Buildroot] [PATCH v2 1/1] package/open-isns: new package

[Thomas Petazzoni <thomas.petazzoni@bootlin.com>]

Hello,

On Tue, 13 Sep 2022 00:00:08 -0700
TIAN Yuanhao <tianyuanhao3@163.com> wrote:

> Signed-off-by: TIAN Yuanhao <tianyuanhao3@163.com>

I have applied... but in fact applied the v1 because I'm seeing only
now that there was a v2. Anyway, not a big deal, because...

> - LGPL-2.1+

I had fixed that up.

> - Shorter install commands

And I dropped all the install commands. It's really not great to have
to re-implement all these install commands manually in the .mk file,
while the meson-based build system of open-isns already does all the
work nicely for us. In addition, the programs were really small, so it
didn't make a lot of sense to have an option to be able to disable them.

> +if BR2_PACKAGE_OPEN_ISNS
> +
> +config BR2_PACKAGE_OPEN_ISNS_PROGS
> +	bool "open-isns programs"
> +	help
> +	  This option tells open-isns to not only install the libraries,
> +	  but also the programs.
> +
> +endif

So, I dropped this option.

> diff --git a/package/open-isns/open-isns.hash b/package/open-isns/open-isns.hash
> new file mode 100644
> index 0000000000..653fabaa9f
> --- /dev/null
> +++ b/package/open-isns/open-isns.hash
> @@ -0,0 +1,3 @@
> +# Locally calculated
> +sha256  9611344733c0cdf14395f60880950ea4c3c7d6b765565b6493ad3e1afbe216de  open-isns-0.102.tar.gz
> +sha256  00a89b0d18aacd4114decf79122db87bf35bddaf2bc50e383c9c9f4c263390b2  COPYING
> diff --git a/package/open-isns/open-isns.mk b/package/open-isns/open-isns.mk
> new file mode 100644
> index 0000000000..3db1271d4e
> --- /dev/null
> +++ b/package/open-isns/open-isns.mk
> @@ -0,0 +1,59 @@
> +################################################################################
> +#
> +# open-isns
> +#
> +################################################################################
> +
> +OPEN_ISNS_VERSION = 0.102
> +OPEN_ISNS_SITE = $(call github,open-iscsi,open-isns,v$(OPEN_ISNS_VERSION))
> +OPEN_ISNS_LICENSE = LGPL-2.1+

Indeed adjusted the license to LGPL-2.1+ like you did in your v2.

> +OPEN_ISNS_LICENSE_FILES = COPYING
> +OPEN_ISNS_INSTALL_STAGING = YES
> +
> +OPEN_ISNS_CONF_OPTS = -Dslp=disabled
> +
> +ifeq ($(BR2_PACKAGE_OPENSSL),y)
> +OPEN_ISNS_DEPENDENCIES += openssl
> +OPEN_ISNS_CONF_OPTS += -Dsecurity=enabled
> +else
> +OPEN_ISNS_CONF_OPTS += -Dsecurity=disabled
> +endif
> +
> +define OPEN_ISNS_INSTALL_STAGING_CMDS
> +	$(INSTALL) -d -m 755 $(STAGING_DIR)/usr/{include/libisns,lib}
> +	$(INSTALL) -m 644 -t $(STAGING_DIR)/usr/include/libisns \
> +		$(@D)/{,build/}include/libisns/*.h
> +	cp -dpf $(if $(BR2_STATIC_LIBS),$(@D)/build/libisns.a,$(@D)/build/libisns.so{,.0}) \
> +		$(STAGING_DIR)/usr/lib/
> +	$(INSTALL) -D -m 644 {$(@D),$(STAGING_DIR)/usr/lib/pkgconfig}/libisns.pc
> +endef
> +
> +define OPEN_ISNS_INSTALL_TARGET_CMDS
> +	$(OPEN_ISNS_INSTALL_LIBS)
> +	$(OPEN_ISNS_INSTALL_PROGS)
> +endef
> +
> +ifeq ($(BR2_STATIC_LIBS),)
> +define OPEN_ISNS_INSTALL_LIBS
> +	$(INSTALL) -d -m 755 $(TARGET_DIR)/usr/lib
> +	cp -dpf $(@D)/build/libisns.so{,.0} $(TARGET_DIR)/usr/lib/
> +endef
> +endif
> +
> +ifeq ($(BR2_PACKAGE_OPEN_ISNS_PROGS),y)
> +define OPEN_ISNS_INSTALL_PROGS
> +	$(INSTALL) -d -m 755 $(TARGET_DIR)/{etc/isns,usr/sbin}
> +	$(INSTALL) -d -m 700 $(TARGET_DIR)/var/lib/isns
> +	$(INSTALL) -m 555 -t $(TARGET_DIR)/usr/sbin \
> +		$(@D)/build/{isnsadm,isnsd,isnsdd}
> +	$(INSTALL) -m 644 -t $(TARGET_DIR)/etc/isns $(@D)/etc/*.conf
> +endef
> +
> +define OPEN_ISNS_INSTALL_INIT_SYSTEMD
> +	$(INSTALL) -d -m 755 $(TARGET_DIR)/usr/lib/systemd/system
> +	$(INSTALL) -m 644 -t $(TARGET_DIR)/usr/lib/systemd/system \
> +		$(@D)/isnsd.{service,socket}
> +endef
> +endif

Dropped all of that, and replaced it with just the removal of
isnsd.{service,socket} when systemd is not enabled. This would be a
potentially interesting contribution to the upstream project: have an
option to enable/disable the installation of the systemd unit files.

See the final commit:

  https://gitlab.com/buildroot.org/buildroot/-/commit/4052bad5adc47b9fe964548e7608e9b784acaf28

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot