Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buildpack API inconsistency around naming/data-type of targets.distros.version field in buildpack.toml #401

Closed
edmorley opened this issue Apr 8, 2024 · 0 comments · Fixed by #402 or #403

Comments

@edmorley
Copy link
Contributor

edmorley commented Apr 8, 2024

In Buildpack API 0.10, the concept of stacks was replaced by targets:

Part of the targets feature is the concept of distro names/versions.

For example, the spec currently gives an example buildpack.toml as:

[[targets]]
os = "<OS name>"
arch = "<architecture>"
variant = "<architecture variant>"
[[targets.distros]]
name = "<OS distribution name>"
version = "<OS distribution version>"

(from https://github.com/buildpacks/spec/blob/87dc65b9611f88349cead9676e22657349072c90/buildpack.md#buildpacktoml-toml)

And also gives a similar example here:

Tools reading `buildpack.toml` will translate any section that sets `stacks.id = "io.buildpacks.stacks.bionic"` to:

```toml
[[targets]]
os = "linux"
arch = "amd64"
[[targets.distros]]
name = "ubuntu"
version = "18.04"
```

(from https://github.com/buildpacks/spec/blob/87dc65b9611f88349cead9676e22657349072c90/buildpack.md#buildpacktoml-toml-stacks-array)

However, elsewhere in the spec, the version field has a different name, versions:

Each target in `targets`:
- MUST identify a compatible runtime environment:
   - ...
   - `distros` if provided MUST describe the OS distributions supported by the buildpack
     - For Linux-based images, `distros.name` and `distros.versions` SHOULD contain the values specified in `/etc/os-release` (`$ID` and `$VERSION_ID`), as the `os.version` field in an image config may contain combined distribution and version information
     - For Windows-based images, `distros.name` SHOULD be empty; `distros.versions` SHOULD contain the value of `os.version` in the image config (e.g., `10.0.14393.1066`)

(from https://github.com/buildpacks/spec/blob/87dc65b9611f88349cead9676e22657349072c90/buildpack.md#targets-1)

It seems the original RFC used the name versions with data-type array:
https://github.com/buildpacks/rfcs/blob/ce8991b3d739f763b8356ae8696ae582f18c4af6/text/0096-remove-stacks-mixins.md#base-image-metadata

...but that the field was renamed/changed to a string in this commit when the changes were added to the RFC:
8652ec5

And version is also what's used in the implementation:
https://github.com/buildpacks/lifecycle/blob/cd50eb8848492eac9b58c56d060d7d47ac56c693/buildpack/bp_descriptor.go#L39-L43

As such, I think the version usages in the spec are what was intended, and we should standardise everything on that.

I'll open PRs against the spec. (And I'll also backport the changes to the RFC as part of the existing PR I have open for that: buildpacks/rfcs#310)

cc @natalieparellano

edmorley added a commit to edmorley/spec that referenced this issue Apr 8, 2024
The correct name for the field is `distros.version` (singular), rather than
`distros.versions`.

The former is what is used elsewhere in the spec, and in the lifecycle
implementation.

The plural form looks like a leftover from the rename in:
buildpacks@8652ec5

Fixes buildpacks#401.
edmorley added a commit to edmorley/spec that referenced this issue Apr 8, 2024
The correct name for the field is `distros.version` (singular), rather than
`distros.versions`.

The former is what is used elsewhere in the spec, and in the lifecycle
implementation.

The plural form looks like a leftover from the rename in:
buildpacks@8652ec5

Fixes buildpacks#401.

Signed-off-by: Ed Morley <501702+edmorley@users.noreply.github.com>
edmorley added a commit to edmorley/spec that referenced this issue Apr 11, 2024
The correct name for the field is `distros.version` (singular), rather than
`distros.versions`.

The former is what is used elsewhere in the spec, and in the lifecycle
implementation.

The plural form looks like a leftover from the rename in:
buildpacks@8652ec5

Fixes buildpacks#401.
Backport of buildpacks#402.

Signed-off-by: Ed Morley <501702+edmorley@users.noreply.github.com>
edmorley added a commit to edmorley/spec that referenced this issue Apr 11, 2024
The correct name for the field is `distros.version` (singular), rather than
`distros.versions`.

The former is what is used elsewhere in the spec, and in the lifecycle
implementation.

The plural form looks like a leftover from the rename in:
buildpacks@8652ec5

Fixes buildpacks#401.
Backport of buildpacks#402.

Signed-off-by: Ed Morley <501702+edmorley@users.noreply.github.com>
edmorley added a commit to edmorley/spec that referenced this issue Apr 11, 2024
The correct name for the field is `distros.version` (singular), rather than
`distros.versions`.

The former is what is used elsewhere in the spec, and in the lifecycle
implementation.

The plural form looks like a leftover from the rename in:
buildpacks@8652ec5

Fixes buildpacks#401.
Backport of buildpacks#402.

Signed-off-by: Ed Morley <501702+edmorley@users.noreply.github.com>
edmorley added a commit to edmorley/spec that referenced this issue Apr 11, 2024
The correct name for the field is `distros.version` (singular), rather than
`distros.versions`.

The former is what is used elsewhere in the spec, and in the lifecycle
implementation.

The plural form looks like a leftover from the rename in:
buildpacks@8652ec5

Fixes buildpacks#401.
Backport of buildpacks#402.

Signed-off-by: Ed Morley <501702+edmorley@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant