Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v3 1/2] package/runc: add host package
@ 2022-01-27  6:25 Christian Stewart via buildroot
  2022-01-27  6:25 ` [Buildroot] [PATCH v3 2/2] package/buildah: new package Christian Stewart via buildroot
  2022-01-27  7:57 ` [Buildroot] [PATCH v3 1/2] package/runc: add host package Thomas Petazzoni
  0 siblings, 2 replies; 7+ messages in thread
From: Christian Stewart via buildroot @ 2022-01-27  6:25 UTC (permalink / raw)
  To: buildroot; +Cc: Christian Stewart, Thomas Petazzoni

Adds support for building runc as a host package.

The bin name and gomod have to be specified as the pkg-golang infrastructure
assumes the bin name will be "host-runc" on default.

Signed-off-by: Christian Stewart <christian@paral.in>
---
 package/Config.in.host      | 1 +
 package/runc/Config.in.host | 8 ++++++++
 package/runc/runc.mk        | 8 ++++++++
 3 files changed, 17 insertions(+)
 create mode 100644 package/runc/Config.in.host

diff --git a/package/Config.in.host b/package/Config.in.host
index 341e24926b..2ce015eacf 100644
--- a/package/Config.in.host
+++ b/package/Config.in.host
@@ -81,6 +81,7 @@ menu "Host utilities"
 	source "package/raspberrypi-usbboot/Config.in.host"
 	source "package/rauc/Config.in.host"
 	source "package/riscv-isa-sim/Config.in.host"
+	source "package/runc/Config.in.host"
 	source "package/rustc/Config.in.host"
 	source "package/s6-rc/Config.in.host"
 	source "package/sam-ba/Config.in.host"
diff --git a/package/runc/Config.in.host b/package/runc/Config.in.host
new file mode 100644
index 0000000000..ce0dd518a6
--- /dev/null
+++ b/package/runc/Config.in.host
@@ -0,0 +1,8 @@
+config BR2_PACKAGE_HOST_RUNC
+	bool "host runc"
+	depends on BR2_PACKAGE_HOST_GO_HOST_ARCH_SUPPORTS
+	help
+	  runC is a CLI tool for spawning and running containers
+	  according to the OCP specification.
+
+	  https://github.com/opencontainers/runc
diff --git a/package/runc/runc.mk b/package/runc/runc.mk
index c4e45a00a9..932bf39eea 100644
--- a/package/runc/runc.mk
+++ b/package/runc/runc.mk
@@ -10,6 +10,8 @@ RUNC_LICENSE = Apache-2.0
 RUNC_LICENSE_FILES = LICENSE
 RUNC_CPE_ID_VENDOR = linuxfoundation
 
+RUNC_GOMOD = github.com/opencontainers/runc
+
 RUNC_LDFLAGS = -X main.version=$(RUNC_VERSION)
 RUNC_TAGS = cgo static_build
 
@@ -23,4 +25,10 @@ RUNC_TAGS += seccomp
 RUNC_DEPENDENCIES += libseccomp host-pkgconf
 endif
 
+HOST_RUNC_BIN_NAME = runc
+HOST_RUNC_LDFLAGS = $(RUNC_LDFLAGS)
+HOST_RUNC_TAGS = $(RUNC_TAGS)
+HOST_RUNC_INSTALL_BINS = $(HOST_RUNC_BIN_NAME)
+
 $(eval $(golang-package))
+$(eval $(host-golang-package))
-- 
2.35.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH v3 2/2] package/buildah: new package
  2022-01-27  6:25 [Buildroot] [PATCH v3 1/2] package/runc: add host package Christian Stewart via buildroot
