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

ci: Add Packit configuration for RPM builds on PRs #1159

Merged
merged 2 commits into from
Aug 2, 2023

Conversation

martinpitt
Copy link
Contributor

Configure with minimal options, so that we can build a dist tarball without too many extra dependencies. In particular, we want to avoid libblockdev, as we usually need the latest upstream master one, not the distro package.


This is a fixed replacement of PR #1080. I got a successful build on my fork PR.

Note that this currently uses the default transient packit COPR to build the packages. I would strongly recommend to use them, unless you have a good reason to use your custom one.

If you want "daily builds" done by packit, this needs to happen in a trigger: commit (on master branch) job. These are different from this trigger: pull_request builds, which are done from the proposed forks/branches. They should not land in the user-facing COPR that "daily" is. It should be fairly obvious how to set these up, but if you want me to, I'm happy to send a follow-up PR which sets this up.

Unconditionally define the various *_in_files, so that they appear in
`make dist` tarballs independent of the configure options (in
particular, `--disable-daemon`).
@StorageGhoul
Copy link

Can one of the admins verify this patch?

.packit.yaml Outdated Show resolved Hide resolved
Configure with minimal options, so that we can build a dist tarball
without too many extra dependencies. In particular, we want to avoid
libblockdev, as we usually need the latest upstream master one, not the
distro package.
@martinpitt
Copy link
Contributor Author

@vojtechtrefny Does that supersede https://github.com/storaged-project/udisks/blob/master/.github/workflows/rpmbuild.yml ? Does any test CI job download/use that? AFAICS it only uploads the build log as an artifact, not the actual rpm. What do the integration tests use? Could they pull it from the packit COPR, or should there be a persistent COPR for their benefit, or do they rebuild the rpm again?

@vojtechtrefny
Copy link
Member

@vojtechtrefny Does that supersede https://github.com/storaged-project/udisks/blob/master/.github/workflows/rpmbuild.yml ?

Yes, this can be removed now.

What do the integration tests use? Could they pull it from the packit COPR, or should there be a persistent COPR for their benefit, or do they rebuild the rpm again?

We are using the @storage/udisks-daily for our CI runners. The daily builds are now done by Jenkins, but we'll want to switch that to Packit too (we already use Packit for daily builds for libblockdev and other projects).

Copy link
Member

@vojtechtrefny vojtechtrefny left a comment

Choose a reason for hiding this comment

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

Thank you! @tbzatek should look at the build system changes, but the packit configuration looks good to me.

@martinpitt
Copy link
Contributor Author

Ah, so the CI runners only test landed PRs, not the code in the PR?

Yes, this can be removed now.

Want me to do this in this PR, or do you want to clean this up in a follow-up?

If you want "daily builds" done by packit, this needs to happen in a trigger: commit

Are you going to set this up, or want me to here?

Cheers!

@vojtechtrefny
Copy link
Member

Ah, so the CI runners only test landed PRs, not the code in the PR?

The tests on the PR run only against the compiled code (both dbus tests and integration tests do that). But we use the daily builds Copr repo when testing our other projects that depend on udisks.

Want me to do this in this PR, or do you want to clean this up in a follow-up?

I'll do a follow up PR after we merge this.

@martinpitt martinpitt changed the title ci: Add Packit configuration for RPM builds on PRs [fixed] ci: Add Packit configuration for RPM builds on PRs Aug 2, 2023
@tbzatek
Copy link
Member

tbzatek commented Aug 2, 2023

Jenkins, ok to test.

Copy link
Member

@tbzatek tbzatek left a comment

Choose a reason for hiding this comment

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

Hmm, after wrapping my head around to see if there are any other ways to avoid the dist tarball creation over a minimal configured project I still think this is good enough.

Comment on lines +3 to +7
- './autogen.sh --disable-modules --disable-daemon'
# FIXME: just calling `make dist` doesn't build udisks/libudisks2.la for gtk-doc
- 'make'
- 'make dist'
- 'bash -c "ls *.tar*"'
Copy link
Member

Choose a reason for hiding this comment

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

So first I thought making a tarball from git sources directly and calling autogen.sh within the spec file might be a cleaner way, however then I realized the spec file wouldn't work against proper dist tarball as there's no autogen.sh then. Sure, a bunch of if-else stuff might do the trick but it's not a clean way either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I thought the same -- it's quite nice to be able to build a package straight from a git archive tarball, without autoconfiscation. But indeed that then wouldn't have ./configure, Makefile.in, etc.

@tbzatek tbzatek merged commit d84a51f into storaged-project:master Aug 2, 2023
8 checks passed
@martinpitt martinpitt deleted the master_packit branch August 2, 2023 15:51
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