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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1334253002: Fix running gyp with configuration_policy=0 (Closed)

Created:
5 years, 3 months ago by the_jk
Modified:
5 years, 2 months ago
CC:
chromium-reviews, chromoting-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix running gyp with configuration_policy=0 Running gyp in chromium with configuration_policy set to 0 fails as multiple targets that are behind condition of configuration_policy==1 are referenced without corresponding conditionals BUG=530579 Committed: https://crrev.com/a7b7b998391c399fdfbafb4df10645a0f2ce9c5b Cr-Commit-Position: refs/heads/master@{#348903}

Patch Set 1 #

Patch Set 2 : Fix for GN #

Total comments: 1

Patch Set 3 : Changed to dependency addition #

Patch Set 4 : History rewrite #

Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -11 lines) Patch
M chrome/chrome.gyp View 3 chunks +3 lines, -4 lines 0 comments Download
M components/components_tests.gyp View 2 chunks +12 lines, -2 lines 0 comments Download
M components/policy/core/browser/BUILD.gn View 1 1 chunk +3 lines, -1 line 0 comments Download
M components/policy/core/common/BUILD.gn View 1 1 chunk +3 lines, -1 line 0 comments Download
M remoting/host/BUILD.gn View 1 2 chunks +4 lines, -1 line 0 comments Download
M remoting/remoting_test.gypi View 1 2 4 chunks +10 lines, -2 lines 0 comments Download

Messages

Total messages: 14 (3 generated)
the_jk
5 years, 3 months ago (2015-09-11 12:50:10 UTC) #2
Mattias Nissler (ping if slow)
FWIW, the gyp changes LGTM - how about equivalent GN changes?
5 years, 3 months ago (2015-09-11 12:58:31 UTC) #3
the_jk
Now with GN changes
5 years, 3 months ago (2015-09-11 14:05:31 UTC) #4
Sergey Ulanov
lgtm
5 years, 3 months ago (2015-09-11 23:10:13 UTC) #5
blundell
//components lgtm https://codereview.chromium.org/1334253002/diff/20001/remoting/remoting_test.gypi File remoting/remoting_test.gypi (right): https://codereview.chromium.org/1334253002/diff/20001/remoting/remoting_test.gypi#newcode117 remoting/remoting_test.gypi:117: ['configuration_policy == 0', { Why did you ...
5 years, 3 months ago (2015-09-14 07:54:23 UTC) #6
the_jk
5 years, 3 months ago (2015-09-14 08:49:25 UTC) #7
jochen (gone - plz use gerrit)
lgtm
5 years, 3 months ago (2015-09-14 09:49:01 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1334253002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1334253002/60001
5 years, 3 months ago (2015-09-15 15:25:55 UTC) #11
commit-bot: I haz the power
Committed patchset #4 (id:60001)
5 years, 3 months ago (2015-09-15 16:15:04 UTC) #12
commit-bot: I haz the power
Patchset 4 (id:??) landed as https://crrev.com/a7b7b998391c399fdfbafb4df10645a0f2ce9c5b Cr-Commit-Position: refs/heads/master@{#348903}
5 years, 3 months ago (2015-09-15 16:15:42 UTC) #13
commit-bot: I haz the power
5 years, 2 months ago (2015-09-23 12:45:42 UTC) #14
Message was sent while issue was closed. 
Patchset 4 (id:??) landed as
https://crrev.com/a7b7b998391c399fdfbafb4df10645a0f2ce9c5b
Cr-Commit-Position: refs/heads/master@{#348903}

Powered by Google App Engine
This is Rietveld 408576698