@ 2022-01-27  6:25 ` Christian Stewart via buildroot
  2022-01-27  8:01   ` Thomas Petazzoni
  2022-01-27  7:57 ` [Buildroot] [PATCH v3 1/2] package/runc: add host package Thomas Petazzoni
  1 sibling, 1 reply; 7+ messages in thread
From: Christian Stewart via buildroot @ 2022-01-27  6:25 UTC (permalink / raw)
  To: buildroot; +Cc: Christian Stewart, Thomas Petazzoni

Adds both host and target packages for buildah.

Buildah is a tool that facilitates building OCI images.

https://github.com/containers/buildah

The buildah tree does not ship with a default policy.json file, and instead
relies on packagers to provide one. A patch is added to create a basic barebones
policy.json which is installed to /etc/containers/policy.json with a hook.

Signed-off-by: Christian Stewart <christian@paral.in>

---

v1 -> v2:

 - add package to developers
 - add host runc dependency for host package
 - add libgpgme runtime dependency

v2 -> v3:

 - add policy.json to target: required by some commands
 - example: buildah pull docker.io/library/alpine
 - pull: tested on raspberry pi 4

Signed-off-by: Christian Stewart <christian@paral.in>
---
 DEVELOPERS                                    |  1 +
 package/Config.in                             |  1 +
 package/Config.in.host                        |  1 +
 ...01-contrib-add-buildroot-policy-json.patch | 38 +++++++++++++++
 package/buildah/Config.in                     | 23 ++++++++++
 package/buildah/Config.in.host                |  8 ++++
 package/buildah/buildah.hash                  |  3 ++
 package/buildah/buildah.mk                    | 46 +++++++++++++++++++
 8 files changed, 121 insertions(+)
 create mode 100644 package/buildah/0001-contrib-add-buildroot-policy-json.patch
 create mode 100644 package/buildah/Config.in
 create mode 100644 package/buildah/Config.in.host
 create mode 100644 package/buildah/buildah.hash
 create mode 100644 package/buildah/buildah.mk

diff --git a/DEVELOPERS b/DEVELOPERS
index fe8de1916e..80e7c5abee 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -529,6 +529,7 @@ F:	package/python-pylibftdi/
 
 N:	Christian Stewart <christian@paral.in>
 F:	package/batman-adv/
+F:	package/buildah/
 F:	package/containerd/
 F:	package/delve/
 F:	package/docker-cli/
diff --git a/package/Config.in b/package/Config.in
index e4ca195beb..91eb66a454 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -2523,6 +2523,7 @@ menu "System tools"
 	source "package/audit/Config.in"
 	source "package/balena-engine/Config.in"
 	source "package/bubblewrap/Config.in"
+	source "package/buildah/Config.in"
 	source "package/cgroupfs-mount/Config.in"
 	source "package/circus/Config.in"
 	source "package/containerd/Config.in"
diff --git a/package/Config.in.host b/package/Config.in.host
index 2ce015eacf..6aadb1b640 100644
--- a/package/Config.in.host
+++ b/package/Config.in.host
@@ -7,6 +7,7 @@ menu "Host utilities"
 	source "package/babeltrace2/Config.in.host"
 	source "package/bmap-tools/Config.in.host"
 	source "package/btrfs-progs/Config.in.host"
+	source "package/buildah/Config.in.host"
 	source "package/cbootimage/Config.in.host"
 	source "package/checkpolicy/Config.in.host"
 	source "package/checksec/Config.in.host"
diff --git a/package/buildah/0001-contrib-add-buildroot-policy-json.patch b/package/buildah/0001-contrib-add-buildroot-policy-json.patch
new file mode 100644
index 0000000000..7a8ca57a5e
--- /dev/null
+++ b/package/buildah/0001-contrib-add-buildroot-policy-json.patch
@@ -0,0 +1,38 @@
+From 6808cfa788f03fca36a41202d9475ee5bc9feac7 Mon Sep 17 00:00:00 2001
+From: Christian Stewart <christian@paral.in>
+Date: Wed, 26 Jan 2022 22:07:09 -0800
+Subject: [PATCH] contrib: add buildroot policy json
+
+Buildah does not ship a default policy.json in-tree.
+
+Signed-off-by: Christian Stewart <christian@paral.in>
+---
+ contrib/buildroot/policy.json | 16 ++++++++++++++++
+ 1 file changed, 16 insertions(+)
+ create mode 100644 contrib/buildroot/policy.json
+
+diff --git a/contrib/buildroot/policy.json b/contrib/buildroot/policy.json
+new file mode 100644
+index 00000000..d8c638a0
+--- /dev/null
++++ b/contrib/buildroot/policy.json
+@@ -0,0 +1,16 @@
++{
++  "default": [
++    {
++      "type": "insecureAcceptAnything"
++    }
++  ],
++  "transports": {
++    "docker-daemon": {
++      "": [
++        {
++          "type": "insecureAcceptAnything"
++        }
++      ]
++    }
++  }
++}
+-- 
+2.35.0
+
diff --git a/package/buildah/Config.in b/package/buildah/Config.in
new file mode 100644
index 0000000000..05bd0eec31
--- /dev/null
+++ b/package/buildah/Config.in
@@ -0,0 +1,23 @@
+config BR2_PACKAGE_BUILDAH
+	bool "buildah"
+	depends on BR2_PACKAGE_HOST_GO_TARGET_ARCH_SUPPORTS
+	depends on BR2_PACKAGE_HOST_GO_TARGET_CGO_LINKING_SUPPORTS
+	depends on BR2_PACKAGE_LIBGPG_ERROR_ARCH_SUPPORTS # libgpgme -> libgpg-error
+	depends on BR2_TOOLCHAIN_HAS_THREADS # runc
+	depends on BR2_USE_MMU # libgpgme -> libassuan
+	depends on !BR2_TOOLCHAIN_USES_UCLIBC # runc -> no fexecve
+	# gnupg and runc are not needed to build, but at runtime.
+	select BR2_PACKAGE_LIBGPGME
+	select BR2_PACKAGE_GNUPG if !BR2_PACKAGE_GNUPG2
+	select BR2_PACKAGE_LIBGPG_ERROR
+	select BR2_PACKAGE_LIBASSUAN
+	select BR2_PACKAGE_RUNC
+	help
+	  Buildah is a tool that facilitates building OCI images.
+
+	  https://github.com/containers/buildah
+
+comment "buildah 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
+	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_TOOLCHAIN_USES_UCLIBC
diff --git a/package/buildah/Config.in.host b/package/buildah/Config.in.host
new file mode 100644
index 0000000000..67fee6d7ac
--- /dev/null
+++ b/package/buildah/Config.in.host
@@ -0,0 +1,8 @@
+config BR2_PACKAGE_HOST_BUILDAH
+	bool "host buildah"
+	depends on BR2_PACKAGE_HOST_GO_HOST_ARCH_SUPPORTS
+	select BR2_PACKAGE_HOST_RUNC
+	help
+	  Buildah is a tool that facilitates building OCI images.
+
+	  https://github.com/containers/buildah
diff --git a/package/buildah/buildah.hash b/package/buildah/buildah.hash
new file mode 100644
index 0000000000..c7e00d02a7
--- /dev/null
+++ b/package/buildah/buildah.hash
@@ -0,0 +1,3 @@
+# Locally calculated
+sha256  d99b5187a25bc9d7385408732a0e155df0458b4d2cea6e8d002f3fa2cbaac76f  buildah-1.24.0.tar.gz
+sha256  b40930bbcf80744c86c46a12bc9da056641d722716c378f5659b9e555ef833e1  LICENSE
diff --git a/package/buildah/buildah.mk b/package/buildah/buildah.mk
new file mode 100644
index 0000000000..658d7ef56f
--- /dev/null
+++ b/package/buildah/buildah.mk
@@ -0,0 +1,46 @@
+################################################################################
+#
+# buildah
+#
+################################################################################
+
+BUILDAH_VERSION = 1.24.0
+BUILDAH_SITE = $(call github,containers,buildah,v$(BUILDAH_VERSION))
+
+BUILDAH_LICENSE = Apache-2.0
+BUILDAH_LICENSE_FILES = LICENSE
+
+BUILDAH_DEPENDENCIES = libgpgme
+
+BUILDAH_CPE_ID_VENDOR = buildah_project
+BUILDAH_CPE_ID_PRODUCT = buildah
+
+BUILDAH_TAGS = \
+	cgo \
+	exclude_graphdriver_aufs \
+	exclude_graphdriver_btrfs \
+	exclude_graphdriver_devicemapper \
+	exclude_graphdriver_zfs
+BUILDAH_BUILD_TARGETS = cmd/buildah
+BUILDAH_GOMOD = github.com/containers/buildah
+
+BUILDAH_LDFLAGS = \
+	-X $(BUILDAH_GOMOD)/cmd/buildah.GitCommit=v$(BUILDAH_VERSION) \
+	-X $(BUILDAH_GOMOD)/define.Version=v$(BUILDAH_VERSION)
+
+BUILDAH_INSTALL_BINS = $(notdir $(BUILDAH_BUILD_TARGETS))
+
+define BUILDAH_INSTALL_CONFIG
+	$(INSTALL) -D -m 644 $(@D)/contrib/buildroot/policy.json \
+		$(TARGET_DIR)/etc/containers/policy.json
+endef
+
+BUILDAH_POST_INSTALL_TARGET_HOOKS += BUILDAH_INSTALL_CONFIG
+
+HOST_BUILDAH_BUILD_TARGETS = $(BUILDAH_BUILD_TARGETS)
+HOST_BUILDAH_TAGS = $(BUILDAH_TAGS)
+HOST_BUILDAH_LDFLAGS = $(BUILDAH_LDFLAGS)
+HOST_BUILDAH_INSTALL_BINS = $(BUILDAH_INSTALL_BINS)
+
+$(eval $(golang-package))
+$(eval $(host-golang-package))
-- 
2.35.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH v3 1/2] package/runc: add host package
  2022-01-27  6:25 [Buildroot] [PATCH v3 1/2] package/runc: add host package Christian Stewart via buildroot
  2022-01-27  6:25 ` [Buildroot] [PATCH v3 2/2] package/buildah: new package Christian Stewart via buildroot
