analyze passes the wrong out dir for iOS |
||||||||||||||
Issue descriptionOn the iOS try bots we see invocations like: python -u /b/build/slave/ios-simulator/build/src/tools/mb/mb.py analyze -m chromium.mac -b ios-simulator -v //out/Release /tmp/tmptvP4_W.json /tmp/tmp_S_NXD.json The -v //out/Release is of interest here, because on ios-simulator this directory should be //out/Debug-iphonesimulator, not //out/Release. This seems to come from this line: https://chromium.googlesource.com/chromium/tools/build/+/78632624a04f54c12996e8d7a80a027eb4d9d9db/scripts/slave/recipe_modules/filter/api.py#264 I have no idea how self.m.chromium.c.build_config_fs is supposed to be set, but it is not being set properly for iOS.
,
Dec 2 2016
,
Dec 8 2016
,
Dec 12 2016
Looking at the analyze step, I see that it creates the out/Release directory, creates the args.gn file with the values coming from the .json bot configuration, run gn gen and analyze step on that directory. https://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/builds/122334/steps/analyze/logs/stdio Writing """\ additional_target_cpus = [ "x86" ] goma_dir = "\$(goma_dir)" ios_enable_code_signing = false is_component_build = false is_debug = true target_cpu = "x64" target_os = "ios" use_goma = true """ to /b/build/slave/ios-simulator/build/src/out/Release/args.gn. /b/build/slave/ios-simulator/build/src/buildtools/mac/gn gen //out/Release --check Done. Made 3218 targets from 459 files in 6177ms /b/build/slave/ios-simulator/build/src/buildtools/mac/gn analyze //out/Release /tmp/tmpJTfg5I.json.gn /tmp/tmpOCXXNK.json.gn So even though the bot do not use the same naming convention for the directory as the one used when building (out/Debug-iphonesimulator), it correctly run analyze with the correct gn args. Marking as WontFix, as I think current behaviour is correct.
,
Dec 12 2016
@sdefresne - I think you're right that the current behavior works (i.e., analyze will return a correct result), but it's still confusing and we should probably fix it.
,
Dec 12 2016
,
Dec 22 2017
,
Dec 22 2017
Moved all Infra>Client>iOS bugs to Infra>Client>Chrome + OS-iOS.
,
Dec 24
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 28
,
Jan 2
,
Jan 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build/+/78f0d11bb0a3fe49ab12e37e55edb343187f251a commit 78f0d11bb0a3fe49ab12e37e55edb343187f251a Author: Garrett Beaty <gbeaty@chromium.org> Date: Wed Jan 02 19:47:31 2019 Update iOS builders to use same dir for generation and analysis. Currently, filter.analyze is run against the directory determined by the chromium config object which is not the same as the directory that the build files are generated to because the ios recipe module specifies the directory when calling chromium.mb_gen. While this doesn't cause any technical issues, it is confusing. The filter.analyze method has been updated to accept the build output directory as an argument and the ios module has been updated to pass the same value to both chromium.mb_gen and filter.analyze. Bug: 670521 Change-Id: I79429d4e7c71d635401673ae59bdabfafbc901a1 Reviewed-on: https://chromium-review.googlesource.com/c/1392599 Reviewed-by: Stephen Martinis <martiniss@chromium.org> Commit-Queue: Garrett Beaty <gbeaty@chromium.org> [modify] https://crrev.com/78f0d11bb0a3fe49ab12e37e55edb343187f251a/scripts/slave/recipes/ios/try.expected/additional_compile_targets.json [modify] https://crrev.com/78f0d11bb0a3fe49ab12e37e55edb343187f251a/scripts/slave/recipe_modules/ios/api.py [modify] https://crrev.com/78f0d11bb0a3fe49ab12e37e55edb343187f251a/scripts/slave/README.recipes.md [modify] https://crrev.com/78f0d11bb0a3fe49ab12e37e55edb343187f251a/scripts/slave/recipes/ios/try.expected/goma_compilation_failure.json [modify] https://crrev.com/78f0d11bb0a3fe49ab12e37e55edb343187f251a/scripts/slave/recipes/ios/try.expected/no_tests.json [modify] https://crrev.com/78f0d11bb0a3fe49ab12e37e55edb343187f251a/scripts/slave/recipe_modules/filter/api.py [modify] https://crrev.com/78f0d11bb0a3fe49ab12e37e55edb343187f251a/scripts/slave/recipes/ios/try.expected/icu_patch.json [modify] https://crrev.com/78f0d11bb0a3fe49ab12e37e55edb343187f251a/scripts/slave/recipes/ios/try.expected/gn.json [modify] https://crrev.com/78f0d11bb0a3fe49ab12e37e55edb343187f251a/scripts/slave/recipes/ios/try.expected/no_compilation.json [modify] https://crrev.com/78f0d11bb0a3fe49ab12e37e55edb343187f251a/scripts/slave/recipes/ios/try.expected/basic.json [modify] https://crrev.com/78f0d11bb0a3fe49ab12e37e55edb343187f251a/scripts/slave/recipes/ios/try.expected/parent.json
,
Jan 2
,
Jan 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build/+/f23137dbac11445e394376ea21362e867dd33f2b commit f23137dbac11445e394376ea21362e867dd33f2b Author: Garrett Beaty <gbeaty@chromium.org> Date: Wed Jan 02 21:10:05 2019 Revert "Update iOS builders to use same dir for generation and analysis." This reverts commit 78f0d11bb0a3fe49ab12e37e55edb343187f251a. Reason for revert: Broke ios-simulator CQ bots Original change's description: > Update iOS builders to use same dir for generation and analysis. > > Currently, filter.analyze is run against the directory determined by > the chromium config object which is not the same as the directory that > the build files are generated to because the ios recipe module > specifies the directory when calling chromium.mb_gen. While this > doesn't cause any technical issues, it is confusing. > > The filter.analyze method has been updated to accept the build output > directory as an argument and the ios module has been updated to pass > the same value to both chromium.mb_gen and filter.analyze. > > Bug: 670521 > Change-Id: I79429d4e7c71d635401673ae59bdabfafbc901a1 > Reviewed-on: https://chromium-review.googlesource.com/c/1392599 > Reviewed-by: Stephen Martinis <martiniss@chromium.org> > Commit-Queue: Garrett Beaty <gbeaty@chromium.org> TBR=martiniss@chromium.org,gbeaty@chromium.org Change-Id: Ib7b03f0218b51c6c9d67853c3296c49d6b8216ec No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 670521 Reviewed-on: https://chromium-review.googlesource.com/c/1392603 Reviewed-by: Garrett Beaty <gbeaty@chromium.org> Commit-Queue: Garrett Beaty <gbeaty@chromium.org> [modify] https://crrev.com/f23137dbac11445e394376ea21362e867dd33f2b/scripts/slave/recipes/ios/try.expected/additional_compile_targets.json [modify] https://crrev.com/f23137dbac11445e394376ea21362e867dd33f2b/scripts/slave/recipe_modules/ios/api.py [modify] https://crrev.com/f23137dbac11445e394376ea21362e867dd33f2b/scripts/slave/README.recipes.md [modify] https://crrev.com/f23137dbac11445e394376ea21362e867dd33f2b/scripts/slave/recipes/ios/try.expected/goma_compilation_failure.json [modify] https://crrev.com/f23137dbac11445e394376ea21362e867dd33f2b/scripts/slave/recipes/ios/try.expected/no_tests.json [modify] https://crrev.com/f23137dbac11445e394376ea21362e867dd33f2b/scripts/slave/recipe_modules/filter/api.py [modify] https://crrev.com/f23137dbac11445e394376ea21362e867dd33f2b/scripts/slave/recipes/ios/try.expected/icu_patch.json [modify] https://crrev.com/f23137dbac11445e394376ea21362e867dd33f2b/scripts/slave/recipes/ios/try.expected/gn.json [modify] https://crrev.com/f23137dbac11445e394376ea21362e867dd33f2b/scripts/slave/recipes/ios/try.expected/no_compilation.json [modify] https://crrev.com/f23137dbac11445e394376ea21362e867dd33f2b/scripts/slave/recipes/ios/try.expected/basic.json [modify] https://crrev.com/f23137dbac11445e394376ea21362e867dd33f2b/scripts/slave/recipes/ios/try.expected/parent.json
,
Jan 2
The change to update the out directory used in analyze caused failures if the analyze step needed to perform compilation, e.g. https://ci.chromium.org/p/chromium/builders/luci.chromium.try/ios-simulator/165845
,
Jan 2
Running "mb analyze" calls "gn gen", which results in the args.gn being modified so that goma_dir = "\$(goma_dir)" instead of the prior value (/b/s/w/ir/cache/goma/client) provided to "mb gen".
,
Yesterday
(26 hours ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build/+/c761a1f1a387f1e5bf3a7c76509510cd45dbd664 commit c761a1f1a387f1e5bf3a7c76509510cd45dbd664 Author: Garrett Beaty <gbeaty@chromium.org> Date: Wed Jan 23 02:24:28 2019 Update output directory used during analysis for iOS builders. Currently, filter.analyze is run against the directory determined by the chromium config object, which is not the same as the directory that the build files are generated to because the ios recipe module specifies the directory when calling chromium.mb_gen. While this doesn't cause any technical issues, it is confusing because the directory name determined by the chromium config doesn't necessarily correlate with the actual directory. The filter.analyze method has been updated to accept the build output directory as an argument and the ios module has been updated to pass a directory whose name is based off of the build directory. The build directory was not re-used because filter.analyze doesn't handle the goma directory and so mb analyze would cause the args.gn file to be re-generated with a bad value for goma_dir. Bug: 670521 Change-Id: Ia6075d0a6cd1cbe41002107a3c93e04eab4690ef Reviewed-on: https://chromium-review.googlesource.com/c/1426178 Reviewed-by: Stephen Martinis <martiniss@chromium.org> Commit-Queue: Stephen Martinis <martiniss@chromium.org> Auto-Submit: Garrett Beaty <gbeaty@chromium.org> [modify] https://crrev.com/c761a1f1a387f1e5bf3a7c76509510cd45dbd664/scripts/slave/recipes/ios/try.expected/additional_compile_targets.json [modify] https://crrev.com/c761a1f1a387f1e5bf3a7c76509510cd45dbd664/scripts/slave/recipes/ios/try.expected/icu_patch.json [modify] https://crrev.com/c761a1f1a387f1e5bf3a7c76509510cd45dbd664/scripts/slave/recipe_modules/ios/api.py [modify] https://crrev.com/c761a1f1a387f1e5bf3a7c76509510cd45dbd664/scripts/slave/README.recipes.md [modify] https://crrev.com/c761a1f1a387f1e5bf3a7c76509510cd45dbd664/scripts/slave/recipes/ios/try.expected/goma_compilation_failure.json [modify] https://crrev.com/c761a1f1a387f1e5bf3a7c76509510cd45dbd664/scripts/slave/recipes/ios/try.expected/no_tests.json [modify] https://crrev.com/c761a1f1a387f1e5bf3a7c76509510cd45dbd664/scripts/slave/recipes/ios/try.expected/basic.json [modify] https://crrev.com/c761a1f1a387f1e5bf3a7c76509510cd45dbd664/scripts/slave/recipe_modules/chromium/api.py [modify] https://crrev.com/c761a1f1a387f1e5bf3a7c76509510cd45dbd664/scripts/slave/recipes/ios/try.expected/gn.json [modify] https://crrev.com/c761a1f1a387f1e5bf3a7c76509510cd45dbd664/scripts/slave/recipes/ios/try.expected/no_compilation.json [modify] https://crrev.com/c761a1f1a387f1e5bf3a7c76509510cd45dbd664/scripts/slave/recipe_modules/filter/api.py [modify] https://crrev.com/c761a1f1a387f1e5bf3a7c76509510cd45dbd664/scripts/slave/recipes/ios/try.expected/parent.json
,
Today
(13 hours ago)
|
||||||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by olivierrobin@chromium.org
, Dec 2 2016Owner: smut@chromium.org
Status: Assigned (was: Untriaged)