Opened 11 months ago
Closed 11 months ago
#29104 closed task (fixed)
Rebase Tor Browser patches against 60.5.0esr
Reported by: | gk | Owned by: | tbb-team |
---|---|---|---|
Priority: | Very High | Milestone: | |
Component: | Applications/Tor Browser | Version: | |
Severity: | Normal | Keywords: | TorBrowserTeam201901R, GeorgKoppen201901 |
Cc: | mcs, brade | Actual Points: | |
Parent ID: | Points: | ||
Reviewer: | Sponsor: |
Description
I make a ticket this time for the rebase for the upcoming release because I like to have someone review the result. The reason for that is that Mozilla reformatted the whole ESR60 tree to adhere to Google's coding style (see: bug 1513900) and we now have a ton of fallout from that.
Child Tickets
Change History (5)
comment:1 Changed 11 months ago by
comment:2 Changed 11 months ago by
Keywords: | TorBrowserTeam201901R added; TorBrowserTeam201901 removed |
---|---|
Status: | new → needs_review |
Replying to mcs:
I did not realize that the reformatting was applied to the ESR code, but it make sense from Mozilla's perspective to do so. What is the status of automated tools to help with the rebase (after the reformatting)? I know Mozilla created some tools for hg and git, but I did not follow the conversation closely on their dev-platform list.
That's been a good point which I had forgotten about.
https://groups.google.com/forum/#!topic/mozilla.dev.platform/kQ0q29dlmJQ
is relevant here and contains two tools for Git. I used https://github.com/emilio/clang-format-merge which I had to patch: ./mach clang-format
did not work for me (not sure if that's expected for esr60), instead I made sure that the wrapper hit cat "$1" | clang-format "-assume-filename=$REPO_PATH" > "$1.tmp"
(needs to have clang-format
installed).
After that the rewriting mostly worked. There are five patches (IIRC) which I needed to adapt manually. bug_29104_rebase_6050esr_v2
(https://gitweb.torproject.org/user/gk/tor-browser.git/log/?h=bug_29104_rebase_6050esr_v2) in my tor-browser
repo is the result. Let me know if that makes sense (it's not the final rebase for the release but I plan to use it for comparison to make sure the final rebase is correct, both for the alpha and the stable branch).
I additionally filed #29141 to track a clean-up of our search engine list patch.
comment:3 Changed 11 months ago by
Cc: | mcs brade added |
---|
tor-browser-60.5.0esr-8.0-1
(https://gitweb.torproject.org/user/gk/tor-browser.git/log/?h=tor-browser-60.5.0esr-8.0-1) in my public tor-browser
repo is what could be the branch for the 8.0.5 stable.
Assuming you have git-wrapper
in your path you should get it by doing git-wrapper rebase -i --autosquash 5305958d84a17c0203074504874350683476b8de
and git-wrapper rebase --continue
after each of the conflicts got resolved, starting from tor-browser.60.4.0esr-8.0-1
.
comment:4 follow-up: 5 Changed 11 months ago by
r=brade, r=mcs
Everything looks good to us. As a double-check against the work of the automated reformatting tools, we quickly compared each of the rebased patches against each of the tor-browser-60.4.0esr-8.0-1
patches. The only thing we noticed that was a little surprising was that for some patches reformatting was done that was unrelated to the original patch, e.g., note the whitespace changes in the second of these two patches:
- 432f8e57e03a3be2d4e5932eb61a6b3b88168d14 (60.4.0esr patch for #18900)
- b81a07a586f7a2312645d35fd5a08b94fda919f5 (60.5.0esr patch for #18900)
Presumably slightly different clang-format
rules were used. It looks like the format in the rebased patches matches that used on mozilla-central
so it seems fine.
comment:5 Changed 11 months ago by
Resolution: | → fixed |
---|---|
Status: | needs_review → closed |
Replying to mcs:
r=brade, r=mcs
Everything looks good to us. As a double-check against the work of the automated reformatting tools, we quickly compared each of the rebased patches against each of thetor-browser-60.4.0esr-8.0-1
patches. The only thing we noticed that was a little surprising was that for some patches reformatting was done that was unrelated to the original patch, e.g., note the whitespace changes in the second of these two patches:
- 432f8e57e03a3be2d4e5932eb61a6b3b88168d14 (60.4.0esr patch for #18900)
- b81a07a586f7a2312645d35fd5a08b94fda919f5 (60.5.0esr patch for #18900)
Presumably slightly different
clang-format
rules were used. It looks like the format in the rebased patches matches that used onmozilla-central
so it seems fine.
Yeah, I think the whitespace changes are okay. I took the rebased 8.0 branch for 8.0.5 build1 and will use the 8.5 one as a baseline for the final branch for the alpha. I think we are done here, thanks!
I did not realize that the reformatting was applied to the ESR code, but it make sense from Mozilla's perspective to do so. What is the status of automated tools to help with the rebase (after the reformatting)? I know Mozilla created some tools for hg and git, but I did not follow the conversation closely on their dev-platform list.