@ 2022-01-27  7:57 ` Thomas Petazzoni
  1 sibling, 0 replies; 7+ messages in thread
From: Thomas Petazzoni @ 2022-01-27  7:57 UTC (permalink / raw)
  To: Christian Stewart; +Cc: buildroot

Hello,

On Wed, 26 Jan 2022 22:25:03 -0800
Christian Stewart <christian@paral.in> wrote:

> diff --git a/package/runc/runc.mk b/package/runc/runc.mk
> index c4e45a00a9..932bf39eea 100644
> --- a/package/runc/runc.mk
> +++ b/package/runc/runc.mk
> @@ -10,6 +10,8 @@ RUNC_LICENSE = Apache-2.0
>  RUNC_LICENSE_FILES = LICENSE
>  RUNC_CPE_ID_VENDOR = linuxfoundation
>  
> +RUNC_GOMOD = github.com/opencontainers/runc

How is this related to  the addition of the host package? This seems
unrelated. Why is this needed now?

> +HOST_RUNC_BIN_NAME = runc

Why is this not needed for the target package ?

> +HOST_RUNC_LDFLAGS = $(RUNC_LDFLAGS)
> +HOST_RUNC_TAGS = $(RUNC_TAGS)
> +HOST_RUNC_INSTALL_BINS = $(HOST_RUNC_BIN_NAME)

This makes me think we should probably have some inheritance of those
variables between the target variant and the host variant of the
package.

However, HOST_RUNC_TAGS = $(RUNC_TAGS) looks most likely wrong. Indeed:

ifeq ($(BR2_PACKAGE_LIBAPPARMOR),y)
RUNC_DEPENDENCIES += libapparmor
RUNC_TAGS += apparmor
endif

ifeq ($(BR2_PACKAGE_LIBSECCOMP),y)
RUNC_TAGS += seccomp
RUNC_DEPENDENCIES += libseccomp host-pkgconf
endif

and you most likely don't want HOST_RUNC_TAGS to contain apparmor or
seccomp, because we are not building libseccomp or libapparmor for the
host.

Best regards,

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH v3 2/2] package/buildah: new package
  2022-01-27  6:25 ` [Buildroot] [PATCH v3 2/2] package/buildah: new package Christian Stewart via buildroot
