New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 755622 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

"analyze" step skips compile on CLs that need a compile check

Project Member Reported by thakis@chromium.org, Aug 15 2017

Issue description

https://chromium-review.googlesource.com/c/615523 changes global cflags on Windows. Yet analyze decided to skip compile (https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_compile_dbg_ng/builds/4782140).

https://chromium-review.googlesource.com/c/615542 also changes build flags at least for nacl builds, yet it didn't have a compiler stage anywhere.

Maybe something broke analyze?
 

Comment 1 by thakis@chromium.org, Aug 15 2017

Cc: brettw@chromium.org jbudorick@chromium.org
We rolled gn yesterday: https://chromium.googlesource.com/chromium/buildtools/+/ceb050498e43dd0a1c8489022bf21fe687394366

Maybe that's related?
Cc: alex...@yandex-team.ru
Components: Build
Labels: -Pri-3 Pri-1
Status: Available (was: Untriaged)
Yes, that's almost certainly related, and probably due to:

https://codereview.chromium.org/2940873002

We should test that and probably revert that change and re-roll GN if we can't fix it relatively easily. 

Comment 3 by thakis@chromium.org, Aug 16 2017

https://chromium-review.googlesource.com/c/617764 also didn't see any compiles.

alex-ac, this seems like a serious regression. What's the plan here?
I plan to deal with this this afternoon. I agree that it is a serious regression.
Owner: dpranke@chromium.org
Status: Started (was: Available)
I tested this by just adding a generic #define to //build/config/compiler and re-running analyze, and can confirm that it incorrectly returned "no compile necessary". And, reverting the above CL reverted to the default "give up and compile everything" behavior.

In the interest of not leaving things broken I'm going to revert the change, roll GN, and then we can look at relanding a fixed version.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c9126ec981a4f32d6219cee1eee3a976dba5cead

commit c9126ec981a4f32d6219cee1eee3a976dba5cead
Author: Dirk Pranke <dpranke@chromium.org>
Date: Thu Aug 17 15:05:07 2017

Revert "Implement tracking of BUILD.gn files ..."

This change reverts reverts commit 44fd682d (#483337);
it looks like it doesn't correctly handle at least
changes to the default configs.

TBR=brettw@chromium.org, alex-ac@yandex-team.ru, thakis@chromium.org
BUG= 755622 

Change-Id: Ic8ad61736ad208bcce6004e8047febf405c001e1
Reviewed-on: https://chromium-review.googlesource.com/617774
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495174}
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/analyzer.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/analyzer.h
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/analyzer_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/builder_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/command_analyze.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/config.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/config.h
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/config_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/config_values_extractors_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/filesystem_utils_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/function_get_target_outputs_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/function_toolchain.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/functions.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/header_checker_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/import_manager.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/inherited_libraries_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/input_conversion.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/input_file.h
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/item.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/item.h
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/loader.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/loader_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/ninja_action_target_writer_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/ninja_binary_target_writer_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/ninja_build_writer_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/ninja_bundle_data_target_writer_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/ninja_copy_target_writer_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/ninja_create_bundle_target_writer_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/ninja_group_target_writer_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/ninja_target_writer_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/operators_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/runtime_deps_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/scope.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/scope.h
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/scope_per_file_provider_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/scope_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/settings.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/setup.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/setup.h
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/string_utils_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/substitution_writer_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/target.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/target.h
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/target_generator.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/target_unittest.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/test_with_scope.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/toolchain.cc
[modify] https://crrev.com/c9126ec981a4f32d6219cee1eee3a976dba5cead/tools/gn/toolchain.h

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/buildtools/+/d36e2d975b599009ff311719896b88f602987ff8

commit d36e2d975b599009ff311719896b88f602987ff8
Author: Dirk Pranke <dpranke@chromium.org>
Date: Thu Aug 17 18:01:16 2017

Roll gn 8d5e7fb9..c9126ec9 (r494532..r495172)

  c9126ec981a4 Revert "Implement tracking of BUILD.gn files ..."
  fbb03dd16b7d Remove base::Value::SetStringWithoutPathExpansion
  bafca0f82098 [iOS] Clean up xcode_extra_attributes usages in XcodeWriter.
  82cd07781b74 [iOS] Clean up xcode_test_application_name usages in XcodeWriter.

TBR=thakis@chromium.org
BUG= 755622 

Change-Id: I0b6b17be7754dc1269b176a801e36496adfb6856

[modify] https://crrev.com/d36e2d975b599009ff311719896b88f602987ff8/linux64/gn.sha1
[modify] https://crrev.com/d36e2d975b599009ff311719896b88f602987ff8/mac/gn.sha1
[modify] https://crrev.com/d36e2d975b599009ff311719896b88f602987ff8/win/gn.exe.sha1

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eecb3738a8667b0416d164fa56d17776860f5d3a

commit eecb3738a8667b0416d164fa56d17776860f5d3a
Author: Dirk Pranke <dpranke@chromium.org>
Date: Thu Aug 17 20:21:11 2017

Roll buildtools f90f6a5a..d36e2d97

  In order to roll GN 8d5e7fb9..c9126ec9 (r494532..r495172) and pick up
  the following changes:

    c9126ec981a4 Revert "Implement tracking of BUILD.gn files ..."
    fbb03dd16b7d Remove base::Value::SetStringWithoutPathExpansion
    bafca0f82098 [iOS] Clean up xcode_extra_attributes usages in XcodeWriter.
    82cd07781b74 [iOS] Clean up xcode_test_application_name usages in XcodeWriter.

TBR=thakis@chromium.org
BUG= 755622 

Change-Id: I2d593d82fb038eebccb37b9c7d2ced4ca413de6e
Reviewed-on: https://chromium-review.googlesource.com/618790
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495299}
[modify] https://crrev.com/eecb3738a8667b0416d164fa56d17776860f5d3a/DEPS

Status: Fixed (was: Started)
This should be (and seems to be) fixed now. https://chromium-review.googlesource.com/c/619991 has a link to a patch that modified a base compiler config and analyze worked correctly in a tryjob on linux_chromium_rel_ng.

Of course, we still want to land a working version of the GN patch, but that's not this bug.

Sign in to add a comment