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

Issue 1154213004: Revert of Fix use of 'sysroot' variable in harfbuzz.gyp (Closed)

Created:
5 years, 7 months ago by spang
Modified:
5 years, 7 months ago
CC:
chromium-reviews, feature-media-reviews_chromium.org, cbentzel+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Fix use of 'sysroot' variable in harfbuzz.gyp (patchset #6 id:100001 of https://codereview.chromium.org/1151753002/) Reason for revert: Breaks GYP for Chrome OS builds. Repro steps: cros chrome-sdk --board=amd64-generic gclient runhooks Updating projects from gyp files... /bin/sh: /build/amd64-generic/build/bin/pkg-config: No such file or directory gyp: Call to '/build/amd64-generic/build/bin/pkg-config --libs-only-l nss | sed -e "s/-lssl3//"' returned exit status 0. /bin/sh: /build/amd64-generic/build/bin/pkg-config: No such file or directory gyp: Call to '/build/amd64-generic/build/bin/pkg-config --libs libcras' returned exit status 127. /bin/sh: /build/amd64-generic/build/bin/pkg-config: No such file or directory gyp: Call to '/build/amd64-generic/build/bin/pkg-config --cflags x11' returned exit status 127. Error: Command /usr/bin/python src/build/gyp_chromium returned non-zero exit status 1 in /ssd/src/chromium TEST=cros chrome-sdk --board=amd64-generic && gclient runhooks Original issue's description: > Fix use of 'sysroot' variable in harfbuzz.gyp > > The sysroot variable is set conditionally in common.gypi > but not at the nesting level required for other conditional > to depend on it like the one in hardbuzz does. This means > that sysroot always appeared to be empty to harfbuzz.gyp, > and hence the special handling for ARM linux was required. > > This change moves the setting of 'sysroot' up one level of > nesting and defines 'pkg-config' in common.gypi so it can > be shared. > > Committed: https://crrev.com/6a80b32f309490f44347f2925589a6f0fc23c3ff > Cr-Commit-Position: refs/heads/master@{#331506} TBR=mazda@chromium.org,dpranke@chromium.org,cpu@chromium.org,sbc@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Committed: https://crrev.com/8505ef356013ec9239f295501a283ec9fd43b08a Cr-Commit-Position: refs/heads/master@{#331642}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+88 lines, -34 lines) Patch
M build/common.gypi View 6 chunks +23 lines, -34 lines 0 comments Download
M build/linux/system.gyp View 1 chunk +8 lines, -0 lines 0 comments Download
M chrome/chrome_browser_chromeos.gypi View 1 chunk +7 lines, -0 lines 0 comments Download
M media/media.gyp View 1 chunk +5 lines, -0 lines 0 comments Download
M net/third_party/nss/ssl.gyp View 1 chunk +16 lines, -0 lines 0 comments Download
M third_party/harfbuzz-ng/harfbuzz.gyp View 2 chunks +18 lines, -0 lines 0 comments Download
M third_party/libexif/libexif.gyp View 1 chunk +11 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
spang
Created Revert of Fix use of 'sysroot' variable in harfbuzz.gyp
5 years, 7 months ago (2015-05-27 20:40:31 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1154213004/1
5 years, 7 months ago (2015-05-27 20:41:36 UTC) #2
Sam Clegg
On 2015/05/27 20:41:36, commit-bot: I haz the power wrote: > CQ is trying da patch. ...
5 years, 7 months ago (2015-05-27 20:43:13 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 7 months ago (2015-05-27 20:43:36 UTC) #4
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/8505ef356013ec9239f295501a283ec9fd43b08a Cr-Commit-Position: refs/heads/master@{#331642}
5 years, 7 months ago (2015-05-27 20:44:20 UTC) #5
spang
5 years, 7 months ago (2015-05-27 20:44:22 UTC) #6
Message was sent while issue was closed.
On 2015/05/27 20:43:13, Sam Clegg wrote:
> On 2015/05/27 20:41:36, commit-bot: I haz the power wrote:
> > CQ is trying da patch. Follow status at
> >  https://chromium-cq-status.appspot.com/patch-status/1154213004/1
> 
> lgtm.
> 
> Is there a trybot I can use to test this config in future?

There will be: https://code.google.com/p/chromium/issues/detail?id=489795

Powered by Google App Engine
This is Rietveld 408576698