teg
commented
8 months ago Thanks for picking this up @mrc0mmand!
Closed mrc0mmand closed 8 months ago
teg
commented
8 months ago Thanks for picking this up @mrc0mmand!
mrc0mmand
commented
8 months ago 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
commented
8 months ago 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?
teg
commented
8 months ago 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
commented
8 months ago 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
commented
8 months ago 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
commented
8 months ago 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 ?
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: https://github.com/bus1/dbus-broker/pull/279