Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(2)

Issue 124003004: Correct gyp toolsets for all combinations of want_separate_host_toolset and use_system_icu. (Closed)

Created:
5 years, 11 months ago by dshwang
Modified:
5 years, 11 months ago
Base URL:
https://chromium.googlesource.com/chromium/deps/icu46.git@separateOld
Visibility:
Public.

Description

Correct gyp toolsets for all combinations of want_separate_host_toolset and use_system_icu. This CL ensures following logics: if want_separate_host_toolset == 0 and use_system_icu == 0 don't use system icu build icu for target toolset elif want_separate_host_toolset == 0 and use_system_icu == 1 use system icu for target toolset don't build icu elif want_separate_host_toolset == 1 and use_system_icu == 0 don't use system icu build icu for host and target elif want_separate_host_toolset == 1 and use_system_icu == 1 use system icu for target toolset build icu for host toolset All complexity is due to the condition: want_separate_host_toolset == 1 and use_system_icu == 1. Although use_system_icu == 1, this condition requires to build icu for host toolset. BUG= R=jochen@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=244134

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -38 lines) Patch
M icu.gyp View 10 chunks +27 lines, -38 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
dshwang
5 years, 11 months ago (2014-01-03 20:22:36 UTC) #1
jochen (gone - plz use gerrit)
generally looks good can you verify that this builds on the various configurations? android_aosp has ...
5 years, 11 months ago (2014-01-07 10:16:02 UTC) #2
dshwang
On 2014/01/07 10:16:02, jochen wrote: > generally looks good > > can you verify that ...
5 years, 11 months ago (2014-01-09 11:50:20 UTC) #3
jochen (gone - plz use gerrit)
lgtm
5 years, 11 months ago (2014-01-09 11:52:42 UTC) #4
dshwang
On 2014/01/09 11:52:42, jochen wrote: > lgtm Thank you for lgtm. I guess there is ...
5 years, 11 months ago (2014-01-09 16:10:46 UTC) #5
inactive_dshwang_plz_cc_intel
Committed patchset #1 manually as r00f025a (presubmit successful).
5 years, 11 months ago (2014-01-10 09:23:44 UTC) #6
dshwang
On 2014/01/10 09:23:44, dongseong.hwang wrote: > Committed patchset #1 manually as r00f025a (presubmit successful). I ...
5 years, 11 months ago (2014-01-10 09:28:46 UTC) #7
jochen (gone - plz use gerrit)
Committed patchset #1 manually as r244134 (presubmit successful).
5 years, 11 months ago (2014-01-10 09:36:19 UTC) #8
jochen (gone - plz use gerrit)
On 2014/01/10 09:36:19, jochen wrote: > Committed patchset #1 manually as r244134 (presubmit successful). I ...
5 years, 11 months ago (2014-01-10 09:36:49 UTC) #9
jochen (gone - plz use gerrit)
can you post a CL to update the chromium deps?
5 years, 11 months ago (2014-01-10 09:37:10 UTC) #10
dshwang
On 2014/01/10 09:37:10, jochen wrote: > can you post a CL to update the chromium ...
5 years, 11 months ago (2014-01-10 10:19:24 UTC) #11
dshwang
On 2014/01/10 10:19:24, dshwang wrote: > On 2014/01/10 09:37:10, jochen wrote: > > can you ...
5 years, 11 months ago (2014-01-10 10:23:08 UTC) #12
jochen (gone - plz use gerrit)
5 years, 11 months ago (2014-01-10 10:38:51 UTC) #13
Message was sent while issue was closed.
On 2014/01/10 10:23:08, dshwang wrote:
> On 2014/01/10 10:19:24, dshwang wrote:
> > On 2014/01/10 09:37:10, jochen wrote:
> > > can you post a CL to update the chromium deps?
> > 
> > Thank you! yes, I'll do it.
> 
> I see the your commit. If the commit has something like 'authored by
dongseong',
> it would be better.
> I'm fine, no problem.

yeah, I tried to add that, but somehow it failed :-/

sorry about that

Powered by Google App Engine
This is Rietveld 408576698