Skip to content

Follow up on #2654#2687

Merged
lsm5 merged 4 commits intocontainers:mainfrom
lsm5:ci-agnostic-2
Sep 2, 2025
Merged

Follow up on #2654#2687
lsm5 merged 4 commits intocontainers:mainfrom
lsm5:ci-agnostic-2

Conversation

@lsm5
Copy link
Copy Markdown
Member

@lsm5 lsm5 commented Aug 27, 2025

Please review and merge #2654 first if that makes things simpler.

See individual commits. This makes Packit config and process simpler. Reusability of TMT tests is also much easier:

See: https://src.fedoraproject.org/rpms/skopeo/pull-request/83# and containers/container-libs#287

Copy link
Copy Markdown
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

I know very little about Packit, and this looks plausible overall.

Just to be sure nothing is missed, @jnovy please speak up if there were any downstream impacts.

@lsm5 lsm5 marked this pull request as ready for review August 29, 2025 18:06
@lsm5
Copy link
Copy Markdown
Member Author

lsm5 commented Aug 29, 2025

Btw, this will not affect centos stream/rhel as Packit isn't used for updates there.

@lsm5 lsm5 marked this pull request as draft August 29, 2025 18:36
@lsm5 lsm5 marked this pull request as ready for review August 29, 2025 18:39
@lsm5
Copy link
Copy Markdown
Member Author

lsm5 commented Aug 29, 2025

Rebased on main. I'll handle Makefile cleanups to remove unused targets in another PR later.

@jnovy @mtrmac good for a final review.

@mtrmac
Copy link
Copy Markdown
Contributor

mtrmac commented Aug 29, 2025

I was more worried about dist-git depending on some files existing in the upstream repo — but, also, I didn’t (yet) take the time to look.

@lsm5
Copy link
Copy Markdown
Member Author

lsm5 commented Aug 29, 2025

@mtrmac Ack, sure thing. If it helps ...

$ sudo dnf repoquery --whatrequires skopeo-tests
Updating and loading repositories:
Repositories loaded.

.. no packages listed.

Also, see the CI jobs on this dist-git PR for corresponding downstream tests: https://src.fedoraproject.org/rpms/skopeo/pull-request/83# where we removed dependency on skopeo-tests in TMT jobs. That diff isn't dependent on skopeo-tests anymore for CI jobs.

The rpm/gating.yaml removed didn't matter to upstream anyway, and it does exist downstream too where it matters. Earlier, I was thinking of having everything upstream, but if the file isn't gonna change often downstream, then it makes no sense for these files to exist upstream and get synced via the files_to_sync Packit feature.

So, I'd say we're fine.

@jnovy
Copy link
Copy Markdown
Contributor

jnovy commented Sep 1, 2025

I'm more fond of skopeo-tests still being shipped as (meta)package - the reason is simple - if we ever decide to run a different test suite than TMT we need to reintroduce it again - or manually rewrite dependencies again into a new test framework with every framework change. The existence of skopeo-tests just takes care of it independently.

@lsm5
Copy link
Copy Markdown
Member Author

lsm5 commented Sep 1, 2025

I'm more fond of skopeo-tests still being shipped as (meta)package - the reason is simple - if we ever decide to run a different test suite than TMT we need to reintroduce it again - or manually rewrite dependencies again into a new test framework with every framework change. The existence of skopeo-tests just takes care of it independently.

ya, metapackage, to confirm meaning something that only installs the test dependencies sounds like a great thing. I can do that and skip the test files installation.

@lsm5 lsm5 marked this pull request as draft September 1, 2025 13:58
@lsm5 lsm5 marked this pull request as ready for review September 1, 2025 15:06
@lsm5 lsm5 marked this pull request as draft September 1, 2025 15:06
@lsm5 lsm5 marked this pull request as ready for review September 1, 2025 15:45
@lsm5
Copy link
Copy Markdown
Member Author

lsm5 commented Sep 1, 2025

Ready for another review. skopeo-tests now only installs the test dependencies, no test files anymore. Also, I added an extra commit to re-enable ELN Packit jobs. Made fakeroot a Recommends in the rpm spec and that's working on ELN too.

PTAL @mtrmac @jnovy

Copy link
Copy Markdown
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

The rpm/gating.yaml removed didn't matter to upstream anyway, and it does exist downstream too where it matters.

Ah, indeed, and same for plans/. That resolves that concern.

@jnovy ?

lsm5 added 4 commits September 2, 2025 08:49
Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
The sole purpose of skopeo-tests subpackage was to make system test
files readily available for CI on bodhi updates.

Given we can reuse test config from upstream via TMT, there's no
reason to continue shipping the test files. This subpackage can be
repurposed to only install test dependencies.

Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
With TMT being able to fetch and run tests using git url
and ref, there's no need to sync files on every propose_downstream
Packit action.

Removing files_to_sync should be safe as we only need to sync
`.packit.yaml` which is part of Packit's default behavior already.

Other files like gating.yaml only need to exist downstream and
shouldn't need any frequent manual changes, so
we can remove those from upstream and packit file-sync too.

New setup: We use a `prepare-files` action that will operate only on
`plans/main.fmf` in downstream dist-git and update the ref with the
tag from the latest release.

Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
@lsm5
Copy link
Copy Markdown
Member Author

lsm5 commented Sep 2, 2025

rebased on main, no code changes.

@jnovy
Copy link
Copy Markdown
Contributor

jnovy commented Sep 2, 2025

For gating.yaml - yes, it's important only downstream where it is already present in dist-git and CentOS gitlab. @lsm5 @mtrmac

@lsm5
Copy link
Copy Markdown
Member Author

lsm5 commented Sep 2, 2025

Merging..

@lsm5 lsm5 merged commit ef442e3 into containers:main Sep 2, 2025
28 checks passed
@lsm5 lsm5 deleted the ci-agnostic-2 branch September 2, 2025 13:12
@stale-locking-app stale-locking-app Bot locked as resolved and limited conversation to collaborators Dec 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants