Tails

#1 Updated by anonym 2019-02-21 15:35:36

• Status changed from Confirmed to In Progress

Applied in changeset commit:tails|df8a338ea6e078113ef6a03d7e3fbb3ffc7467d1.

#2 Updated by anonym 2019-02-21 15:52:28

• Assignee deleted (anonym)
• Target version deleted (Tails_3.13)
• % Done changed from 0 to 50
• QA Check set to Ready for QA
• Feature Branch set to bugfix/16476-clone-submodules-locally

Fixed, and while I was at it I managed to optimize our Git usage a bit as well.

@intrigeri, I’ll count this as FT work, but I’m highlighting you in case you have a better idea.

The rationale for me doing this is that I personally have wasted an hour at two separate occasions from this type of build failure when I have worked on submodules. But I think it’s even worse than that. IMHO, this is a bug: so far we have failed to exactly replicate the Git state from the host into the builder VM. I bet this problem will bite (and waste the time of) contributors working on submodules in Tails, and the workaround to push to the submodules’ repos isn’t obvious. That workaround won’t work for contributor’s without push rights to the submodules repos, so they will have to go even further to be able to build and test their work (e.g. they can update their .gitmodules to point to a repo they control in a temporary commit… not very obvious and quite a lot of work).

#3 Updated by anonym 2019-02-21 15:53:04

• blocks Feature #15507: Core work 2019Q1: Foundations Team added

#4 Updated by intrigeri 2019-02-21 16:15:18

• Assignee set to intrigeri

#5 Updated by intrigeri 2019-02-22 07:07:59

• Assignee changed from intrigeri to anonym
• QA Check changed from Ready for QA to Dev Needed

This branch FTBFS on Jenkins: at first glance, it seems these changes break the “mergebasebranch” feature.

#7 Updated by anonym 2019-02-22 11:11:10

• % Done changed from 50 to 60
• QA Check changed from Dev Needed to Ready for QA

anonym wrote:
> On it! I actually tried mergebasebranch successfully before I pushed, but I think all that is missing it a git fetch origin "$(cat config/base_branch)".

For some reason that doesn’t work on Jenkins (works fine for me locally though). Whatever, I’ll just revert the fanciness I’m trying with only fetching the exact refs we need which would shave ~30 seconds off the build time. KISS!

Also, I worried about whether this branch might have some strange effects when building a release from a tag, but I managed to reproduce Tails 3.12.1 (I imported the changes into tails-build without commiting and built with ignorechanges) so it looks good!

Let’s see what Jenkins thinks: https://jenkins.tails.boum.org/job/build_Tails_ISO_bugfix-16476-clone-submodules-locally/4/

#8 Updated by anonym 2019-02-22 16:26:18

• Assignee changed from anonym to intrigeri
• % Done changed from 60 to 70

Success!

commit:d910948ef2f141a7469f7675dec1e29c06ec3712 is a bit of a shame, though, since a full fetch takes around 30 seconds. Do you have a quick answer (i.e. don’t investigate) why git fetch origin $(cat config/base_branch) fails on Jenkins?

#9 Updated by intrigeri 2019-02-22 16:51:20

• Assignee changed from intrigeri to anonym

> Do you have a quick answer (i.e. don’t investigate) why git fetch origin $(cat config/base_branch) fails on Jenkins?

I don’t remember if the tweaks we do to change the meaning of “origin” and/or for mapping remote refs to local refs is 100% in tails.git or if there are other, special bits in Jenkins too (puppet-tails.git, jenkins-jobs.git). I suspect there’s Jenkins-specific stuff somewhere in those repos.

#10 Updated by anonym 2019-02-25 10:09:58

• Assignee changed from anonym to intrigeri

intrigeri wrote:
> > Do you have a quick answer (i.e. don’t investigate) why git fetch origin $(cat config/base_branch) fails on Jenkins?
>
> I don’t remember if the tweaks we do to change the meaning of “origin” and/or for mapping remote refs to local refs is 100% in tails.git or if there are other, special bits in Jenkins too (puppet-tails.git, jenkins-jobs.git). I suspect there’s Jenkins-specific stuff somewhere in those repos.

Thanks! I had a look but quickly determined that I would have to RTFM way more than I am willing for a non-critical time optimization, which was only a bonus as far as this ticket/branch is concerned. Let’s skip the git fetch optimization! Please review’n’merge!

#14 Updated by anonym 2019-02-28 20:40:34

• Assignee changed from intrigeri to anonym
• QA Check changed from Ready for QA to Dev Needed

Actually, I want to spend some more time thinking about various scenarios before I push a time bomb that will cause mass-breakage on Jenkins some time soon.

I have a specific scenario in mind: using mergebasebranches when the base branch has bumped submodules. I just want to make sure that the new submodule commits actually are present in our submodule clones, or if we might have to do some extra fetching.

#16 Updated by anonym 2019-03-01 10:44:01

• Assignee changed from anonym to intrigeri
• QA Check changed from Dev Needed to Ready for QA
• Feature Branch changed from bugfix/16476-clone-submodules-locally to bugfix/16476-clone-submodules-locally-optimized

anonym wrote:
> Actually, I want to spend some more time thinking about various scenarios before I push a time bomb that will cause mass-breakage on Jenkins some time soon.
>
> I have a specific scenario in mind: using mergebasebranches when the base branch has bumped submodules. I just want to make sure that the new submodule commits actually are present in our submodule clones, or if we might have to do some extra fetching.

I tested this without issue, and haven’t been able to come up with any other potentially problematic scenarios. Let’s see what you can find!

intrigeri wrote:
> Also, I would suggest recommending — in contribute/git — developers to set the push.recurseSubmodules=check Git pref, to avoid issues like we’ve seen already recently (one forgets to push required commits from submodules).

That’s great! I just set it myself, and added the above recommendation.

#17 Updated by intrigeri 2019-03-04 11:58:58

• Target version set to Tails_3.13

#21 Updated by intrigeri 2019-03-04 13:40:00

• Assignee changed from intrigeri to anonym
• QA Check changed from Ready for QA to Dev Needed

anonym, see above comments & questions. Apart of that, code review passes. I’ve triggered a build on Jenkins after updated the stable branch, which will trigger mergebasebranch and tell us how this code behaves when the branch lags behind its base branch. I’ll monitor that build and will report back if anything breaks.

#23 Updated by anonym 2019-03-05 14:27:31

• Assignee changed from anonym to intrigeri
• Target version deleted (Tails_3.13)
• QA Check changed from Dev Needed to Ready for QA

intrigeri wrote:
> anonym wrote:
> > But I think it’s even worse than that. IMHO, this is a bug: so far we have failed to exactly replicate the Git state from the host into the builder VM.
>
> anonym I’m not sure I understand the “worse than that” correctly. Indeed, in the failure mode this ticket was originally about (FTBFS when the tails.git branch references a submodule commit that’s not been pushed), we fail to replicate the Git state but then we fail loudly, so at least it’s obvious that something is wrong. What exactly is “worse than that”?

Edit: I misread you first so might first answer (i.e. what’s in your email notification) might not make sense. Let me just avoid any confusion by answering this question:

> Is there any case where we would successfully build but using the wrong code?

No, running git submodule update under set -e ensures that we won’t ever be able to successfully build with the submodules in the wrong state, so there is/was never any risk for that. Just forget about the “worse” part. :)

> I guess it’s safe to drop the —tags Git fetch option because $GIT_REF is built from git_current_head_name, which returns the tag name when building from a tag. I assume git fetch origin 3.42 will forcefully replace an existing 3.42 tag that exists locally. Correct?

I tested, and actually no. I also RTFM:ed and think what we’d need is: git fetch origin --force tag ${REF} (so a special case vs when $REF is a branch).

> For background, see a6aa7504de1c3c139c660d299ec08349d8973229.

Indeed that used to be an RM annoyance before the rework of our build system we did during the reproducibility adventure. Before the rework the builder VM was never shutdown automatically so you would reuse it for the next build, and that is what enabled the annoyance to happen. But since the rework we shut down the VM on success, so I haven’t seen this issue ever since.

I definitely could implement the fix mentioned above if you think it is useful, though! Personally I think it isn’t worth it since it would only benefit those playing with TAILS_BUILD_OPTIONS=keeprunning (or rescue) and retagging, and it would “magically” be fixed after a rake vm:destroy, i.e. the advertised remedy for all problems. :)

> I can’t parse this sentence: “We work around this by fetching the ref that the host’s origin (most likely the official Tails repo) where the base branch refs must exist and be as up-to-date as possible”.

Sorry, fixed in commit:8ac09db5fc (s/that/from/).

#25 Updated by intrigeri 2019-03-05 14:53:53

• Target version set to Tails_3.13

(I don’t want this to remain in a WIP state for too long; let’s get this into 3.13 so we can switch to other matters.)

#26 Updated by anonym 2019-03-05 17:25:54

• Target version deleted (Tails_3.13)

intrigeri wrote:
> anonym, I’m now even more confused :)

Let me guess, you only read the email notification? I edited the message 1 minute after posting it because I realized I was confused, so please look at the online version! :)

#27 Updated by intrigeri 2019-03-06 06:52:37

• Target version set to Tails_3.13