@ 2022-01-27  8:01   ` Thomas Petazzoni
  2022-01-27 20:31     ` Christian Stewart via buildroot
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Petazzoni @ 2022-01-27  8:01 UTC (permalink / raw)
  To: Christian Stewart; +Cc: buildroot

Hello,

On Wed, 26 Jan 2022 22:25:04 -0800
Christian Stewart <christian@paral.in> wrote:


> v1 -> v2:
> 
>  - add package to developers
>  - add host runc dependency for host package
>  - add libgpgme runtime dependency

Considering that there are runtime dependency concerns, it would be
nice to have a simple test case in support/testing/.


> diff --git a/package/buildah/buildah.hash b/package/buildah/buildah.hash
> new file mode 100644
> index 0000000000..c7e00d02a7
> --- /dev/null
> +++ b/package/buildah/buildah.hash
> @@ -0,0 +1,3 @@
> +# Locally calculated
> +sha256  d99b5187a25bc9d7385408732a0e155df0458b4d2cea6e8d002f3fa2cbaac76f  buildah-1.24.0.tar.gz
> +sha256  b40930bbcf80744c86c46a12bc9da056641d722716c378f5659b9e555ef833e1  LICENSE
> diff --git a/package/buildah/buildah.mk b/package/buildah/buildah.mk
> new file mode 100644
> index 0000000000..658d7ef56f
> --- /dev/null
> +++ b/package/buildah/buildah.mk
> @@ -0,0 +1,46 @@
> +################################################################################
> +#
> +# buildah
> +#
> +################################################################################
> +
> +BUILDAH_VERSION = 1.24.0
> +BUILDAH_SITE = $(call github,containers,buildah,v$(BUILDAH_VERSION))
> +
> +BUILDAH_LICENSE = Apache-2.0
> +BUILDAH_LICENSE_FILES = LICENSE
> +
> +BUILDAH_DEPENDENCIES = libgpgme

