Opened 5 years ago
Closed 5 years ago
#14422 closed defect (fixed)
Fix makefile rules to make+hash incrementals by default
Reported by: | mikeperry | Owned by: | mikeperry |
---|---|---|---|
Priority: | Medium | Milestone: | |
Component: | Applications/Tor Browser | Version: | |
Severity: | Keywords: | tbb-gitian, TorBrowserTeam201502R, MikePerry201502 | |
Cc: | gk, boklm | Actual Points: | |
Parent ID: | Points: | ||
Reviewer: | Sponsor: |
Description
The new incremental Makefile rules are not run by default, and the resulting incrementals are not hashed by default. This is annoying, and leads to extra steps (and more chance for mistakes) during build.
Child Tickets
Change History (11)
comment:1 Changed 5 years ago by
Cc: | gk boklm added |
---|---|
Status: | new → needs_review |
comment:2 Changed 5 years ago by
Ok, it looks good to me. Maybe the commit message should also mention that the *-beta rules are being removed.
comment:3 Changed 5 years ago by
I had been thinking about streamlining this process but hesitated so far due to possible breakage for anonymous builders. They might not have all the things to build the incrementals which makes it harder for them to verify our builds.
comment:4 Changed 5 years ago by
Do the incremental rules cause the build to fail if the previous version is not present? Perhaps we can make that not be a failure condition, but instead simply print out a warning during 'make hash' or equivalent.
We could also have this warning give a wget scriptlet/command to download the previous build's mar files, or also make that a rule, or something?
comment:5 Changed 5 years ago by
Yes, if the previous version is not present, the incremental rule will fail and the hash rule will not be run.
If we want to remove the incrementals failures and replace them with a warning, we can change the incrementals rules to something like this:
incrementals: ../tools/update-responses/gen_incrementals release || echo 'Warning: could not generate incremental MARs.' >&2
Downloading the previous version if it is missing sounds like a good idea. However, I don't think we currently have a place from where to download it, as we are removing it from dist.tpo when there is a new release.
comment:6 follow-up: 7 Changed 5 years ago by
I am not a big fan of "soft failures" such as emitting a warning instead of stopping (easy to miss). But I am also not one of the people who produces our official builds, at least not at the moment.
comment:7 Changed 5 years ago by
Replying to mcs:
I am not a big fan of "soft failures" such as emitting a warning instead of stopping (easy to miss).
Me neither. But I don't want to make it harder for anonymous builder and doing less steps during the release cycle can't hurt. So, I am in favor of having the warning in case someone does not have all the things needed for the incrementals.
Downloading and using the other MARs + getting reproducible MAR files in return that match ours might need more thinking anyway due to the signatures which start to get included. I think this complication can be handled in the future if need arises, in a different bug.
But I am also not one of the people who produces our official builds, at least not at the moment.
If you want, just say so. :)
comment:8 Changed 5 years ago by
Keywords: | TorBrowserTeam201502R added; TorBrowserTeam201501R removed |
---|
comment:9 Changed 5 years ago by
Keywords: | MikePerry201502 added |
---|---|
Status: | needs_review → needs_revision |
Ok, I will update my branch to include boklm's change, as well as to emit another scriptlet suggestion at the end of the hashing step, if possible.
comment:10 Changed 5 years ago by
Owner: | changed from tbb-team to mikeperry |
---|---|
Status: | needs_revision → assigned |
comment:11 Changed 5 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Ok, I merged this with boklm's suggestions.
boklm - it turns out we do have archives of all of our packages, so we can in fact download missing MAR files during the incremental step. As GK said, though, we may still need to deal with signatures. I filed #14959 for this.
See mikeperry/bug14422-master.