(Same rationale as last time.)

#29 Updated by intrigeri 2019-03-06 07:50:28

• Assignee changed from intrigeri to anonym

> But since the rework we shut down the VM on success, so I haven’t seen this issue ever since.

@anonym, my understanding is that you can’t possibly have seen this issue, precisely because the command we have will overwrite a previously existing tag, which was the whole point of the aforementioned commit. So I’m not sure what relevant conclusion we can draw from “I haven’t seen this issue ever since”. Anyways, this does not matter much because:

> I definitely could implement the fix mentioned above if you think it is useful, though! Personally I think it isn’t worth it since it would only benefit those playing with TAILS_BUILD_OPTIONS=keeprunning (or rescue) and retagging, and it would “magically” be fixed after a rake vm:destroy, i.e. the advertised remedy for all problems. :)

I’m not a big fan of adding yet another potential failure mode in the release process, which I understand this branch does; the fact there’s a good chance the RM discovers the fix won’t make them much happier once they’ve stumbled upon this regression. This being said, I agree it’s probably not worth reworking the new code again (we’ve probably spent more than enough time on this already and I suspect we initially underestimated the cost/benefit of fixing this issue), so I’ve pushed a commit to the release process to clarify what a RM must do to avoid meeting this problem.

Please review & merge into stable → devel :)

#30 Updated by anonym 2019-03-06 10:36:13

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

Applied in changeset commit:tails|365ade2ecc5f915cd74c5e97cb8557b42d0df56d.

#31 Updated by anonym 2019-03-06 10:58:01

• Status changed from Fix committed to In Progress
• Assignee changed from anonym to intrigeri
• % Done changed from 100 to 90

intrigeri wrote:
> > But since the rework we shut down the VM on success, so I haven’t seen this issue ever since.
> >
> anonym, my understanding is that you can’t possibly have seen this issue, precisely because the command we have will overwrite a previously existing tag, which was the whole point of the aforementioned commit. So I’m not sure what relevant conclusion we can draw from “I haven’t seen this issue ever since”.

Eh, yeah that argument is just incorrect. :S

> Anyways, this does not matter much because:
>
> > I definitely could implement the fix mentioned above if you think it is useful, though! Personally I think it isn’t worth it since it would only benefit those playing with TAILS_BUILD_OPTIONS=keeprunning (or rescue) and retagging, and it would “magically” be fixed after a rake vm:destroy, i.e. the advertised remedy for all problems. :)
>
> I’m not a big fan of adding yet another potential failure mode in the release process, which I understand this branch does; the fact there’s a good chance the RM discovers the fix won’t make them much happier once they’ve stumbled upon this regression. This being said, I agree it’s probably not worth reworking the new code again (we’ve probably spent more than enough time on this already and I suspect we initially underestimated the cost/benefit of fixing this issue), so I’ve pushed a commit to the release process to clarify what a RM must do to avoid meeting this problem.
>
> Please review & merge into stable → devel :)

Merged!

But at least today the sound of “adding yet another potential failure mode in the release process” makes me more uneasy than when I thought about this last. So I pushed yet another commit fixing this. If you indeed feel like you’ve reached your limit with this ticket, feel free to ignore it and just close this ticket (and I’ll manually kill the branch since it won’t get garbage collected thanks to the extra commits), it was barely 10 minutes of hacking/testing so whatever. :)

#32 Updated by intrigeri 2019-03-06 11:32:10

• Status changed from In Progress to Fix committed

> Merged!

:)

> But at least today the sound of “adding yet another potential failure mode in the release process” makes me more uneasy than when I thought about this last. So I pushed yet another commit fixing this. If you indeed feel like you’ve reached your limit with this ticket, feel free to ignore it and just close this ticket

Indeed, I’ve reached the limit of the time/energy I think this is worth.

> (and I’ll manually kill the branch since it won’t get garbage collected thanks to the extra commits), it was barely 10 minutes of hacking/testing so whatever. :)

It looks like you did not actually push your branch so there’s no clean up to do.

#33 Updated by intrigeri 2019-03-06 11:39:20

• Assignee deleted (intrigeri)
• % Done changed from 90 to 100
• QA Check changed from Ready for QA to Pass

#34 Updated by CyrilBrulebois 2019-03-20 14:25:50

• Status changed from Fix committed to Resolved

#35 Updated by intrigeri 2019-04-03 09:44:52

• related to Bug #16607: FTBFS with "repository '/amnesia.git/.git/modules/submodules/aufs4-standalone' does not exist" added

#36 Updated by intrigeri 2019-05-19 05:10:48

• related to Bug #16730: Reproducible build CI job uses the HEAD of the current branch instead of the commit the 1st build was built from added