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: introduce Packit #347

Merged
merged 1 commit into from
Mar 9, 2024
Merged

ci: introduce Packit #347

merged 1 commit into from
Mar 9, 2024

Conversation

mrc0mmand
Copy link
Contributor

@mrc0mmand mrc0mmand commented Mar 7, 2024

Build (and test) dbus-broker on all active Fedora releases using Packit.
This uses Fedora's spec file (from Rawhide) with a couple of tweaks, so
we don't have to ship our own.

Replaces: #279

@teg
Copy link
Contributor

teg commented Mar 7, 2024

Thanks for picking this up @mrc0mmand!

@mrc0mmand
Copy link
Contributor Author

Thanks for picking this up @mrc0mmand!

No problem, I'll shamelessly copy & tweak what I've already done for systemd. (I originally wanted to play around with this a bit without disturbing the main repo, hence the "systemd-incubator" source, but apparently managed to hit the wrong button, oh well.)

@mrc0mmand mrc0mmand force-pushed the packit branch 4 times, most recently from f9f049c to 7e101d2 Compare March 7, 2024 14:55
Build (and test) dbus-broker on all active Fedora releases using Packit.
This uses Fedora's spec file (from Rawhide) with a couple of tweaks, so
we don't have to ship our own.

Replaces: bus1#279
@mrc0mmand
Copy link
Contributor Author

So, this should be now (hopefully) ready for a review I looked at the original PR (#279) and it also added a couple of sanity tests that run in Testing Farm, and also enabled the propose downstream stuff (which I've never played around with, so far). Do you want it enabled now as well or shall we leave that for future PRs?

@mrc0mmand mrc0mmand marked this pull request as ready for review March 7, 2024 15:27
@teg
Copy link
Contributor

teg commented Mar 7, 2024

Feel free to just build for now, but could you share your intention with this work (for anyone watching)? Do you just want to build the package to make sure it keeps working, or do you want to follow up with PRs to run integration tests? I assume propose downstream stuff would be out of scope for you (I don't even know if that is the right path, so no complaints from me)?

@teg
Copy link
Contributor

teg commented Mar 7, 2024

Could you comment on the choice of getting the specfile from Rawhide rather than moving it upstream? Wouldn't that potentially get messy if we make changes that require specfile changes?

@mrc0mmand
Copy link
Contributor Author

mrc0mmand commented Mar 7, 2024

Feel free to just build for now, but could you share your intention with this work (for anyone watching)? Do you just want to build the package to make sure it keeps working, or do you want to follow up with PRs to run integration tests? I assume propose downstream stuff would be out of scope for you (I don't even know if that is the right path, so no complaints from me)?

Ideally this PR would be followed by addition of some integration tests, that would run in Testing Farm (where they can run on x86_64 and aarch64, ATTOW). This is a big ? since, to my knowledge, there are no integration tests for dbus-broker here or in Fedora, but there's a couple of candidates which could serve as a starting point. The Testing Farm itself provides an installability & sanity check, since it installs the built RPMs and reboots the machine, and with broken dbus it wouldn't get too far. Another candidate could be running dfuzzer, which we run as part of the systemd test suite, that tests dbus-broker as collateral damage.

Could you comment on the choice of getting the specfile from Rawhide rather than moving it upstream?

This is mostly force of habit from other projects I've worked on, and also the fact that there are no distro-specific artifacts in the dbus-broker upstream. I don't really have a preference, what I mainly want to avoid is code & configuration duplication.

Wouldn't that potentially get messy if we make changes that require specfile changes?

That's a valid concern. Speaking from my (systemd) experience, such changes are usually not that common, and if they happen they're not something that cannot be (temporarily) resolved by one or two seds in the Packit configuration file (which is messy by definition, I know). Also, similarly to systemd's case, both upstream and Fedora downstream are controlled by the same people, which make such changes much easier to coordinate when they happen.

@teg
Copy link
Contributor

teg commented Mar 8, 2024

Feel free to just build for now, but could you share your intention with this work (for anyone watching)? Do you just want to build the package to make sure it keeps working, or do you want to follow up with PRs to run integration tests? I assume propose downstream stuff would be out of scope for you (I don't even know if that is the right path, so no complaints from me)?

Ideally this PR would be followed by addition of some integration tests, that would run in Testing Farm (where they can run on x86_64 and aarch64, ATTOW). This is a big ? since, to my knowledge, there are no integration tests for dbus-broker here or in Fedora, but there's a couple of candidates which could serve as a starting point. The Testing Farm itself provides an installability & sanity check, since it installs the built RPMs and reboots the machine, and with broken dbus it wouldn't get too far. Another candidate could be running dfuzzer, which we run as part of the systemd test suite, that tests dbus-broker as collateral damage.

I think both sound like a great starting point. Booting is certainly a great sanity test for dbus.

You are right there are no integration tests. I think creating tests (long term) using busctl to validate the spec would be the way to go. Maybe creating a trivial example of that would be a great start so others can easily follow the example and add more (covering the whole spec will be quite a bit of work).

Could you comment on the choice of getting the specfile from Rawhide rather than moving it upstream?

This is mostly force of habit from other projects I've worked on, and also the fact that there are no distro-specific artifacts in the dbus-broker upstream. I don't really have a preference, what I mainly want to avoid is code & configuration duplication.

Wouldn't that potentially get messy if we make changes that require specfile changes?

That's a valid concern. Speaking from my (systemd) experience, such changes are usually not that common, and if they happen they're not something that cannot be (temporarily) resolved by one or two seds in the Packit configuration file (which is messy by definition, I know). Also, similarly to systemd's case, both upstream and Fedora downstream are controlled by the same people, which make such changes much easier to coordinate when they happen.

I'm happy to leave this as is and potentially change the way we handle spec files in a follow up. @dvdhrm ?

@teg teg merged commit 0a0b0fa into bus1:main Mar 9, 2024
33 checks passed
@mrc0mmand mrc0mmand deleted the packit branch March 11, 2024 08:56
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