Skip to content

Replaces priority#1166

Merged
xnox merged 4 commits intochainguard-dev:mainfrom
xnox:replaces-priority
May 28, 2024
Merged

Replaces priority#1166
xnox merged 4 commits intochainguard-dev:mainfrom
xnox:replaces-priority

Conversation

@xnox
Copy link
Member

@xnox xnox commented Apr 23, 2024 •
edited
Loading

This makes things awesome for python.

Melange Pull Request Template

Functional Changes

  • This change can build all of Wolfi without errors (describe results in notes)

Notes:

SCA Changes

  • Examining several representative APKs show no regression / the desired effect (details in notes)

Notes:

Linter

  • The new check is clean across Wolfi
  • The new check is opt-in or a warning

Notes:

@xnox xnox marked this pull request as draft April 23, 2024 23:56
@xnox xnox force-pushed the replaces-priority branch from 750afdd to 4d4ec1b Compare April 24, 2024 14:42
@xnox
Add ReplacesPriroity like ProviderPriority, and allow substitutions
2f5a7d1 
This enables one to use variables, and range substitutions in the
ReplacesPriority and ProviderPriority.

Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@chainguard.dev>
@xnox xnox force-pushed the replaces-priority branch from 4d4ec1b to 2f5a7d1 Compare April 26, 2024 22:21
@xnox xnox marked this pull request as ready for review April 26, 2024 23:05
xnox added a commit to xnox/os that referenced this pull request Apr 26, 2024
@xnox
Drop pybins and allow to install them all
27b22d6 
Requires chainguard-dev/melange#1166

Drop pybins, instead build all packages with commands in /usr/bin.
Set replaces, replaces-priority to ensure they are apk installable.
Also set provider-priority such that `apk add py3-pip` transparently
installs py3.12-pip alone.

This is all great, however not at all supported by apko. But imho
should be supported. As one simply has to sort packages in replaces
priority order prior to streaming. And allow overriding files between
packages that have replaces declared.

```console
...
(13/25) Installing python-3.10-base (3.10.14-r1)
(14/25) Installing py3.10-flit-core (3.9.0-r2)
(15/25) Installing py3.10-setuptools (69.5.1-r1)
(16/25) Installing py3.10-pip (24.0-r2)
(17/25) Installing python-3.11-base (3.11.9-r1)
(18/25) Installing py3.11-flit-core (3.9.0-r2)
(19/25) Installing py3.11-setuptools (69.5.1-r1)
(20/25) Installing py3.11-pip (24.0-r2)
(21/25) Installing python-3.12-base (3.12.3-r1)
(22/25) Installing py3.12-flit-core (3.9.0-r2)
(23/25) Installing py3.12-setuptools (69.5.1-r1)
(24/25) Installing py3.12-pip (24.0-r2)
(25/25) Installing py3-supported-pip (24.0-r2)
OK: 197 MiB in 40 packages

-rwxrwxr-x    1 root     root           946 Apr 26 22:22 /usr/bin/pip3.12
-rwxrwxr-x    1 root     root           946 Apr 26 22:22 /usr/bin/pip3.11
-rwxrwxr-x    1 root     root           946 Apr 26 22:22 /usr/bin/pip3.10
-rwxrwxr-x    1 root     root           940 Apr 26 22:22 /usr/bin/pip3
-rwxrwxr-x    1 root     root           938 Apr 26 22:22 /usr/bin/pip
```
smoser
smoser reviewed Apr 27, 2024
View reviewed changes
@xnox
schmea: validate priority integer strings, and update schema comment
31a4c30 
Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@chainguard.dev>
@xnox xnox requested a review from smoser April 29, 2024 11:59
@kaniini
Copy link
Contributor

kaniini commented May 15, 2024

with my apk-tools maintainer hat on, this looks fine

smoser
smoser requested changes May 16, 2024
View reviewed changes
Copy link
Contributor

@smoser smoser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some doc on range would be nice. I can't seem to find anything in doc/ about range at all. But I understand you didn't add this.

It looks like it would be pretty easy to add test coverage to this in pkg/config/config_test.go . Could you add some coverage?

@xnox xnox added the blocked indicates there are blocking issues that need to be addressed before progress can be made label May 21, 2024
@xnox
Copy link
Member Author

xnox commented May 21, 2024

@kaniini thank you for your comments. "replaces_priority" is only available in the installed database as q, but it is not available in the APKINDEX, meaning looking at APKINDEX alone one cannot know how to order things ahead of time. Would that be just an oversight? as "provider_priority" is available under k in both installled database and apkindex.

Or is it optional and it wouldn't hurt anything to add it in the APKINDEX?

xnox
xnox commented May 23, 2024
View reviewed changes
Copy link
Member Author

@xnox xnox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need this.

@xnox
Copy link
Member Author

xnox commented May 24, 2024

Some doc on range would be nice. I can't seem to find anything in doc/ about range at all. But I understand you didn't add this.

There is https://github.com/chainguard-dev/melange/blob/main/NEWS.md#major-changes-from-010-to-020 and also schema (but it just says that range is an acceptable key, without explaining which arguments it supports)

It looks like it would be pretty easy to add test coverage to this in pkg/config/config_test.go . Could you add some coverage?

Will check.

@xnox
tests: add range priority replacement tests
5e4d519 
Signed-off-by: Dimitri John Ledkov <dimitri.ledkov@chainguard.dev>
@xnox xnox requested a review from smoser May 24, 2024 10:35
@xnox xnox removed the blocked indicates there are blocking issues that need to be addressed before progress can be made label May 24, 2024
@xnox
Copy link
Member Author

xnox commented May 24, 2024

Tests added. Docs are in academy and in the repo.

Will see where best to improve stuff.

@smoser
Copy link
Contributor

smoser commented May 28, 2024

Just a comment here for myself on our intended use case (per @xnox)
The goal is for us to create py3.10-foo, py3.11-foo, py3.12-foo (each supported python version, as we already do in some cases). We want the py3.12-foo (the latest) to be "default", such that if something depends on py3-foo it will get the highest priority version without us having to encode that specifically in py3-foo.yaml.

xnox wrote:

all three py3.x-foo packages can provide py3-foo and highest one gets automatically installed without encoding which one is default in the top level py3-foo package as explicit dep.
so "provider-priority:" variable substitution will be used
and "replaces-priority" will be supported but in-so-far unused

smoser
smoser approved these changes May 28, 2024
View reviewed changes
Copy link
Contributor

@smoser smoser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for adding a test!

@xnox xnox merged commit 427ebb1 into chainguard-dev:main May 28, 2024
@xnox xnox mentioned this pull request May 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smoser smoser smoser approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@xnox @kaniini @smoser