Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(410)

Issue 2303293004: Revert of Remove unneeded split_count assignments (Closed)

Created:
4 years, 3 months ago by Nico
Modified:
4 years, 3 months ago
Reviewers:
brettw, brucedawson
CC:
chromium-reviews, blink-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Remove unneeded split_count assignments (patchset #4 id:60001 of https://codereview.chromium.org/2311493002/ ) Reason for revert: Due to this landing after my revert of Brett's change, it broke the world, e.g. https://build.chromium.org/p/chromium/builders/Android/builds/61684 . Since I'm not sure if the patch is quite right, I'll revert this too. Original issue's description: > Remove unneeded split_count assignments > > Refactoring of WebKit gn files has made splitting libraries less > necessary. This broke generating of build files. This change > removes the unneeded assignments. > > The error, in the generate_build_files step, was: > > C:\b\c\b\win_chrome\src\buildtools\win\gn.exe gen //out/Release --check > -> returned 1 > ERROR at //third_party/WebKit/Source/core/BUILD.gn:52:19: Assignment had no effect. > split_count = 5 > ^ > You set the variable "split_count" here and it was unused before it went > out of scope. > See //third_party/WebKit/public/BUILD.gn:53:5: which caused the file to be included. > "//third_party/WebKit/Source/core", > ^--------------------------------- > GN gen failed: 1 > > It first appeared in build 10455: > https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win/builds/10455/steps/generate_build_files/logs/stdio > > Suspect is crrev.com/2299143005 > > TBR=brettw@chromium.org > > Committed: https://crrev.com/f7d8febdf0166e2193b5e34e81b6b9dfb9ffd7c4 > Cr-Commit-Position: refs/heads/master@{#416441} TBR=brettw@chromium.org,brucedawson@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Committed: https://crrev.com/91f1772747088c07ba2ebd2afb8999d72b56bfaf Cr-Commit-Position: refs/heads/master@{#416443}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -10 lines) Patch
M third_party/WebKit/Source/core/BUILD.gn View 2 chunks +13 lines, -10 lines 0 comments Download

Messages

Total messages: 6 (2 generated)
Nico
Created Revert of Remove unneeded split_count assignments
4 years, 3 months ago (2016-09-03 04:09:09 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2303293004/1
4 years, 3 months ago (2016-09-03 04:09:17 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 3 months ago (2016-09-03 04:10:11 UTC) #4
commit-bot: I haz the power
4 years, 3 months ago (2016-09-03 04:12:25 UTC) #6
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/91f1772747088c07ba2ebd2afb8999d72b56bfaf
Cr-Commit-Position: refs/heads/master@{#416443}

Powered by Google App Engine
This is Rietveld 408576698