Is libgpgme really a runtime dependency, as noted in your changelog? If
it is, then it's not needed in BUILDAH_DEPENDENCIES. However, it would
be somewhat surprising for it to be only a runtime dependency. Could
you confirm? Does it get dlopen()ed at runtime? Or it's not the library
that is used at runtime, but some program that is installed by libgpgme?


> +define BUILDAH_INSTALL_CONFIG
> +	$(INSTALL) -D -m 644 $(@D)/contrib/buildroot/policy.json \
> +		$(TARGET_DIR)/etc/containers/policy.json
> +endef
> +
> +BUILDAH_POST_INSTALL_TARGET_HOOKS += BUILDAH_INSTALL_CONFIG
> +
> +HOST_BUILDAH_BUILD_TARGETS = $(BUILDAH_BUILD_TARGETS)
> +HOST_BUILDAH_TAGS = $(BUILDAH_TAGS)
> +HOST_BUILDAH_LDFLAGS = $(BUILDAH_LDFLAGS)
> +HOST_BUILDAH_INSTALL_BINS = $(BUILDAH_INSTALL_BINS)

That "repetition" also makes me think we should have some level of
inheritance between target and host variables in the golang-package
infrastructure. For BUILD_TARGETS, TAGS and INSTALL_BINS, it sounds
fine to do it. However, for LDFLAGS, it's a bit weird, as normally,
LDFLAGS are different between host and target. However here, there are
mostly used to pass these version-related -X options, that are in fact
the same between host and target. Should we have a separate variable to
pass those flags ? Not sure.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH v3 2/2] package/buildah: new package
  2022-01-27  8:01   ` Thomas Petazzoni
@ 2022-01-27 20:31     ` Christian Stewart via buildroot
  2022-08-21 15:14       ` Yann E. MORIN
  0 siblings, 1 reply; 7+ messages in thread
From: Christian Stewart via buildroot @ 2022-01-27 20:31 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: Christian Stewart, Buildroot Mailing List

Hi Thomas,


On Thu, Jan 27, 2022 at 12:01 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
> Considering that there are runtime dependency concerns, it would be
> nice to have a simple test case in support/testing/.

OK, something like "buildah pull" or so?

> > +BUILDAH_DEPENDENCIES = libgpgme
>
> Is libgpgme really a runtime dependency, as noted in your changelog? If

Gpg is a runtime dependency, libgpgme is a build dependency.


> > +HOST_BUILDAH_BUILD_TARGETS = $(BUILDAH_BUILD_TARGETS)
> > +HOST_BUILDAH_TAGS = $(BUILDAH_TAGS)
> > +HOST_BUILDAH_LDFLAGS = $(BUILDAH_LDFLAGS)
> > +HOST_BUILDAH_INSTALL_BINS = $(BUILDAH_INSTALL_BINS)
>
> That "repetition" also makes me think we should have some level of
> inheritance between target and host variables in the golang-package
> infrastructure. For BUILD_TARGETS, TAGS and INSTALL_BINS, it sounds
> fine to do it.

Yes - unless the package overrides them - this should be fine.

I was a bit surprised that it, on default, tries to build the binary
named "host-runc" as well as a Go package named "host-runc". Of
course, this is not the name of neither the binary nor the package.

So the host-golang infra should remove the host- prefix for the
default BUILD_TARGETS and INSTALL_BINS.

This would also be fixed if those values were inherited from the target package.

> However, for LDFLAGS, it's a bit weird, as normally,
> LDFLAGS are different between host and target. However here, there are
> mostly used to pass these version-related -X options, that are in fact
> the same between host and target. Should we have a separate variable to
> pass those flags ? Not sure.

They are quite common for Go packages - could call them DEFINES or
something similar.

The syntax overrides the value of a global variable in a package.

Best regards,
Christian
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH v3 2/2] package/buildah: new package
  2022-01-27 20:31     ` Christian Stewart via buildroot
