Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1934083002:
[Cronet] Use gn desc deps to find third_party licenses. (Closed)
Description[Cronet] Use gn desc deps to find third_party licenses.
BUG=606859
Committed: https://crrev.com/c3208e7d54e2d6369ab68cb8580ea97ab5ff5ce5
Cr-Commit-Position: refs/heads/master@{#395716}
Patch Set 1 #Patch Set 2 : Add components/cronet/ios/BUILD.gn #
Total comments: 10
Patch Set 3 : Make gn deps work from script. #Patch Set 4 : Address Brett's comments. #
Total comments: 14
Patch Set 5 : Sync #Patch Set 6 : Sync from Air #Patch Set 7 : Address Brett's comments. #Patch Set 8 : Generate gn project in temp directory. #
Total comments: 9
Patch Set 9 : Sync #Patch Set 10 : Address Andrei's comments. #
Total comments: 2
Patch Set 11 : Sync, add better comment. #Patch Set 12 : Add direct gn dependencies. #Patch Set 13 : Fix build from gyp project. #
Messages
Total messages: 22 (5 generated)
mef@chromium.org changed reviewers: + brettw@chromium.org
Hi Brett, Could you take a look? I'm trying to use gn desc deps to get list of Cronet dependencies and combine their license files. I've got 2 questions ATM: 1. What's a good way to inform gn about existence of components/cronet/ios/BUILD.gn? I've added a new group to components/BUILD.gn, but I wonder whether there is a better place. 2. Running 'gn desc outdir net deps' from src folder works fine, however running it from gn script launches it from outdir folder, and I'm getting an error '//outdir/net:net' not found. What's a good way to get it working? Also, what's a good place to ask gn questions like these?
On 2016/05/03 16:05:04, mef wrote: > Hi Brett, > > Could you take a look? I'm trying to use gn desc deps to get list of Cronet > dependencies and combine their license files. > > I've got 2 questions ATM: > > 1. What's a good way to inform gn about existence of > components/cronet/ios/BUILD.gn? I've added a new group to components/BUILD.gn, > but I wonder whether there is a better place. > > 2. Running 'gn desc outdir net deps' from src folder works fine, however running > it from gn script launches it from outdir folder, and I'm getting an error > '//outdir/net:net' not found. What's a good way to get it working? Thet "net" part here is a GN label. Like "//base" or something. Since you're not starting it with a "//" it's getting resolved relative to the current directory (just like a normal label in GN would). If you just wrote "//net" then this problem will go away.
On 2016/05/03 17:04:07, brettw wrote: > On 2016/05/03 16:05:04, mef wrote: > > Hi Brett, > > > > Could you take a look? I'm trying to use gn desc deps to get list of Cronet > > dependencies and combine their license files. > > > > I've got 2 questions ATM: > > > > 1. What's a good way to inform gn about existence of > > components/cronet/ios/BUILD.gn? I've added a new group to components/BUILD.gn, > > but I wonder whether there is a better place. > > > > 2. Running 'gn desc outdir net deps' from src folder works fine, however > running > > it from gn script launches it from outdir folder, and I'm getting an error > > '//outdir/net:net' not found. What's a good way to get it working? > > Thet "net" part here is a GN label. Like "//base" or something. Since you're not > starting it with a "//" it's getting resolved relative to the current directory > (just like a normal label in GN would). If you just wrote "//net" then this > problem will go away. Great, that worked, thanks a lot!
https://codereview.chromium.org/1934083002/diff/20001/components/cronet/ios/B... File components/cronet/ios/BUILD.gn (right): https://codereview.chromium.org/1934083002/diff/20001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:27: static_library("cronet_static") { This should probably be a source set. If you need a static library for some reason, it should have a comment about why. https://codereview.chromium.org/1934083002/diff/20001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:131: rebase_path(_license_path, root_build_dir), Here you're running GN on the same build directory from inside the build. We shouldn't be doing this because running GN has side effects. Some scripts and such write stuff to the build directory as a side effect of running, which can cause flakes and dependency issues if this build is currently being run from that directory. You should actually make a temporary directory to run GN in again. You will want your script to copy <builddir>/args.gn to it to duplicate the exact setup of the current one. https://codereview.chromium.org/1934083002/diff/20001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:133: # "--gn_out_dir=" + rebase_path(root_out_dir, root_build_dir), We should delete this or fix it. https://codereview.chromium.org/1934083002/diff/20001/components/cronet/tools... File components/cronet/tools/cronet_licenses.py (right): https://codereview.chromium.org/1934083002/diff/20001/components/cronet/tools... components/cronet/tools/cronet_licenses.py:70: third_party_deps.append(build_dep.replace("/BUILD.gn", "")) The BUILD file in a third party directory isn't necessarily in the root. It's true most of them are, but there are some counter examples. The only BUILD directory in third_party/widevine is third_party/widevine/cdm/BUILD.gn, for example. Second, asking for the build file names will be incorrect when they use the secondary directory. For example: gn desc out/Default //third_party/libjpeg_turbo:libjpeg deps --as=buildfile gives /home/brettw/src/build/secondary/third_party/libjpeg_turbo/BUILD.gn (you can see the "build/secondary" thing in there) which won't be the directory where the license resides. I think you probably want to take the full label GN outputs and convert those to directories yourself.
Thanks, PTAL! https://codereview.chromium.org/1934083002/diff/20001/components/cronet/ios/B... File components/cronet/ios/BUILD.gn (right): https://codereview.chromium.org/1934083002/diff/20001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:27: static_library("cronet_static") { On 2016/05/03 17:59:00, brettw wrote: > This should probably be a source set. If you need a static library for some > reason, it should have a comment about why. Done. https://codereview.chromium.org/1934083002/diff/20001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:131: rebase_path(_license_path, root_build_dir), On 2016/05/03 17:59:00, brettw wrote: > Here you're running GN on the same build directory from inside the build. We > shouldn't be doing this because running GN has side effects. Some scripts and > such write stuff to the build directory as a side effect of running, which can > cause flakes and dependency issues if this build is currently being run from > that directory. > > You should actually make a temporary directory to run GN in again. You will want > your script to copy <builddir>/args.gn to it to duplicate the exact setup of the > current one. Does it mean that the cronet_licenses.py script would should create directory, run gn gen on that directory, run 'gn desc' in that directory, and then delete it? I can do it, but that seems like inefficient way to find third party dependencies. https://codereview.chromium.org/1934083002/diff/20001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:133: # "--gn_out_dir=" + rebase_path(root_out_dir, root_build_dir), On 2016/05/03 17:58:59, brettw wrote: > We should delete this or fix it. Done. https://codereview.chromium.org/1934083002/diff/20001/components/cronet/tools... File components/cronet/tools/cronet_licenses.py (right): https://codereview.chromium.org/1934083002/diff/20001/components/cronet/tools... components/cronet/tools/cronet_licenses.py:70: third_party_deps.append(build_dep.replace("/BUILD.gn", "")) On 2016/05/03 17:59:00, brettw wrote: > The BUILD file in a third party directory isn't necessarily in the root. It's > true most of them are, but there are some counter examples. The only BUILD > directory in third_party/widevine is third_party/widevine/cdm/BUILD.gn, for > example. > > Second, asking for the build file names will be incorrect when they use the > secondary directory. For example: > gn desc out/Default //third_party/libjpeg_turbo:libjpeg deps --as=buildfile > gives > /home/brettw/src/build/secondary/third_party/libjpeg_turbo/BUILD.gn > (you can see the "build/secondary" thing in there) which won't be the directory > where the license resides. I think you probably want to take the full label GN > outputs and convert those to directories yourself. Argh, that's unfortunate! Luckily Cronet's set of dependencies is pretty limited and missing licenses should trigger an error inside of licenses.ParseDir with require_license_file=True. WDYT?
https://codereview.chromium.org/1934083002/diff/20001/components/cronet/ios/B... File components/cronet/ios/BUILD.gn (right): https://codereview.chromium.org/1934083002/diff/20001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:131: rebase_path(_license_path, root_build_dir), On 2016/05/03 21:50:20, mef wrote: > Does it mean that the cronet_licenses.py script would should create directory, > run gn gen on that directory, run 'gn desc' in that directory, and then delete > it? I can do it, but that seems like inefficient way to find third party > dependencies. Yes. An alternative would be to check in the licenses and have a test that verifies that the license file matches the gn desc output. https://codereview.chromium.org/1934083002/diff/60001/components/BUILD.gn File components/BUILD.gn (right): https://codereview.chromium.org/1934083002/diff/60001/components/BUILD.gn#new... components/BUILD.gn:21: "//components/cronet/ios:cronet_package", I'd actually put this in the root build file. You can add it to the condition in both_gn_and_gyp or something (whatever is appropriate, shouldn't need a new target). https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... File components/cronet/ios/BUILD.gn (right): https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:6: import("//build/util/version.gni") You can delete this when you change the version processing below. https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:11: assert(!is_component_build, "Cronet requires static library build.") The link you have going into this file should match this condition or the builds will fail. https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:23: "VERSION_FULL=\"$chrome_version_full\"", I don't like chrome_version_full and we should only use this when necessary. See components/data_reduction_proxy/core/common:version_header for how to express this without depending on data being read from the version file at GN-time. https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:30: static_library("cronet_static") { source_set. https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:140: "--gn_out_dir=" + rebase_path(root_build_dir), This is a bit suspicious because it will always be the absolute path to the scripts working directory. It seems pointless to pass this in. https://codereview.chromium.org/1934083002/diff/60001/components/cronet/tools... File components/cronet/tools/cr_cronet.py (right): https://codereview.chromium.org/1934083002/diff/60001/components/cronet/tools... components/cronet/tools/cr_cronet.py:102: out_dir = 'out/Release' + out_dir_suffix We should pick a different directory name to avoid clobbering the user's build. This is actually really bad for several reasons: - Currently you can have GYP and GN side-by-side. If you run GN build, this will corrupt your GYP build to the point you need to delete the directory to erase all sources of flakiness. - If the user is using GN and put their build in one of these locations, this code will delete their build configuration and replace it with the args specified here. We should be making a temporary directory that doesn't exist before.
mef@chromium.org changed reviewers: + kapishnikov@chromium.org
PTAL. https://codereview.chromium.org/1934083002/diff/20001/components/cronet/ios/B... File components/cronet/ios/BUILD.gn (right): https://codereview.chromium.org/1934083002/diff/20001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:131: rebase_path(_license_path, root_build_dir), On 2016/05/03 22:13:03, brettw wrote: > On 2016/05/03 21:50:20, mef wrote: > > Does it mean that the cronet_licenses.py script would should create directory, > > run gn gen on that directory, run 'gn desc' in that directory, and then delete > > it? I can do it, but that seems like inefficient way to find third party > > dependencies. > > Yes. An alternative would be to check in the licenses and have a test that > verifies that the license file matches the gn desc output. I see. That's a terrible alternative. I've reworked the script to do gn gen / gn desc in temp directory. Done. https://codereview.chromium.org/1934083002/diff/60001/components/BUILD.gn File components/BUILD.gn (right): https://codereview.chromium.org/1934083002/diff/60001/components/BUILD.gn#new... components/BUILD.gn:21: "//components/cronet/ios:cronet_package", On 2016/05/03 22:13:03, brettw wrote: > I'd actually put this in the root build file. You can add it to the condition in > both_gn_and_gyp or something (whatever is appropriate, shouldn't need a new > target). Done. https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... File components/cronet/ios/BUILD.gn (right): https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:6: import("//build/util/version.gni") On 2016/05/03 22:13:03, brettw wrote: > You can delete this when you change the version processing below. Hrm, process_version is undefined without it. https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:11: assert(!is_component_build, "Cronet requires static library build.") On 2016/05/03 22:13:03, brettw wrote: > The link you have going into this file should match this condition or the builds > will fail. Done. https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:23: "VERSION_FULL=\"$chrome_version_full\"", On 2016/05/03 22:13:03, brettw wrote: > I don't like chrome_version_full and we should only use this when necessary. See > components/data_reduction_proxy/core/common:version_header for how to express > this without depending on data being read from the version file at GN-time. Done. https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:30: static_library("cronet_static") { On 2016/05/03 22:13:03, brettw wrote: > source_set. Done. https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... components/cronet/ios/BUILD.gn:140: "--gn_out_dir=" + rebase_path(root_build_dir), On 2016/05/03 22:13:03, brettw wrote: > This is a bit suspicious because it will always be the absolute path to the > scripts working directory. It seems pointless to pass this in. Done. https://codereview.chromium.org/1934083002/diff/60001/components/cronet/tools... File components/cronet/tools/cr_cronet.py (right): https://codereview.chromium.org/1934083002/diff/60001/components/cronet/tools... components/cronet/tools/cr_cronet.py:102: out_dir = 'out/Release' + out_dir_suffix On 2016/05/03 22:13:03, brettw wrote: > We should pick a different directory name to avoid clobbering the user's build. > This is actually really bad for several reasons: > > - Currently you can have GYP and GN side-by-side. If you run GN build, this > will corrupt your GYP build to the point you need to delete the directory to > erase all sources of flakiness. > > - If the user is using GN and put their build in one of these locations, this > code will delete their build configuration and replace it with the args > specified here. > > We should be making a temporary directory that doesn't exist before. Done.
On 2016/05/11 15:57:28, mef wrote: > PTAL. > > https://codereview.chromium.org/1934083002/diff/20001/components/cronet/ios/B... > File components/cronet/ios/BUILD.gn (right): > > https://codereview.chromium.org/1934083002/diff/20001/components/cronet/ios/B... > components/cronet/ios/BUILD.gn:131: rebase_path(_license_path, root_build_dir), > On 2016/05/03 22:13:03, brettw wrote: > > On 2016/05/03 21:50:20, mef wrote: > > > Does it mean that the cronet_licenses.py script would should create > directory, > > > run gn gen on that directory, run 'gn desc' in that directory, and then > delete > > > it? I can do it, but that seems like inefficient way to find third party > > > dependencies. > > > > Yes. An alternative would be to check in the licenses and have a test that > > verifies that the license file matches the gn desc output. > > I see. That's a terrible alternative. I've reworked the script to do gn gen / gn > desc in temp directory. > Done. > > https://codereview.chromium.org/1934083002/diff/60001/components/BUILD.gn > File components/BUILD.gn (right): > > https://codereview.chromium.org/1934083002/diff/60001/components/BUILD.gn#new... > components/BUILD.gn:21: "//components/cronet/ios:cronet_package", > On 2016/05/03 22:13:03, brettw wrote: > > I'd actually put this in the root build file. You can add it to the condition > in > > both_gn_and_gyp or something (whatever is appropriate, shouldn't need a new > > target). > > Done. > > https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... > File components/cronet/ios/BUILD.gn (right): > > https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... > components/cronet/ios/BUILD.gn:6: import("//build/util/version.gni") > On 2016/05/03 22:13:03, brettw wrote: > > You can delete this when you change the version processing below. > > Hrm, process_version is undefined without it. > > https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... > components/cronet/ios/BUILD.gn:11: assert(!is_component_build, "Cronet requires > static library build.") > On 2016/05/03 22:13:03, brettw wrote: > > The link you have going into this file should match this condition or the > builds > > will fail. > > Done. > > https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... > components/cronet/ios/BUILD.gn:23: "VERSION_FULL=\"$chrome_version_full\"", > On 2016/05/03 22:13:03, brettw wrote: > > I don't like chrome_version_full and we should only use this when necessary. > See > > components/data_reduction_proxy/core/common:version_header for how to express > > this without depending on data being read from the version file at GN-time. > > Done. > > https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... > components/cronet/ios/BUILD.gn:30: static_library("cronet_static") { > On 2016/05/03 22:13:03, brettw wrote: > > source_set. > > Done. > > https://codereview.chromium.org/1934083002/diff/60001/components/cronet/ios/B... > components/cronet/ios/BUILD.gn:140: "--gn_out_dir=" + > rebase_path(root_build_dir), > On 2016/05/03 22:13:03, brettw wrote: > > This is a bit suspicious because it will always be the absolute path to the > > scripts working directory. It seems pointless to pass this in. > > Done. > > https://codereview.chromium.org/1934083002/diff/60001/components/cronet/tools... > File components/cronet/tools/cr_cronet.py (right): > > https://codereview.chromium.org/1934083002/diff/60001/components/cronet/tools... > components/cronet/tools/cr_cronet.py:102: out_dir = 'out/Release' + > out_dir_suffix > On 2016/05/03 22:13:03, brettw wrote: > > We should pick a different directory name to avoid clobbering the user's > build. > > This is actually really bad for several reasons: > > > > - Currently you can have GYP and GN side-by-side. If you run GN build, this > > will corrupt your GYP build to the point you need to delete the directory to > > erase all sources of flakiness. > > > > - If the user is using GN and put their build in one of these locations, > this > > code will delete their build configuration and replace it with the args > > specified here. > > > > We should be making a temporary directory that doesn't exist before. > > Done. Brett, friendly ping. Andrei, PTAL.
https://codereview.chromium.org/1934083002/diff/140001/components/cronet/ios/... File components/cronet/ios/BUILD.gn (right): https://codereview.chromium.org/1934083002/diff/140001/components/cronet/ios/... components/cronet/ios/BUILD.gn:53: } Should we add '-fvisibility=hidden' and '-fvisibility-inlines-hidden' C flags here or in the 'shared_library' target? https://codereview.chromium.org/1934083002/diff/140001/components/cronet/ios/... components/cronet/ios/BUILD.gn:55: shared_library("libcronet") { I have renamed the target to 'libcronet_shered' in GYP in order to avoid name conflicts with Cronet framework: see https://codereview.chromium.org/1985893002/ https://codereview.chromium.org/1934083002/diff/140001/components/cronet/ios/... components/cronet/ios/BUILD.gn:56: sources = [ "cronet_static" already lists these sources. Do we need them here? https://codereview.chromium.org/1934083002/diff/140001/components/cronet/ios/... components/cronet/ios/BUILD.gn:67: libs = [ "UIKit.Framework" ] I wonder why we need UIKit.Framework? Will Foundation.framework be enough?
Thanks, PTAL. https://codereview.chromium.org/1934083002/diff/140001/components/cronet/ios/... File components/cronet/ios/BUILD.gn (right): https://codereview.chromium.org/1934083002/diff/140001/components/cronet/ios/... components/cronet/ios/BUILD.gn:53: } On 2016/05/17 22:03:28, kapishnikov wrote: > Should we add '-fvisibility=hidden' and '-fvisibility-inlines-hidden' C flags > here or in the 'shared_library' target? Symbol visibility=hidden seems to be default:https://code.google.com/p/chromium/codesearch#chromium/src/build/config/gcc/BUILD.gn&l=18 https://codereview.chromium.org/1934083002/diff/140001/components/cronet/ios/... components/cronet/ios/BUILD.gn:55: shared_library("libcronet") { On 2016/05/17 22:03:28, kapishnikov wrote: > I have renamed the target to 'libcronet_shered' in GYP in order to avoid name > conflicts with Cronet framework: see https://codereview.chromium.org/1985893002/ Done. https://codereview.chromium.org/1934083002/diff/140001/components/cronet/ios/... components/cronet/ios/BUILD.gn:56: sources = [ On 2016/05/17 22:03:28, kapishnikov wrote: > "cronet_static" already lists these sources. Do we need them here? Done. https://codereview.chromium.org/1934083002/diff/140001/components/cronet/ios/... components/cronet/ios/BUILD.gn:67: libs = [ "UIKit.Framework" ] On 2016/05/17 22:03:28, kapishnikov wrote: > I wonder why we need UIKit.Framework? Will Foundation.framework be enough? I'm getting these errors without UIKit: Undefined symbols for architecture x86_64: "_OBJC_CLASS_$_UIApplication", referenced from: objc-class-ref in libbase.a(scoped_critical_action.o) "_OBJC_CLASS_$_UIDevice", referenced from: objc-class-ref in libbase.a(sys_info_ios.o) objc-class-ref in libbase.a(critical_closure_internal_ios.o) "_UIBackgroundTaskInvalid", referenced from: base::ios::ScopedCriticalAction::Core::EndBackgroundTask() in libbase.a(scoped_critical_action.o) base::ios::ScopedCriticalAction::Core::Core() in libbase.a(scoped_critical_action.o) base::ios::ScopedCriticalAction::Core::~Core() in libbase.a(scoped_critical_action.o)
LGTM https://codereview.chromium.org/1934083002/diff/140001/components/cronet/ios/... File components/cronet/ios/BUILD.gn (right): https://codereview.chromium.org/1934083002/diff/140001/components/cronet/ios/... components/cronet/ios/BUILD.gn:67: libs = [ "UIKit.Framework" ] On 2016/05/18 16:46:19, mef wrote: > On 2016/05/17 22:03:28, kapishnikov wrote: > > I wonder why we need UIKit.Framework? Will Foundation.framework be enough? > > I'm getting these errors without UIKit: > > Undefined symbols for architecture x86_64: > "_OBJC_CLASS_$_UIApplication", referenced from: > objc-class-ref in libbase.a(scoped_critical_action.o) > "_OBJC_CLASS_$_UIDevice", referenced from: > objc-class-ref in libbase.a(sys_info_ios.o) > objc-class-ref in libbase.a(critical_closure_internal_ios.o) > "_UIBackgroundTaskInvalid", referenced from: > base::ios::ScopedCriticalAction::Core::EndBackgroundTask() in > libbase.a(scoped_critical_action.o) > base::ios::ScopedCriticalAction::Core::Core() in > libbase.a(scoped_critical_action.o) > base::ios::ScopedCriticalAction::Core::~Core() in > libbase.a(scoped_critical_action.o) I see. It is used in https://code.google.com/p/chromium/codesearch#chromium/src/components/history... to extend CPU allocation when application goes into the background. Not sure if we need this class in Cronet.
Sorry for the delay, was on vacation. Will look shortly.
lgtm https://codereview.chromium.org/1934083002/diff/180001/components/cronet/tool... File components/cronet/tools/cronet_licenses.py (right): https://codereview.chromium.org/1934083002/diff/180001/components/cronet/tool... components/cronet/tools/cronet_licenses.py:68: # Generate gn project in temp directory and use it to find dependencies. Might be good to mention why we do the temp directory instead of the current one so somebody doesn't try to "fix" this later.
Thanks! https://codereview.chromium.org/1934083002/diff/180001/components/cronet/tool... File components/cronet/tools/cronet_licenses.py (right): https://codereview.chromium.org/1934083002/diff/180001/components/cronet/tool... components/cronet/tools/cronet_licenses.py:68: # Generate gn project in temp directory and use it to find dependencies. On 2016/05/23 20:05:33, brettw wrote: > Might be good to mention why we do the temp directory instead of the current one > so somebody doesn't try to "fix" this later. Done.
The CQ bit was checked by mef@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org, kapishnikov@chromium.org Link to the patchset: https://codereview.chromium.org/1934083002/#ps240001 (title: "Fix build from gyp project.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1934083002/240001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1934083002/240001
Message was sent while issue was closed.
Committed patchset #13 (id:240001)
Message was sent while issue was closed.
Description was changed from ========== [Cronet] Use gn desc deps to find third_party licenses. BUG=606859 ========== to ========== [Cronet] Use gn desc deps to find third_party licenses. BUG=606859 Committed: https://crrev.com/c3208e7d54e2d6369ab68cb8580ea97ab5ff5ce5 Cr-Commit-Position: refs/heads/master@{#395716} ==========
Message was sent while issue was closed.
Patchset 13 (id:??) landed as https://crrev.com/c3208e7d54e2d6369ab68cb8580ea97ab5ff5ce5 Cr-Commit-Position: refs/heads/master@{#395716} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
