|
|
Created:
4 years, 7 months ago by Michael Achenbach Modified:
4 years, 7 months ago Reviewers:
vogelheim CC:
v8-reviews_googlegroups.com, jochen (gone - plz use gerrit) Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[gn] Make gyp include dirs equal to gn
BUG=chromium:474921
LOG=n
Committed: https://crrev.com/64a518fa8e9c38742b94187c3c6eba3d8112a464
Cr-Commit-Position: refs/heads/master@{#36319}
Patch Set 1 #
Total comments: 1
Messages
Total messages: 15 (8 generated)
Description was changed from ========== [gn] Make gyp include dirs equal to gn BUG= ========== to ========== [gn] Make gyp include dirs equal to gn BUG=chromium:474921 LOG=n NOTRY=true ==========
Description was changed from ========== [gn] Make gyp include dirs equal to gn BUG=chromium:474921 LOG=n NOTRY=true ========== to ========== [gn] Make gyp include dirs equal to gn BUG=chromium:474921 LOG=n ==========
machenbach@chromium.org changed reviewers: + vogelheim@chromium.org
PTAL https://codereview.chromium.org/1989193002/diff/1/src/v8.gyp File src/v8.gyp (right): https://codereview.chromium.org/1989193002/diff/1/src/v8.gyp#newcode1912 src/v8.gyp:1912: '../include', This accounts for a semantic difference between public configs in gn and direct_dependent_settings in gyp. See description of direct_dependent_settings in gyp: https://gyp.gsrc.io/docs/UserDocumentation.md#Skeleton-of-a-typical-library-t... See description of public configs in gn: https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/referen... GN includes configs also on the target itself, while in gyp, configs are applied to the dependent targets only. In gn, the include dir is in the public configs of libplatform: https://code.google.com/p/chromium/codesearch#chromium/src/v8/BUILD.gn&l=115 While in gyp it's in direct_dependent_settings. See line 1930. This change won't have a visible effect on compilation, but it will silence a lot of differences in the gyp/gn diff tool.
The CQ bit was checked by machenbach@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1989193002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1989193002/1
lgtm Again, many thanks for explanation + references. I wouldn't have understood the change without. :)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by machenbach@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1989193002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1989193002/1
Message was sent while issue was closed.
Description was changed from ========== [gn] Make gyp include dirs equal to gn BUG=chromium:474921 LOG=n ========== to ========== [gn] Make gyp include dirs equal to gn BUG=chromium:474921 LOG=n ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== [gn] Make gyp include dirs equal to gn BUG=chromium:474921 LOG=n ========== to ========== [gn] Make gyp include dirs equal to gn BUG=chromium:474921 LOG=n Committed: https://crrev.com/64a518fa8e9c38742b94187c3c6eba3d8112a464 Cr-Commit-Position: refs/heads/master@{#36319} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/64a518fa8e9c38742b94187c3c6eba3d8112a464 Cr-Commit-Position: refs/heads/master@{#36319} |