@ 2022-08-21 15:14       ` Yann E. MORIN
  2022-08-21 17:05         ` Christian Stewart via buildroot
  0 siblings, 1 reply; 7+ messages in thread
From: Yann E. MORIN @ 2022-08-21 15:14 UTC (permalink / raw)
  To: Christian Stewart; +Cc: Thomas Petazzoni, Buildroot Mailing List

Christian, All,

Sorry for the late review...

(I've also quoted snippets from your commit log)

On 2022-01-26 12:25 -0800, Christian Stewart via buildroot spake thusly:
> Adds both host and target packages for buildah.

I would question the need for having buildah on the target. Buildah is
advertised as: "A tool that facilitates building OCI container images."
I don't see a good reason for building images *on* the target. Please
explain in the commit log why that would make sense to have buildah on
the target.

> The buildah tree does not ship with a default policy.json file, and instead
> relies on packagers to provide one. A patch is added to create a basic barebones
> policy.json which is installed to /etc/containers/policy.json with a hook.

Don't add a patch; just add that as a file in the Buildroot tree, e.g.
package/buildah/policy.json, and install that from the package dir, i.e
$(BUILDAH_PKGDIR)/policy.json rather than from $(@D)/..../policy.json.


On 2022-01-27 12:31 -0800, Christian Stewart via buildroot spake thusly:
> On Thu, Jan 27, 2022 at 12:01 AM Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
> > Considering that there are runtime dependency concerns, it would be
> > nice to have a simple test case in support/testing/.
> OK, something like "buildah pull" or so?

A runtime test should validate that the package works as expected, in a
minimal way. So, if 'buildah pull' can prove that runtime dependencies
are exercised, then that's OK, yes.

Obviously if runtiem dependencies are only required for specific cases
and only "loaded" when needed, we will not notice any missing ones. But
having an exhaustive test is not very important either; we do not want
to have to run the full project's test-suite...

[--SNIP--]
> I was a bit surprised that it, on default, tries to build the binary
> named "host-runc" as well as a Go package named "host-runc". Of
> course, this is not the name of neither the binary nor the package.
> So the host-golang infra should remove the host- prefix for the
> default BUILD_TARGETS and INSTALL_BINS.

We now have 8539378771f7 (package/pkg-golang: default to rawname to
install binaries) which is supopsed to fix that.

> This would also be fixed if those values were inherited from the target package.

I am very wary of inheriting target values for host packages. Back in
the day, we had such an inheritance in the autotools infra, but we
eventually got rid of it because it was causing too much issue, see
4bdb067e380e (infra: remove auto derivation of host dependencies).

The only variables that should be automatically inherited are those that
actualy identify the package, like the site, the version, the license,
etc.. and not the ones that /define/ it (configure and build options and
so on...)

Here, TAGS and BUILD_TARGETS are typically not something that should be
automatically inherited, because they define what is built, and the host
and target packages should not by default have the same feature set (we
tend to only have conditionals for the target one, and enable everything
in the host variant, for example).

The GOMOD you provided suspiciously looks like what the infra would have
computed already, no?

> > However, for LDFLAGS, it's a bit weird, as normally,
> > LDFLAGS are different between host and target. However here, there are
> > mostly used to pass these version-related -X options, that are in fact
> > the same between host and target. Should we have a separate variable to
> > pass those flags ? Not sure.
> They are quite common for Go packages - could call them DEFINES or
> something similar.

But with LDFLAGS, it is possible to pass flags to the actual linker,
with -extldflags something-something, so they should not be
automatically inherited.

And if we had something the DEFINES you hint at, they would again be
different by default, because the host and target builds do not follow
the same logic.

I've marked this patch, the oldest in patchwork, as changes-requested.

Regards,
Yann E. MORIN.

> The syntax overrides the value of a global variable in a package.
> 
> Best regards,
> Christian
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH v3 2/2] package/buildah: new package
  2022-08-21 15:14       ` Yann E. MORIN
@ 2022-08-21 17:05         ` Christian Stewart via buildroot
  0 siblings, 0 replies; 7+ messages in thread
