Project

General

Profile

Bug #16730

Reproducible build CI job uses the HEAD of the current branch instead of the commit the 1st build was built from

Added by intrigeri 3 months ago. Updated about 2 months ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
Build system
Target version:
Start date:
Due date:
% Done:

100%

Feature Branch:
bugfix/16730-have-reproducible-build-attempt-use-same-commit-as-first-build, https://salsa.debian.org/tails-team/tails/merge_requests/22
Type of work:
Code
Blueprint:
Starter:
Affected tool:

Description

Since a few months, reproducibly_build_Tails_ISO_* jobs started failing when they shouldn't. I think this happens every time the branch the job is about was updated in our official Git repo, between the time the 1st build started and the time the 2nd build did. This creates lots of noise on the RMs list and decreases my confidence in these jobs' results, which is sad.

I looked closer at https://jenkins.tails.boum.org/view/RM/job/reproducibly_build_Tails_ISO_devel/613/ today and there we see that the 2nd build used a different commit than the 1st build: build-tails did reset the tree to the commit the branch was cloned at, i.e. its latest state in our canonical repo, while it should have reset the tree to the commit used by the 1st build, which Jenkins pretends is the current HEAD of the branch being (re)built. It seems that the recent optimizations in build-tails (done as part of #16476) override the trick we have on Jenkins for this (checkout_upstream_job_branch in https://git.tails.boum.org/jenkins-jobs/tree/macros/builders.yaml).

I suspect these optimizations gain us very little so I'll try reverting them, will measure the impact, and if they don't matter much I'll try reverting them to see if it fixes this bug.


Related issues

Related to Tails - Bug #16476: Allow building a branch using unpublished Git submodule changes Resolved 02/21/2019
Blocks Tails - Feature #16209: Core work: Foundations Team Confirmed 03/22/2019

Associated revisions

Revision 26f636bd (diff)
Added by intrigeri 3 months ago

Revert "Build system: try to be smart again by fetching only the refs we need."

