[Buildroot] [PATCH v3 2/2] package/buildah: new package

[Yann E. MORIN]

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 at 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 at 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.  |
'------------------------------^-------^------------------^--------------------'