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 #1080

Closed

Conversation

vojtechtrefny
Copy link
Member

No description provided.

@vojtechtrefny vojtechtrefny force-pushed the master_packit branch 2 times, most recently from e54fe96 to 27eda0f Compare March 24, 2023 14:24
@packit-as-a-service
Copy link

Based on your Packit configuration the settings of the @storage/udisks-pr Copr project would need to be updated as follows:

field old value new value
additional_repos ['copr://@storage/udisks-daily'] []

Packit was unable to update the settings above as it is missing admin permissions on the @storage/udisks-pr Copr project.

To fix this you can do one of the following:

  • Grant Packit admin permissions on the @storage/udisks-pr Copr project on the permissions page.
  • Change the above Copr project settings manually on the settings page to match the Packit configuration.
  • Update the Packit configuration to match the Copr project settings.

Please retrigger the build, once the issue above is fixed.

@packit-as-a-service
Copy link

Failed to load packit config file:

Cannot parse package config: ValidationError({'jobs': {0: {'packages': defaultdict(<class 'dict'>, {'udisks2': {'value': {'additional_repos': ['Not a valid list.']}}})}}}).

For more info, please check out the documentation or contact the Packit team.

@packit-as-a-service
Copy link

We have requested the builder permissions for the @storage/udisks-pr Copr project.

Please confirm the request on the @storage/udisks-pr Copr project permissions page and retrigger the build by a /packit build pull-request comment or click on a Re-run button.

@vojtechtrefny
Copy link
Member Author

/packit build

1 similar comment
@vojtechtrefny
Copy link
Member Author

/packit build

@vojtechtrefny
Copy link
Member Author

@martinpitt so the problem with this is we want to use the latest build of libblockdev (also available in @storage/udisks-daily) to build the SRPM, the problem is that the additional_repos property adds the repo only for the RPM build phase, the SRPM build phase runs in a different environment where we cannot add the repo.

@packit-as-a-service
Copy link

Account martinpitt has no write access nor is author of PR!

@martinpitt
Copy link
Contributor

martinpitt commented Aug 2, 2023

Thanks for the ping @vojtechtrefny ! The srpm build apparently succeeded, just the binary one failed. The package builds are long gone, so I'll retry this in my own fork: martinpitt#1

But conceptionally, why would you even need all these build dependencies and the extra COPR for building the source RPMs? This shouldn't be much more than git archive (or make dist from a minimal config, with the minimal build deps to satisfy ./configure) plus adding the .spec. The result of make dist must not depend on configure options anyway, you should always ship the whole source.

@vojtechtrefny
Copy link
Member Author

/packit build

@martinpitt
Copy link
Contributor

That said, apparently you can do srpm builds in a custom repo as well. The podman team does this, and e.g. this PR build and the srpm build log at least appears to have happened in their custom copr, not in the standard packit srpm copr. @lachmanfrantisek, @lbarcziova, is that right?

That said, this is still fishy -- it would IMHO be better to have a way to get a dist tarball without having to install the latest and greatest libblockdev (for binary builds you need it of course)

@vojtechtrefny
Copy link
Member Author

That said, this is still fishy -- it would IMHO be better to have a way to get a dist tarball without having to install the latest and greatest libblockdev (for binary builds you need it of course)

I agree with that, but I am not sure how to do that without rewriting a big portion of configure.ac -- we need to run autogen and configure to generate working Makefile to be able to run make dist. (But maybe I am missing some simple way how to bypass the configure step. make dist -f Makefile.am doesn't seem to do the trick.)

@martinpitt
Copy link
Contributor

@vojtechtrefny : I have a current build in martinpitt#1 , and indeed it fails to install libblockdev-nvme-devel . Give me an hour or so, I have an idea.

Comment on lines +10 to +11
owner: "@storage"
project: udisks-pr
Copy link
Contributor

Choose a reason for hiding this comment

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

@vojtechtrefny Just one question: Why do you want a custom COPR for PR builds? Aren't the transient ones that packit provides good enough for testing PR? (We only use them in cockpit, and they are just fine).

Note that this is not the same as your "daily" COPR -- that should be a permanent one, but that will have trigger: push (on master).

Copy link
Member Author

@vojtechtrefny vojtechtrefny Aug 2, 2023

Choose a reason for hiding this comment

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

Oh, I forgot about this. Packit team recommended using a custom repo and adding the @storage/udisks-daily repo to the external repositories. This also didn't work and since that I removed the repo from @storage. (This also explains why Packit failed to do a new build when I triggered it manually.)

@martinpitt
Copy link
Contributor

I sent PR #1159 to fix this.

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.

2 participants