This optimization overrides the trick we have on
Jenkins (set_origin_base_branch_head in
https://git.tails.boum.org/jenkins-jobs/tree/macros/builders.yaml), that ensures
that a reproducibly_build_Tails_ISO_* job builds from the commit used by the
first build.

This reverts commit 6093f369a00a95bc8705aef31a10f918996f5fc3.

Refs: #16730

Revision c9404239 (diff)
Added by intrigeri 3 months ago

Honor $BASE_BRANCH_GIT_COMMIT (refs: #16730)

So far we relied on a hack in our Jenkins job (set_origin_base_branch_head) to
tweak Git state in a way that git_base_branch_head gets it right. But that's
complex, fragile, and interacts badly with the refs mangling we do in
build-tails. So let's instead honor a pre-existing $BASE_BRANCH_GIT_COMMIT, so
we can drop the set_origin_base_branch_head hack from the Jenkins job.

What I'm doing here seems to have been the original intent of the work that was
done on #11972: f4a20a417d431d742f125ace08f90867c4b62ca8 and
49c293048fb53e00ca75138c1c1c955726708a76 have started work in this direction,
that was never finished, and $BASE_BRANCH_GIT_COMMIT was left entirely unused,
which confused me a great deal when starting to work on this.

Incidentally, this change makes $BASE_BRANCH_GIT_COMMIT behave in the same way
as $GIT_COMMIT (which is already taken from the environment if set), which
is more consistent and less confusing.

Commit c579fc97df8ea41dda67e24047cae131b2425250 in jenkins-jobs.git
sets $BASE_BRANCH_GIT_COMMIT to the commit that the base branch was in
during the first build, that we're trying to reproduce.

Revision 211f15af (diff)
Added by intrigeri 3 months ago

Honor $BASE_BRANCH_GIT_COMMIT (refs: #16730)

So far we relied on a hack in our Jenkins job (set_origin_base_branch_head) to
tweak Git state in a way that git_base_branch_head gets it right. But that's
complex, fragile, and interacts badly with the refs mangling we do in
build-tails. So let's instead honor a pre-existing $BASE_BRANCH_GIT_COMMIT, so
we can drop the set_origin_base_branch_head hack from the Jenkins job.

What I'm doing here seems to have been the original intent of the work that was
done on #11972: f4a20a417d431d742f125ace08f90867c4b62ca8 and
49c293048fb53e00ca75138c1c1c955726708a76 have started work in this direction,
that was never finished, and $BASE_BRANCH_GIT_COMMIT was left entirely unused,
which confused me a great deal when starting to work on this.

Incidentally, this change makes $BASE_BRANCH_GIT_COMMIT behave in the same way
as $GIT_COMMIT (which is already taken from the environment if set), which
is more consistent and less confusing.

Commit c579fc97df8ea41dda67e24047cae131b2425250 in jenkins-jobs.git
sets $BASE_BRANCH_GIT_COMMIT to the commit that the base branch was in
during the first build, that we're trying to reproduce.

Revision 45a56752
Added by anonym 3 months ago

Merge remote-tracking branch 'origin/bugfix/16730-have-reproducible-build-attempt-use-same-commit-as-first-build' into stable

Fix-committed: #16730

History

#1 Updated by intrigeri 3 months ago

  • Related to Bug #16476: Allow building a branch using unpublished Git submodule changes added

#2 Updated by intrigeri 3 months ago

  • Subject changed from Reproducible build CI job use the HEAD of the current branch instead of the commit the 1st build was built from to Reproducible build CI job uses the HEAD of the current branch instead of the commit the 1st build was built from
  • Feature Branch set to bugfix/16730-have-reproducible-build-attempt-use-same-commit-as-first-build

#3 Updated by intrigeri 3 months ago

  • Status changed from Confirmed to In Progress

#4 Updated by intrigeri 3 months ago

#5 Updated by intrigeri 3 months ago

  • QA Check set to Ready for QA

(Forcing Jenkins to try & reproduce builds from this branch, so that I can confirm that it actually fixes the bug :)

#6 Updated by intrigeri 3 months ago

intrigeri wrote:

I suspect these optimizations gain us very little so I'll try reverting them, will measure the impact

On https://jenkins.tails.boum.org/job/build_Tails_ISO_bugfix-16730-have-reproducible-build-attempt-use-same-commit-as-first-build/1/consoleFull, the full git fetch --tags took about 10 seconds. So even if the optimization brings this down to zero, it's definitely not worth it to try to fix this bug while keeping the optimization around, if dropping it is enough to fix the bug.

#7 Updated by intrigeri 3 months ago

  • Description updated (diff)

#8 Updated by intrigeri 3 months ago

The problem is more complicated than just dropping the optimization. The complex interaction between the Git refs tweaks we do on the Jenkins job side and the other refs tweaks we do in build-tails makes the whole things a mess to understand and causes this bug. I'm making this interaction more straightforward (based on passing environment variables to rake build instead of tweaking Git state), which should fix this bug, allow us to drop hard to understand code from the Jenkins job, make it easier to work on this part of our code, and as a bonus, allow us to test locally something closer to what is done on Jenkins. In general I think Jenkins jobs should do as little clever things as possible, and should stick as close as they can to what developers would run.

#9 Updated by intrigeri 3 months ago

And in the end, I'm not even 100% sure that the aforementioned optimizations have anything to do with this: on the one hand, I think the timing of when these jobs started failing for no reason matches; OTOH, reverting the optimization is not enough to fix the bug, so I'm confused. Anyway, it saves only 10s, it makes it harder than needed to reason on issues such as this one, and anonym was ready to drop it himself, so I'll keep the revert on my branch, even if it's not strictly needed (or at least not sufficient).

#11 Updated by intrigeri 3 months ago

  • Target version changed from Tails_3.14 to Tails_3.15

Note to reviewer: in order to test whether my branch fixes this bug, I've pushed additional unrelated code style commits that I wanted to do for a while, but never got to due to the paperwork overhead; so it's on purpose that I'm sneaking them into an unrelated branch.

#12 Updated by intrigeri 3 months ago

https://jenkins.tails.boum.org/job/reproducibly_build_Tails_ISO_bugfix-16730-have-reproducible-build-attempt-use-same-commit-as-first-build/3/ has successfully reproduced https://jenkins.tails.boum.org/job/build_Tails_ISO_bugfix-16730-have-reproducible-build-attempt-use-same-commit-as-first-build/4/ and was built from the same commit (211f15af9ade3381fb5ccc8ea61331ac78cea331), even though I had pushed additional commits to the topic branch in the meantime. This seems to prove that this branch fixes the bug this ticket is about.

Also, https://jenkins.tails.boum.org/job/reproducibly_build_Tails_ISO_bugfix-16730-have-reproducible-build-attempt-use-same-commit-as-first-build/3/consoleFull shows that:

I'll now wait to see if the bunch of unrelated I've pushed on top of the commits relevant to this problem break anything.

Dear reviewer, once this branch is merged into stable, devel, and feature/buster, please reassign to me and I'll remove the set_origin_base_branch_head step from the Jenkins job: this branch does the equivalent itself via the $BASE_BRANCH_GIT_COMMIT it receives on the rake build command line, so we can drop this hack (git update-ref) from the Jenkins job.

#13 Updated by intrigeri 3 months ago

  • Assignee changed from intrigeri to anonym
  • Feature Branch changed from bugfix/16730-have-reproducible-build-attempt-use-same-commit-as-first-build to bugfix/16730-have-reproducible-build-attempt-use-same-commit-as-first-build, https://salsa.debian.org/tails-team/tails/merge_requests/22

@anonym, I think you're the best placed person to review this :)

#14 Updated by anonym 3 months ago

  • Status changed from In Progress to Fix committed
  • % Done changed from 0 to 100

#15 Updated by anonym 3 months ago

  • Assignee deleted (anonym)
  • QA Check changed from Ready for QA to Pass

Except the possibly unnecessary drop of that optimization (which I OTOH won't bother fighting for) looks great! Awesome (unrelated) style changes! :) Merged!

#16 Updated by intrigeri 3 months ago

  • Status changed from Fix committed to In Progress
  • Assignee set to intrigeri
  • % Done changed from 100 to 70
  • QA Check deleted (Pass)

(As per "Dear reviewer, once this branch is merged into stable, devel, and feature/buster, please reassign to me".)

#17 Updated by intrigeri 3 months ago

@anonym, I see you've merged this only in stable; I assume it's a mistake and I've thus merged stable → devel → feature/buster so I can do the next step of the work here.

#18 Updated by intrigeri 3 months ago

intrigeri wrote:

Dear reviewer, once this branch is merged into stable, devel, and feature/buster, please reassign to me and I'll remove the set_origin_base_branch_head step from the Jenkins job: this branch does the equivalent itself via the $BASE_BRANCH_GIT_COMMIT it receives on the rake build command line, so we can drop this hack (git update-ref) from the Jenkins job.

Done and merged $(cat config/base_branch) into all active development branches, apart those in namespaces I am told not to touch. Let's see how it goes.

#19 Updated by intrigeri 3 months ago

  • QA Check set to Ready for QA

r

#20 Updated by anonym 3 months ago

intrigeri wrote:

anonym, I see you've merged this only in stable; I assume it's a mistake and I've thus merged stable → devel → feature/buster so I can do the next step of the work here.

Gah, yes I forgot to push my devel as usual, sorry about that!

(So far I have assumed that feature/buster is just a normal feature branch with devel as base branch so I didn't even consider treating it specially -- am I missing something? Asking to prevent future mistakes.)

#21 Updated by intrigeri 3 months ago

(So far I have assumed that feature/buster is just a normal feature branch with devel as base branch so I didn't even consider treating it specially -- am I missing something? Asking to prevent future mistakes.)

Indeed, you're right :)

#22 Updated by intrigeri 3 months ago

  • Status changed from In Progress to Fix committed
  • Assignee deleted (intrigeri)
  • % Done changed from 70 to 100
  • QA Check changed from Ready for QA to Pass

#23 Updated by intrigeri about 2 months ago

  • Status changed from Fix committed to Resolved
  • Target version changed from Tails_3.15 to Tails_3.14.1

Also available in: Atom PDF