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 mikeperry

Cc: gk boklm added
Status: newneeds_review

See mikeperry/bug14422-master.

comment:2 Changed 5 years ago by boklm

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 gk

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 mikeperry

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 boklm

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 Changed 5 years ago by mcs

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 in reply to:  6 Changed 5 years ago by gk

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 mikeperry

Keywords: TorBrowserTeam201502R added; TorBrowserTeam201501R removed

comment:9 Changed 5 years ago by mikeperry

Keywords: MikePerry201502 added
Status: needs_reviewneeds_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 mikeperry

Owner: changed from tbb-team to mikeperry
Status: needs_revisionassigned

comment:11 Changed 5 years ago by mikeperry

Resolution: fixed
Status: assignedclosed

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.

Note: See TracTickets for help on using tickets.