Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
25698002:
(mostly) working wtf / wtf_unittests gn targets (Closed)
|
Created:
7 years, 2 months ago by jamesr Modified:
7 years, 2 months ago CC:
chromium-reviews, brettw, abarth-chromium Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
Description(mostly) working wtf / wtf_unittests gn targets
This adds BUILD.gn targets for Blink's 'wtf' library and the wtf_unittests
executable. They appear to mostly work (at least on mac) with many things
that I don't understand commented out, mostly for windows.
R=thakis
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=229538
Patch Set 1 #Patch Set 2 : #
Total comments: 17
Patch Set 3 : #Patch Set 4 : #
Total comments: 7
Patch Set 5 : formatting #
Messages
Total messages: 16 (0 generated)
This target semi-works. In a release+static build, wtf_unittests compiles and runs just fine on my mac. In a component build, it compiles and links fine but only if ran from the same directory that the binary is in. If running from a different directory I get errors like: $ ./out/gn/wtf_unittests dyld: Library not loaded: libwtf.dylib Referenced from: /Users/jamesr/chrome/src/./out/gn/wtf_unittests Reason: image not found Trace/BPT trap: 5 which is kind of silly since libwtf.dylib is sitting in out/gn I haven't tried this anywhere except mac yet https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/BUILD.gn File tools/gn/secondary/BUILD.gn (right): https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/BUILD.g... tools/gn/secondary/BUILD.gn:17: "//third_party/WebKit/Source/wtf", i assume i only have to add this because it's an isolated part of the dependency tree currently and we won't have to add every new directory with a BUILD.gn in here, right? https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/base/BU... File tools/gn/secondary/base/BUILD.gn (right): https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/base/BU... tools/gn/secondary/base/BUILD.gn:954: "//base/third_party/dynamic_annotations", on mac, at least, this is needed or _AnnotateHappensBefore (and probably other symbols) are undefined in targets that link against test_support_base https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/testing... File tools/gn/secondary/testing/BUILD.gn (right): https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/testing... tools/gn/secondary/testing/BUILD.gn:52: configs += "//build/config/compiler:no_chromium_code" this is a really annoying stanza to state repeatedly https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... File tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn (right): https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:15: # 'msvs_disabled_warnings': [4291], what does msvs_disabled_warnings map to? https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:21: # if (gcc_version >= 46) { do we have a way to query this? i tried to read about the toolchain stuff but didn't see anything obvious https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:269: # '$(SDKROOT)/System/Library/Frameworks/CoreFoundation.framework', i dunno what this does, so i dunno what it's supposed to map to. Nico? https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:296: # "<(SHARED_INTERMEDIATE_DIR)/blink', <(SHARED_INTERMEDIATE_DIR) is replaced with rebase_path("something", ...), right? https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:298: if (is_component_build) { whoops, no idea why this is here https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:370: # if (is_linux && use_tcmalloc) { it looks like we don't have the tcmalloc targets set up yet, but when we do will it be controlled by a buildargs?
On Wed, Oct 2, 2013 at 2:07 AM, <jamesr@chromium.org> wrote: > Reviewers: thakis, > > Message: > This target semi-works. In a release+static build, wtf_unittests compiles > and > runs just fine on my mac. In a component build, it compiles and links > fine but > only if ran from the same directory that the binary is in. If running > from a > different directory I get errors like: > > $ ./out/gn/wtf_unittests > dyld: Library not loaded: libwtf.dylib > Referenced from: /Users/jamesr/chrome/src/./**out/gn/wtf_unittests > Reason: image not found > Trace/BPT trap: 5 > > which is kind of silly since libwtf.dylib is sitting in out/gn > I'll look at the CL now, but this error is caused by your generated binary not having the right LC_ID_DYLIB entry. The easiest way to look at this is with `otool -L out/Release/wtf_unittests`, you want the executable to link to @rpath/libwtf.dylib (note the "@rpath" bit). gyp gets this right by emulating LD_DYLIB_INSTALL_NAME and DYLIB_INSTALL_NAME_BASE which build/common.gypi sets. (Linux has a similar concept with rpaths, but maybe that's already implemented in gn.) In the end, all these settings result in a '-install_name foo' when building the _dylib_ (to see the LC_ID_DYLIB entry in the dylib, run something like `otool -l out/Release/libwtf.dylib | grep LC_ID_DYLIB -A 5`). Just "@rpath/sharedlibraryname.dylib" isn't always right though, it's not for shared libraries in a bundle for example. And there are a few build actions that change the install name after the fact via install_name_tool in build actions (see cs.chromium.org if curious). > > I haven't tried this anywhere except mac yet > > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/BUILD.gn<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/BUILD.gn> > File tools/gn/secondary/BUILD.gn (right): > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/BUILD.gn#**newcode17<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/BUILD.gn#newcode17> > tools/gn/secondary/BUILD.gn:**17: "//third_party/WebKit/Source/**wtf", > i assume i only have to add this because it's an isolated part of the > dependency tree currently and we won't have to add every new directory > with a BUILD.gn in here, right? > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/base/BUILD.gn<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/base/BUILD.gn> > File tools/gn/secondary/base/BUILD.**gn (right): > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/base/BUILD.gn#**newcode954<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/base/BUILD.gn#newcode954> > tools/gn/secondary/base/BUILD.**gn:954: > "//base/third_party/dynamic_**annotations", > on mac, at least, this is needed or _AnnotateHappensBefore (and probably > other symbols) are undefined in targets that link against > test_support_base > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/testing/BUILD.gn<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/testing/BUILD.gn> > File tools/gn/secondary/testing/**BUILD.gn (right): > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/testing/BUILD.gn#**newcode52<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/testing/BUILD.gn#newcode52> > tools/gn/secondary/testing/**BUILD.gn:52: configs += > "//build/config/compiler:no_**chromium_code" > this is a really annoying stanza to state repeatedly > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/third_party/**WebKit/Source/wtf/BUILD.gn<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn> > File tools/gn/secondary/third_**party/WebKit/Source/wtf/BUILD.**gn > (right): > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/third_party/**WebKit/Source/wtf/BUILD.gn#**newcode15<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn#newcode15> > tools/gn/secondary/third_**party/WebKit/Source/wtf/BUILD.**gn:15: # > 'msvs_disabled_warnings': [4291], > what does msvs_disabled_warnings map to? > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/third_party/**WebKit/Source/wtf/BUILD.gn#**newcode21<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn#newcode21> > tools/gn/secondary/third_**party/WebKit/Source/wtf/BUILD.**gn:21: # if > (gcc_version >= 46) { > do we have a way to query this? i tried to read about the toolchain > stuff but didn't see anything obvious > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/third_party/**WebKit/Source/wtf/BUILD.gn#**newcode269<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn#newcode269> > tools/gn/secondary/third_**party/WebKit/Source/wtf/BUILD.**gn:269: # > '$(SDKROOT)/System/Library/**Frameworks/CoreFoundation.**framework', > i dunno what this does, so i dunno what it's supposed to map to. Nico? > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/third_party/**WebKit/Source/wtf/BUILD.gn#**newcode296<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn#newcode296> > tools/gn/secondary/third_**party/WebKit/Source/wtf/BUILD.**gn:296: # > "<(SHARED_INTERMEDIATE_DIR)/**blink', > <(SHARED_INTERMEDIATE_DIR) is replaced with rebase_path("something", > ...), right? > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/third_party/**WebKit/Source/wtf/BUILD.gn#**newcode298<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn#newcode298> > tools/gn/secondary/third_**party/WebKit/Source/wtf/BUILD.**gn:298: if > (is_component_build) { > whoops, no idea why this is here > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/third_party/**WebKit/Source/wtf/BUILD.gn#**newcode370<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn#newcode370> > tools/gn/secondary/third_**party/WebKit/Source/wtf/BUILD.**gn:370: # if > (is_linux && use_tcmalloc) { > it looks like we don't have the tcmalloc targets set up yet, but when we > do will it be controlled by a buildargs? > > Description: > (mostly) working wtf / wtf_unittests gn targets > > This adds BUILD.gn targets for Blink's 'wtf' library and the wtf_unittests > executable. They appear to mostly work (at least on mac) with many things > that I don't understand commented out, mostly for windows. > > R=thakis > > Please review this at https://codereview.chromium.**org/25698002/<https://codereview.chromium.org/2... > > SVN Base: svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src> > > Affected files (+390, -1 lines): > M tools/gn/secondary/BUILD.gn > M tools/gn/secondary/base/BUILD.**gn > M tools/gn/secondary/testing/**BUILD.gn > A tools/gn/secondary/third_**party/WebKit/Source/wtf/BUILD.**gn > M tools/gn/secondary/third_**party/libxml/BUILD.gn > > > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
I looked at this, but I don't know gn well enough yet to answer some of your questions so Brett will have to look too I'm afraid. https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/base/BU... File tools/gn/secondary/base/BUILD.gn (right): https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/base/BU... tools/gn/secondary/base/BUILD.gn:954: "//base/third_party/dynamic_annotations", On 2013/10/02 09:07:02, jamesr wrote: > on mac, at least, this is needed or _AnnotateHappensBefore (and probably other > symbols) are undefined in targets that link against test_support_base This dependency exists in the gyp test_support_base target too, so I suppose this is correct :-) https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... File tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn (right): https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:15: # 'msvs_disabled_warnings': [4291], On 2013/10/02 09:07:02, jamesr wrote: > what does msvs_disabled_warnings map to? winja gyp translates it to /wd4291 in the compiler flags. Not sure if we just want to add that to the cflags if is_win, seems like something that should be keyed off the toolchain instead (so that we can later build chrome/android on win or use clang on windows etc)? Or does "is_win" mean "is targeting win", not "is host win" in gn? https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:21: # if (gcc_version >= 46) { On 2013/10/02 09:07:02, jamesr wrote: > do we have a way to query this? i tried to read about the toolchain stuff but > didn't see anything obvious Brett? https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:46: ":wtf-config" Is the order here important? You put ":wtf-config" at the front of the configs list in a few places, and at the end in others. https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:313: ":wtf-config" indent https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:335: ":wtf-config" indent https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... File tools/gn/secondary/third_party/libxml/BUILD.gn (right): https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... tools/gn/secondary/third_party/libxml/BUILD.gn:159: # http://www.xmlsoft.org/threads.html says that this is required Looking at that page, it seems to no longer say that, "starting with 2.4.7". third_party/libxml/README.chromium claims we're at 2.7.7.
PTAL. This has everything needed to build+run a wtf_unittests binary on mac. I'd expect it to work fine on linux as well, but it'll probably require mucking with MSVS settings and I won't be able to do that until I'm in front of a windows box sometime next week. I still think this is useful to have as a starting point for other targets. Parts that I don't understand are commented out in the BUILD.gn file.
Sweet, thanks! https://codereview.chromium.org/25698002/diff/14001/tools/gn/secondary/third_... File tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn (right): https://codereview.chromium.org/25698002/diff/14001/tools/gn/secondary/third_... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:59: sources = [ I've been putting sources at the top of the component (above the configs) https://codereview.chromium.org/25698002/diff/14001/tools/gn/secondary/third_... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:297: if (is_component_build) { Remove this? https://codereview.chromium.org/25698002/diff/14001/tools/gn/secondary/third_... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:311: ":wtf-config" Indenting. https://codereview.chromium.org/25698002/diff/14001/tools/gn/secondary/third_... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:323: "//testing:gtest_config" I'd put a comma here, or use the one-line format with no last comma. https://codereview.chromium.org/25698002/diff/14001/tools/gn/secondary/third_... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:326: sources = [ Sources first. https://codereview.chromium.org/25698002/diff/14001/tools/gn/secondary/third_... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:334: ":wtf-config" Indenting. https://codereview.chromium.org/25698002/diff/14001/tools/gn/secondary/third_... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:343: sources = [ Sources first.
lgtm
Brett, did you see my question to you on this 2 weeks ago? On Fri, Oct 18, 2013 at 10:23 AM, <brettw@chromium.org> wrote: > lgtm > > > > https://codereview.chromium.**org/25698002/<https://codereview.chromium.org/2... > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
Oh, about GCC version? If we need that we can easily write a .gni file that runs the existing compiler version script.
On 2013/10/02 15:56:30, Nico wrote: > https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... > File tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn (right): > > https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... > tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:15: # > 'msvs_disabled_warnings': [4291], > On 2013/10/02 09:07:02, jamesr wrote: > > what does msvs_disabled_warnings map to? > > winja gyp translates it to /wd4291 in the compiler flags. Not sure if we just > want to add that to the cflags if is_win, seems like something that should be > keyed off the toolchain instead (so that we can later build chrome/android on > win or use clang on windows etc)? > > Or does "is_win" mean "is targeting win", not "is host win" in gn? I just left these commented out for now - will address when I get in front of a windows box. > > https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... > tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:21: # if (gcc_version > >= 46) { > On 2013/10/02 09:07:02, jamesr wrote: > > do we have a way to query this? i tried to read about the toolchain stuff but > > didn't see anything obvious > > Brett? AFAICT we only use gcc on linux and passing this flags to gccs older than 4.6 isn't a problem, so I just made this unconditional on linux. If it's wrong I can fix it up. > > https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... > tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:46: ":wtf-config" > Is the order here important? You put ":wtf-config" at the front of the configs > list in a few places, and at the end in others. I just sorted these everywhere. I don't think the order is important but consistency is nice. Everything else fixed.
https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... File tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn (right): https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:15: # 'msvs_disabled_warnings': [4291], Remember that this is executed multiple times, once for each toolchain. So the current state refers to the current toolchain. If you're on Windows and cross-compiling to ARM, is_win will be set on the host build, and not on the target build.
On Fri, Oct 18, 2013 at 10:31 AM, <jamesr@chromium.org> wrote: > On 2013/10/02 15:56:30, Nico wrote: > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/third_party/**WebKit/Source/wtf/BUILD.gn<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn> > >> File tools/gn/secondary/third_**party/WebKit/Source/wtf/BUILD.**gn >> (right): >> > > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/third_party/**WebKit/Source/wtf/BUILD.gn#**newcode15<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn#newcode15> > >> tools/gn/secondary/third_**party/WebKit/Source/wtf/BUILD.**gn:15: # >> 'msvs_disabled_warnings': [4291], >> On 2013/10/02 09:07:02, jamesr wrote: >> > what does msvs_disabled_warnings map to? >> > > winja gyp translates it to /wd4291 in the compiler flags. Not sure if we >> just >> want to add that to the cflags if is_win, seems like something that >> should be >> keyed off the toolchain instead (so that we can later build >> chrome/android on >> win or use clang on windows etc)? >> > > Or does "is_win" mean "is targeting win", not "is host win" in gn? >> > > I just left these commented out for now - will address when I get in front > of a > windows box. > > > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/third_party/**WebKit/Source/wtf/BUILD.gn#**newcode21<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn#newcode21> > >> tools/gn/secondary/third_**party/WebKit/Source/wtf/BUILD.**gn:21: # if >> > (gcc_version > >> >= 46) { >> On 2013/10/02 09:07:02, jamesr wrote: >> > do we have a way to query this? i tried to read about the toolchain >> stuff >> > but > >> > didn't see anything obvious >> > > Brett? >> > > AFAICT we only use gcc on linux and passing this flags to gccs older than > 4.6 > isn't a problem, so I just made this unconditional on linux. If it's > wrong I > can fix it up. > That's fine, but we'll likely need to add new flags in 4.8+ in some places, so the question still remains (just doesn't apply to this cl) > > > > https://codereview.chromium.**org/25698002/diff/3001/tools/** > gn/secondary/third_party/**WebKit/Source/wtf/BUILD.gn#**newcode46<https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn#newcode46> > >> tools/gn/secondary/third_**party/WebKit/Source/wtf/BUILD.**gn:46: >> ":wtf-config" >> Is the order here important? You put ":wtf-config" at the front of the >> configs >> list in a few places, and at the end in others. >> > > I just sorted these everywhere. I don't think the order is important but > consistency is nice. > > > Everything else fixed. > > https://codereview.chromium.**org/25698002/<https://codereview.chromium.org/2... > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2013/10/18 17:38:54, brettw wrote: > https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... > File tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn (right): > > https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... > tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:15: # > 'msvs_disabled_warnings': [4291], > Remember that this is executed multiple times, once for each toolchain. So the > current state refers to the current toolchain. If you're on Windows and > cross-compiling to ARM, is_win will be set on the host build, and not on the > target build. Hmm, so should this be based off of testing current_toolchain?
No, just do "if (is_win) ..." On Fri, Oct 18, 2013 at 10:41 AM, <jamesr@chromium.org> wrote: > On 2013/10/18 17:38:54, brettw wrote: > > https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... >> >> File tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn (right): > > > > https://codereview.chromium.org/25698002/diff/3001/tools/gn/secondary/third_p... >> >> tools/gn/secondary/third_party/WebKit/Source/wtf/BUILD.gn:15: # >> 'msvs_disabled_warnings': [4291], >> Remember that this is executed multiple times, once for each toolchain. So >> the >> current state refers to the current toolchain. If you're on Windows and >> cross-compiling to ARM, is_win will be set on the host build, and not on >> the >> target build. > > > Hmm, so should this be based off of testing current_toolchain? > > https://codereview.chromium.org/25698002/ To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamesr@chromium.org/25698002/77001
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jamesr@chromium.org/25698002/77001
Message was sent while issue was closed.
Change committed as 229538 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
