Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2548403003:
Add missing //extensions/ deps for app_shim. (Closed)
|
Created:
4 years ago by tapted Modified:
4 years ago Reviewers:
benwells CC:
chromium-reviews, extensions-reviews_chromium.org, Matt Giuca, tapted, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, chromium-apps-reviews_chromium.org, darin (slow to review), qsr+mojo_chromium.org, chrome-apps-syd-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd missing //extensions/ deps for app_shim.
This currently causes build flakes the first time an output directory
is built, since it depends transitively on a generated header. Ninja is
smart enough add a dependency to the generated header once the generated
file exists, but doesn't do that on the initial build.
Although the errors blame other files, chrome/browser/apps/app_shim/'s
extension_app_shim_handler_mac.cc includes files from extensions/common.
The source set should have this dependency. This fixes the problem.
There are other missing dependencies, such as the path of files named in
the compile error. However, they can't be added without creating a
dependency loop, since its the missing dependencies that depend on these
targets via a source_set.
BUG=665694
Committed: https://crrev.com/6ee456f20d1987d8dfa75f1c431722111eb4ca2a
Cr-Commit-Position: refs/heads/master@{#436540}
Patch Set 1 #Patch Set 2 : rebase for r436385 #
Messages
Total messages: 24 (16 generated)
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
tapted@chromium.org changed reviewers: + benwells@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by tapted@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Add missing //extensions/ deps for app_shim and extensions/browser/mojo. These currently cause build flakes the first time an output dirrectory is built, since they depend transitively on a generated header. Ninja is smart enough add a dependency to the generated header once the generated file exists, but doesn't do that on the initial build. Although the errors blame other files, extensions/browser/mojo/'s service_registration.cc and chrome/browser/apps/app_shim/'s extension_app_shim_handler_mac.cc include files from extensions/common. Their source sets should have this dependency. This fixes the problem. There are other missing dependencies, such as the path of files named in the compile error. However, they can't be added without creating a dependency loop, since its the missing dependencies that depend on these targets via a source_set. BUG=665694 ========== to ========== Add missing //extensions/ deps for app_shim. This currently causes build flakes the first time an output dirrectory is built, since it depends transitively on a generated header. Ninja is smart enough add a dependency to the generated header once the generated file exists, but doesn't do that on the initial build. Although the errors blame other files, chrome/browser/apps/app_shim/'s extension_app_shim_handler_mac.cc includes files from extensions/common. The source set should have this dependency. This fixes the problem. There are other missing dependencies, such as the path of files named in the compile error. However, they can't be added without creating a dependency loop, since its the missing dependencies that depend on these targets via a source_set. BUG=665694 ==========
Hi ben, PTAL. r436385 removed //extensions/browser/mojo so this is now just to fix app_shim
lgtm
Description was changed from ========== Add missing //extensions/ deps for app_shim. This currently causes build flakes the first time an output dirrectory is built, since it depends transitively on a generated header. Ninja is smart enough add a dependency to the generated header once the generated file exists, but doesn't do that on the initial build. Although the errors blame other files, chrome/browser/apps/app_shim/'s extension_app_shim_handler_mac.cc includes files from extensions/common. The source set should have this dependency. This fixes the problem. There are other missing dependencies, such as the path of files named in the compile error. However, they can't be added without creating a dependency loop, since its the missing dependencies that depend on these targets via a source_set. BUG=665694 ========== to ========== Add missing //extensions/ deps for app_shim. This currently causes build flakes the first time an output directory is built, since it depends transitively on a generated header. Ninja is smart enough add a dependency to the generated header once the generated file exists, but doesn't do that on the initial build. Although the errors blame other files, chrome/browser/apps/app_shim/'s extension_app_shim_handler_mac.cc includes files from extensions/common. The source set should have this dependency. This fixes the problem. There are other missing dependencies, such as the path of files named in the compile error. However, they can't be added without creating a dependency loop, since its the missing dependencies that depend on these targets via a source_set. BUG=665694 ==========
The CQ bit was unchecked by tapted@chromium.org
The CQ bit was checked by tapted@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by tapted@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481002925266720,
"parent_rev": "d7bc9fd7fc352700925d930a65724d1d57a577c5", "commit_rev":
"476d3ea4c51470edd8223f01d13c40683db353ad"}
Message was sent while issue was closed.
Description was changed from ========== Add missing //extensions/ deps for app_shim. This currently causes build flakes the first time an output directory is built, since it depends transitively on a generated header. Ninja is smart enough add a dependency to the generated header once the generated file exists, but doesn't do that on the initial build. Although the errors blame other files, chrome/browser/apps/app_shim/'s extension_app_shim_handler_mac.cc includes files from extensions/common. The source set should have this dependency. This fixes the problem. There are other missing dependencies, such as the path of files named in the compile error. However, they can't be added without creating a dependency loop, since its the missing dependencies that depend on these targets via a source_set. BUG=665694 ========== to ========== Add missing //extensions/ deps for app_shim. This currently causes build flakes the first time an output directory is built, since it depends transitively on a generated header. Ninja is smart enough add a dependency to the generated header once the generated file exists, but doesn't do that on the initial build. Although the errors blame other files, chrome/browser/apps/app_shim/'s extension_app_shim_handler_mac.cc includes files from extensions/common. The source set should have this dependency. This fixes the problem. There are other missing dependencies, such as the path of files named in the compile error. However, they can't be added without creating a dependency loop, since its the missing dependencies that depend on these targets via a source_set. BUG=665694 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Add missing //extensions/ deps for app_shim. This currently causes build flakes the first time an output directory is built, since it depends transitively on a generated header. Ninja is smart enough add a dependency to the generated header once the generated file exists, but doesn't do that on the initial build. Although the errors blame other files, chrome/browser/apps/app_shim/'s extension_app_shim_handler_mac.cc includes files from extensions/common. The source set should have this dependency. This fixes the problem. There are other missing dependencies, such as the path of files named in the compile error. However, they can't be added without creating a dependency loop, since its the missing dependencies that depend on these targets via a source_set. BUG=665694 ========== to ========== Add missing //extensions/ deps for app_shim. This currently causes build flakes the first time an output directory is built, since it depends transitively on a generated header. Ninja is smart enough add a dependency to the generated header once the generated file exists, but doesn't do that on the initial build. Although the errors blame other files, chrome/browser/apps/app_shim/'s extension_app_shim_handler_mac.cc includes files from extensions/common. The source set should have this dependency. This fixes the problem. There are other missing dependencies, such as the path of files named in the compile error. However, they can't be added without creating a dependency loop, since its the missing dependencies that depend on these targets via a source_set. BUG=665694 Committed: https://crrev.com/6ee456f20d1987d8dfa75f1c431722111eb4ca2a Cr-Commit-Position: refs/heads/master@{#436540} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/6ee456f20d1987d8dfa75f1c431722111eb4ca2a Cr-Commit-Position: refs/heads/master@{#436540} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
