Re: [Buildroot] [PATCH v1 1/1] package/cni-plugins: new package

From: Thomas Petazzoni via buildroot <buildroot@buildroot.org>
To: Christian Stewart via buildroot <buildroot@buildroot.org>
Cc: "Yann E . MORIN" <yann.morin.1998@free.fr>
Subject: Re: [Buildroot] [PATCH v1 1/1] package/cni-plugins: new package
Date: Mon, 31 Oct 2022 11:44:44 +0100	[thread overview]
Message-ID: <20221031114444.283e4776@windsurf> (raw)
In-Reply-To: <20220821053547.667311-1-christian@paral.in>

Hello Christian,

On Sat, 20 Aug 2022 22:35:47 -0700
Christian Stewart via buildroot <buildroot@buildroot.org> wrote:

> diff --git a/DEVELOPERS b/DEVELOPERS
> index d2bd0d809a..bc02190365 100644
> --- a/DEVELOPERS
> +++ b/DEVELOPERS
> @@ -530,6 +530,7 @@ F:	package/python-pylibftdi/
>  N:	Christian Stewart <christian@paral.in>
>  F:	package/batman-adv/
>  F:	package/containerd/
> +F:	package/cni-plugins/

By alphabetic ordering I believe it falls before containerd, so I fixed
that up.

> diff --git a/package/cni-plugins/Config.in b/package/cni-plugins/Config.in
> new file mode 100644
> index 0000000000..e01cadc026
> --- /dev/null
> +++ b/package/cni-plugins/Config.in
> @@ -0,0 +1,16 @@
> +config BR2_PACKAGE_CNI_PLUGINS
> +	bool "cni-plugins"
> +	depends on BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS
> +	depends on BR2_PACKAGE_HOST_GO_TARGET_CGO_LINKING_SUPPORTS
> +	depends on BR2_TOOLCHAIN_HAS_THREADS
> +	depends on !BR2_TOOLCHAIN_USES_UCLIBC # no fexecve
> +	depends on BR2_USE_MMU
> +	help
> +	  Container Networking Interface plugins.

But they are plugins for what? They don't have any dependency on
anything specific.

> +	  https://github.com/containernetworking/plugins
> +
> +comment "cni-plugins needs a glibc or musl toolchain w/ threads"
> +	depends on BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS && \
> +		BR2_PACKAGE_HOST_GO_TARGET_CGO_LINKING_SUPPORTS

You forgot BR2_USE_MMU here. I forgot it as well initially, so I fixed
it up as part of a follow-up commit.

> +define CNI_PLUGINS_INSTALL_TARGET_CMDS
> +	$(INSTALL) -d -m 0755 $(TARGET_DIR)/opt/cni/bin
> +	$(foreach d,$(CNI_PLUGINS_INSTALL_BINS),\
> +		$(INSTALL) -D -m 0755 $(@D)/bin/$$(basename $(d)) \
> +			$(TARGET_DIR)/opt/cni/bin

I kept it like this, but it's fairly unusual to install stuff in /opt.
Is this the normal location for these plugins? Shouldn't they go in
/usr/libexec/cni/, or something like that?

So overall, I applied, but there are some questions :)

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