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

main: add --use-librepo to support librepo downloads #51

Merged
merged 6 commits into from
Jan 17, 2025

Conversation

mvo5
Copy link
Collaborator

@mvo5 mvo5 commented Jan 8, 2025

With that it then adds a new --use-librepo switch that will
enable librepo based downloading so that people can use
the new backend.

It is on by default currently, the risk is that it got no real testing
with secrets (but there code is there and similar to what curl
is doing) but the upside is that it will help with the centos/fedora
cases as there the mirror retry feature is import important.

@mvo5 mvo5 force-pushed the use-librepo branch 6 times, most recently from 02c5f2b to ccf24c9 Compare January 15, 2025 09:48
@mvo5 mvo5 changed the title main: add experimental --use-librepo to support librepo downloads main: add --use-librepo to support librepo downloads Jan 15, 2025
@mvo5 mvo5 marked this pull request as ready for review January 17, 2025 09:24
mvo5 added 3 commits January 17, 2025 10:50
This commit switches to the librepo enabled `images` library via:
```
go mod -replace github.com/osbuild/iamges=github.com/mvo5/images@librepo-sources-osbuild1974
```
which in turn needs osbuild PR#1974.

With that it then adds a new `--use-librepo` switch that will
enable librepo based downloading so that people can play with
the new backend.
This commit adds a new manifestOptions struct that is passed
to generateManifest. to cleanup the signature of generateManifest().

This can then also be used to carry a new e.g. `--rpmmd/--cachedir`
option.
This commit changes the signature of depsolve in `manifestgen`
to return the full `dnfjson.DepsolveResult`. This gives us
access to the sbom and also is a preparation for images PR#1142
where we will need this anyway.
@@ -185,6 +199,7 @@ operating sytsems like centos and RHEL with easy customizations support.`,
manifestCmd.Flags().String("ostree-ref", "", `OSTREE reference`)
manifestCmd.Flags().String("ostree-parent", "", `OSTREE parent`)
manifestCmd.Flags().String("ostree-url", "", `OSTREE url`)
manifestCmd.Flags().Bool("use-librepo", true, `use librepo to download packages (disable if you use old versions of osbuild)`)
Copy link
Member

Choose a reason for hiding this comment

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

We can 'just' bump the minimum required osbuild version instead of this remark; let's do that.

mvo5 added 2 commits January 17, 2025 11:45
This commit switches the default to librepo. We do not have many
users yet so this is a great place to test the new librepo
functionality. It also is much more stable for fedora/centos
systems that often have flaky mirrors.

This also bumps the minimum required version of osbuild.
This commit adds a (smoke) integration test for librepo based
manifests. It needs a flanking test that also ensures that
--use-librepo really generates librepo sources.
Copy link
Member

@supakeen supakeen left a comment

Choose a reason for hiding this comment

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

Cool, thank you :)

Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

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

The first two commits could be squashed. The commit message for the second one seems like it's talking about both.

Not a big issue though.

Tiny comment about a comment below. Overall LGTM.

Comment on lines +6 to +10
# The minimum required osbuild version, note that this used to be 129
# but got bumped to 138 for librepo support which is not strictly
# required. So if this needs backport to places where there is no
# recent osbuild available we could simply make --use-librepo false
# and go back to 129.
Copy link
Member

Choose a reason for hiding this comment

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

Is this a realistic issue? I guess it might be if you're go installing ib-cli, but in that case, you're on your own and the spec file doesn't matter.

I mean, there's no way this spec file will be used to officially build ib-cli in a distro where a new enough osbuild wont be, so I think this note isn't relevant.

I might be missing something though.

@supakeen supakeen added this pull request to the merge queue Jan 17, 2025
Merged via the queue into osbuild:main with commit bdb3255 Jan 17, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants