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

Issue 1338453002: Fix NaCl IRT copy in GN chrome build. (Closed)

Created:
5 years, 3 months ago by Dirk Pranke
Modified:
5 years, 3 months ago
Reviewers:
Petr Hosek, brettw, jam
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix NaCl IRT copy in GN chrome build. When we build the IRT in GN, it gets built into out/*/irt_x64/nacl_irt.nexe, but it needs to be copied to out/*/nacl_irt_x86_64.nexe. There were two places in the GN build that depended on the IRT; the chrome dependency only depended on the toolchain-specific location, and the browser_tests dependency only worked if the nacl browsertests are enabled (which they currently aren't, by default). As a result, we may not get the IRT build into the right location, and the 'nacl_integration' build step might fail (though it might succeed if it happened to find an old binary lying around). Fixing the chrome dependency should cause the nacl_integration step to pass again. TBR=jam@chromium.org, phosek@chromium.org, brettw@chromium.org BUG=529988 Committed: https://crrev.com/4ba62625fd00601ce4e806f8e1bacdda893bf24d Cr-Commit-Position: refs/heads/master@{#348226}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -4 lines) Patch
M components/nacl/BUILD.gn View 1 chunk +1 line, -4 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 8 (2 generated)
Dirk Pranke
5 years, 3 months ago (2015-09-10 20:02:00 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1338453002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1338453002/1
5 years, 3 months ago (2015-09-10 20:03:23 UTC) #4
jam
rubberstamp lgtm
5 years, 3 months ago (2015-09-10 20:09:35 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 3 months ago (2015-09-10 20:49:23 UTC) #6
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/4ba62625fd00601ce4e806f8e1bacdda893bf24d Cr-Commit-Position: refs/heads/master@{#348226}
5 years, 3 months ago (2015-09-10 20:50:05 UTC) #7
commit-bot: I haz the power
5 years, 3 months ago (2015-09-23 12:13:52 UTC) #8
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/4ba62625fd00601ce4e806f8e1bacdda893bf24d
Cr-Commit-Position: refs/heads/master@{#348226}

Powered by Google App Engine
This is Rietveld 408576698