From: Christian Stewart via buildroot @ 2022-08-21 17:05 UTC (permalink / raw)
  To: Yann E. MORIN; +Cc: Christian Stewart, Thomas Petazzoni, Buildroot Mailing List


[-- Attachment #1.1: Type: text/plain, Size: 3218 bytes --]

Yann,

On Sun, Aug 21, 2022, 8:14 AM Yann E. MORIN <yann.morin.1998@free.fr> wrote:

> On 2022-01-26 12:25 -0800, Christian Stewart via buildroot spake thusly:
> > Adds both host and target packages for buildah.
>
> I would question the need for having buildah on the target. Buildah is
> advertised as: "A tool that facilitates building OCI container images."
> I don't see a good reason for building images *on* the target. Please
> explain in the commit log why that would make sense to have buildah on
> the target.
>

Building container images with a tool like "docker build" on the target is
an extremely common practice. There are numerous reasons to do so. Almost
none of the container images I use on devices were built ahead of time,
rather on the target device. I don't see why it's necessary to explain that
on every commit message.

> The buildah tree does not ship with a default policy.json file, and
> instead
> > relies on packagers to provide one. A patch is added to create a basic
> barebones
> > policy.json which is installed to /etc/containers/policy.json with a
> hook.
>
> Don't add a patch; just add that as a file in the Buildroot tree, e.g.
> package/buildah/policy.json, and install that from the package dir, i.e
> $(BUILDAH_PKGDIR)/policy.json rather than from $(@D)/..../policy.json.
>

Did a similar thing to this with the upcoming podman package. Will update.

On 2022-01-27 12:31 -0800, Christian Stewart via buildroot spake thusly:
> > On Thu, Jan 27, 2022 at 12:01 AM Thomas Petazzoni
> > <thomas.petazzoni@bootlin.com> wrote:
> > > Considering that there are runtime dependency concerns, it would be
> > > nice to have a simple test case in support/testing/.
> > OK, something like "buildah pull" or so?
>
> A runtime test should validate that the package works as expected, in a
> minimal way. So, if 'buildah pull' can prove that runtime dependencies
> are exercised, then that's OK, yes.
>
> Obviously if runtiem dependencies are only required for specific cases
> and only "loaded" when needed, we will not notice any missing ones. But
> having an exhaustive test is not very important either; we do not want
> to have to run the full project's test-suite...
>

I would argue that in this case if the package compiles at all it will
work, as per the upstream tests. Not sure if it's really worth the time to
add the extra test here.

> > However, for LDFLAGS, it's a bit weird, as normally,
> > > LDFLAGS are different between host and target. However here, there are
> > > mostly used to pass these version-related -X options, that are in fact
> > > the same between host and target. Should we have a separate variable to
> > > pass those flags ? Not sure.
> > They are quite common for Go packages - could call them DEFINES or
> > something similar.
>
> But with LDFLAGS, it is possible to pass flags to the actual linker,
> with -extldflags something-something, so they should not be
> automatically inherited.
>
> And if we had something the DEFINES you hint at, they would again be
> different by default, because the host and target builds do not follow
> the same logic.
>

We are defining the Version here, which is the same on the host and target.

Thanks,
Christian

[-- Attachment #1.2: Type: text/html, Size: 4693 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-08-21 17:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-27  6:25 [Buildroot] [PATCH v3 1/2] package/runc: add host package Christian Stewart via buildroot
2022-01-27  6:25 ` [Buildroot] [PATCH v3 2/2] package/buildah: new package Christian Stewart via buildroot
2022-01-27  8:01   ` Thomas Petazzoni
2022-01-27 20:31     ` Christian Stewart via buildroot
2022-08-21 15:14       ` Yann E. MORIN
2022-08-21 17:05         ` Christian Stewart via buildroot
2022-01-27  7:57 ` [Buildroot] [PATCH v3 1/2] package/runc: add host package Thomas Petazzoni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox