Skip to content

Packit: initial enablement#30

Merged
Luap99 merged 1 commit into
podman-container-tools:mainfrom
lsm5:packit
Aug 26, 2025
Merged

Packit: initial enablement#30
Luap99 merged 1 commit into
podman-container-tools:mainfrom
lsm5:packit

Conversation

@lsm5
Copy link
Copy Markdown
Contributor

@lsm5 lsm5 commented Aug 26, 2025

No description provided.

@lsm5 lsm5 marked this pull request as ready for review August 26, 2025 13:10
@lsm5
Copy link
Copy Markdown
Contributor Author

lsm5 commented Aug 26, 2025

@jankaluza @Luap99 PTAL.

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.

Just a brief skim, I don’t know what we want/need to do via Packit, and how/if at all it interacts with other downstream packaging.

Comment thread common/rpm/containers-common.spec
Comment thread common/rpm/containers-common.spec Outdated
Comment thread common/rpm/containers-common.spec
Comment thread common/rpm/containers-common.spec Outdated
Comment thread common/rpm/containers-common.spec Outdated
ensure pkg/config/containers.conf runtime \"crun\"
ensure pkg/config/containers.conf log_driver \"journald\"
ensure common/pkg/config/containers.conf runtime \"crun\"
ensure common/pkg/config/containers.conf log_driver \"journald\"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There was some … discussion in #4 about how packaging happens. Does this risk interfering with downstream RPMs?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This has been happening already in containers-common rpm that was done from the c/common repo and shipped to Fedora for quite sometime now. Only the path was updated to match monorepo. The final list of files installed is not being changed (after I remove the additional docs stuff that you commented about)

Comment thread common/rpm/containers-common.spec Outdated
@jankaluza
Copy link
Copy Markdown
Contributor

btw, The Common: Test is fixed in main, so rebase should help.

@lsm5 lsm5 marked this pull request as draft August 26, 2025 13:52
@lsm5 lsm5 force-pushed the packit branch 2 times, most recently from 586f3b9 to 1e62679 Compare August 26, 2025 14:07
@packit-as-a-service
Copy link
Copy Markdown

Packit jobs failed. @containers/packit-build please check.

@lsm5 lsm5 marked this pull request as ready for review August 26, 2025 14:22
@lsm5
Copy link
Copy Markdown
Contributor Author

lsm5 commented Aug 26, 2025

Ready for review after addressing @mtrmac 's initial comments.

Copy link
Copy Markdown
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

Can you remove the common/.packit.yaml file then? I assume then git would show this file as rename which likely helps for reviewers.

@lsm5
Copy link
Copy Markdown
Contributor Author

lsm5 commented Aug 26, 2025

Can you remove the common/.packit.yaml file then? I assume then git would show this file as rename which likely helps for reviewers.

Let me check. I think .packit.yaml is expected in top-level.

@Luap99
Copy link
Copy Markdown
Member

Luap99 commented Aug 26, 2025

Can you remove the common/.packit.yaml file then? I assume then git would show this file as rename which likely helps for reviewers.

Let me check. I think .packit.yaml is expected in top-level.

Yes that is fine, what I am saying is you should remove common/.packit.yml in your commit. Then git sees that as file move and so the diff should be more readable compared to having to look at a new file.

@lsm5
Copy link
Copy Markdown
Contributor Author

lsm5 commented Aug 26, 2025

Can you remove the common/.packit.yaml file then? I assume then git would show this file as rename which likely helps for reviewers.

Let me check. I think .packit.yaml is expected in top-level.

Yes that is fine, what I am saying is you should remove common/.packit.yml in your commit. Then git sees that as file move and so the diff should be more readable compared to having to look at a new file.

Ah ok. My bad. Doing it.

@lsm5
Copy link
Copy Markdown
Contributor Author

lsm5 commented Aug 26, 2025

Done. Thanks, and sorry for my misread earlier.

Comment thread .packit.yaml Outdated
Signed-off-by: Lokesh Mandvekar <lsm5@redhat.com>
Copy link
Copy Markdown
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM, if the chnage is intentional. Though I don't really have tried to understand most of the changes.

Comment thread .packit.yaml
# Ignore CentOS Stream for now
- job: propose_downstream
trigger: release
trigger: ignore
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I assume that change is intentional?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, this one is intentional. We're not doing centos stream official updates via packit atm.

Copy link
Copy Markdown
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

@Luap99 Luap99 merged commit 3e7a25e into podman-container-tools:main Aug 26, 2025
14 checks passed
@lsm5 lsm5 deleted the packit branch August 26, 2025 17:09
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.

4 participants