-
Notifications
You must be signed in to change notification settings - Fork 141
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
Notification for mismatching builds #1203
Comments
I think we've previously discussed that having CI to look for mismatches in PRs was bad because we want to know about and investigate mismatches. But it also seems like in the PR is when we want to know about that, so maybe we can have a CI task that is ignorable and just serves as an indicator that there is a mismatch? I don't think we'd necessarily want to have a CI task on the main branch that fails when anything mismatches because I there are classes of mismatch which we should keep, e.g. actual nondeterminism in the build process. Also, our guix stuff is a bit less resilient to mismatches than gitian was. If someone didn't build a system, we would get a mismatch, whereas I think this didn't always happen with gitian? Sometimes this is intentional if the builder doesn't have the MacOS SDK for example, but other times it does indicate that something went wrong with the build. |
Agree that in the PR is when you want to know. A daily check on master might already be too late. But yes, blocking the merge on mismatching hashes would be bad, imo. The CI can't tell which set of hashes is correct (if there's such a thing), it's not necessarily the one already in the repository. Having a signaling-only CI run, if that's possible, sounds like a good compromise
Right. Gitian had separate attestations per group of platforms (-windows, -linux, -osx, plus codesigned variants). If someone would sign for only linux, this would be fine. This could only work because the generation and signing of the final SHA256SUMS file was manually done by the release maintainer. Guix's automatic process is more strict. Even if it'd have separate attestations per platform (which was considered, but IIRC we didn't go for it because if one is using, say, a Yubikey one had to press to sign a zillion times*), separate PGP signatures over different fragments likely can't be joined into a single file for release. Which would make checking more burdensome for the end user. * Another thing considered was separate detached PGP signatures per file. But even worse in this regard, the sheer number of signatures needed.
Agree. i had a bug in my scripting where i would still attest when the build errored out halfway and thus only part of the platforms was built. It would have been nice to catch this sooner. Might have two seperate (signalling only) checks:
|
I noticed that for 23.0rc1 and 23.0rc2 there are multiple distinct hashes for edit: see bitcoin/bitcoin#24549, not a problem as this were RCs
|
Maybe a script could be added that maintainers use to check, merge, and push the attestations? Maintainers can then decide on the course of action if there is a mismatch, or missing file. At least that is what I am doing for the gitian.sigs repo I still maintain. |
FWIW, this was my goal for https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/guix-verify.py, to check and display the build results in a table to have a better overview. In any case, i think such a thing is seperate from PR-time signalling. |
Cool, wasn't aware of this. This particular script might be separate from PR-time signaling, but the idea still works? For example, have the CI generate and comment a table/image/heatmap that shows the derivation from mean hash value for each touched release and noncodesigned/all SHASUMS combination. For example, without mismatches is could look like this: A single mismatch like this (there's actually a mismatch there) Missing artifacts/hosts like this And non-determinism across the board like this: edit: having the CI leave a table as comment seems significantly easier - no need to worry about image hosting and similar |
having a text-based table is probably easier, so could be a CI job that comments on a PR for each changed release/release candidate. Before merging, it can be quickly visually verified that there are no mismatches. Example:
char to username mapping
If this looks useful, I'd be happy to PR it. |
Proposed a solution in #1207 |
In #1201 it turns out that @0xB10C had been building mismatching builds for a while, without it being noticed before.
It would be useful to have some kind of notification when this happens. A mismatching build always points to a problem, either (in the good case) a local configuration issue, but it can also be a bug in the build system or worst-case even a backdoor attempt. So it needs to be investigated.
See discussion here: #1201 (comment)
The text was updated successfully, but these errors were encountered: