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

Issue 750260 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression

Blocking:
issue 733409



Sign in to add a comment

generate_build_files fails on GPU FYI Linux Chromium OS Ozone builder.

Project Member Reported by cwallez@chromium.org, Jul 28 2017

Issue description

First failing build:
https://build.chromium.org/p/chromium.gpu.fyi/builders/Linux%20ChromiumOS%20Ozone%20Builder/builds/26506

First failing logs:
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.gpu.fyi%2FLinux_ChromiumOS_Ozone_Builder%2F26506%2F%2B%2Frecipes%2Fsteps%2Fgenerate_build_files%2F0%2Fstdout

From the logs https://chromium-review.googlesource.com/c/572144 seems like it introduce includes that are forbidden in some configurations:
ERROR at //components/wallpaper/wallpaper_info.h:9:11: Include not allowed.
#include "components/user_manager/user.h"
          ^-----------------------------
It is not in any dependency of
  //components/wallpaper:wallpaper
The include file is in the target(s):
  //ash/public/interfaces:interfaces_internal

wzang@, can you take a look it?

The GN args for this build are in the same logs if you want to reproduce:
build_angle_deqp_tests = true
dcheck_always_on = true
enable_package_mash_services = true
ffmpeg_branding = "Chrome"
goma_dir = "/b/c/goma_client"
internal_gles2_conform_tests = true
is_component_build = false
is_debug = false
ozone_auto_platforms = false
ozone_platform = "x11"
ozone_platform_gbm = true
ozone_platform_wayland = true
ozone_platform_x11 = true
proprietary_codecs = true
strip_absolute_paths_from_debug_symbols = true
symbol_level = 1
use_ash = false
use_goma = true
use_ozone = true
use_system_libdrm = true
use_system_minigbm = true
use_xkbcommon = true
 

Comment 1 by wzang@chromium.org, Jul 28 2017

Sure, I'm working on it now.

Comment 2 by wzang@chromium.org, Jul 28 2017

Cc: jdufault@chromium.org x...@chromium.org
I'm not able to repo locally. I have a tentative solution but I'm wondering how to check if it works.

https://chromium-review.googlesource.com/c/592328/

Comment 3 by x...@chromium.org, Jul 28 2017

wzang@, you should be able to repro it locally by changing your GN args as in the bug description. Use the command: gn args out/Default (or whatever your build folder) to change your GN args.

Comment 4 by wzang@chromium.org, Jul 28 2017

xdai@, yeah I already tried, but the above GN args result in lots of compile errors which I don't get using my normal GN args, so I never reached the stage of seeing the errors in the log. Maybe enable_package_mash_services = true or use_ash = false is the reason.

So I'm wondering if there's another way to test this.

Comment 5 by wzang@chromium.org, Jul 28 2017

After offline discussion with @jdufault, it'll be fixed by this CL https://chromium-review.googlesource.com/c/592328/
Project Member

Comment 6 by bugdroid1@chromium.org, Jul 29 2017

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

commit b2ee6c753532130c7b80770265c6f38dbd50e373
Author: Wenzhao Zang <wzang@chromium.org>
Date: Sat Jul 29 20:39:01 2017

Remove redundant dependency of components/user_manager/user.h

As a follow-up of CL 572144, some redundant dependency needs to be
removed.
Please see the bug and the error log for more details.

Bug:  750260 
Change-Id: I00ef044160cd90aac8e39b23144eef21f29a7d31
Reviewed-on: https://chromium-review.googlesource.com/592328
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Oliver Chang <ochang@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490673}
[modify] https://crrev.com/b2ee6c753532130c7b80770265c6f38dbd50e373/ash/public/cpp/wallpaper_struct_traits.h
[modify] https://crrev.com/b2ee6c753532130c7b80770265c6f38dbd50e373/ash/public/interfaces/wallpaper.typemap
[modify] https://crrev.com/b2ee6c753532130c7b80770265c6f38dbd50e373/chrome/browser/extensions/api/autotest_private/autotest_private_api.cc
[modify] https://crrev.com/b2ee6c753532130c7b80770265c6f38dbd50e373/chrome/browser/extensions/api/easy_unlock_private/easy_unlock_private_api.cc
[modify] https://crrev.com/b2ee6c753532130c7b80770265c6f38dbd50e373/chrome/browser/extensions/extension_system_impl.cc
[modify] https://crrev.com/b2ee6c753532130c7b80770265c6f38dbd50e373/chrome/browser/extensions/external_provider_impl.cc
[modify] https://crrev.com/b2ee6c753532130c7b80770265c6f38dbd50e373/chrome/browser/ui/webui/net_internals/net_internals_ui.cc
[modify] https://crrev.com/b2ee6c753532130c7b80770265c6f38dbd50e373/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

Comment 7 by wzang@chromium.org, Jul 30 2017

Status: Fixed (was: Assigned)

Comment 8 by wzang@chromium.org, Aug 8 2017

Labels: Merge-Request-61
Project Member

Comment 9 by sheriffbot@chromium.org, Aug 8 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: M61 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Before we approve merge to M61, please answer followings:
* Is this M61 regression? Is it critical?
* Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M61?
* Any other important details to justify the merge.

Please note M61 is already in Beta, so merge bar is very high. Thank you.
* Is this M61 regression? Is it critical?
--Yes. It is a follow-up to a CL https://chromium-review.googlesource.com/c/596801 which will also be merged. They must be merged together. 

* Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M61?
--Yes, it's in 62.0.3172.0.

Labels: -OS-Linux OS-Chrome
Cc: keta...@chromium.org kbleicher@chromium.org
ketakid@ and kbleicher@ for M61 merge review as this is Chrome OS specific.

Comment 14 by wzang@chromium.org, Aug 10 2017

Blocking: 733409
ketakid@ This needs merge today as it's blocking 733409. Please take a look.
Project Member

Comment 15 by bugdroid1@chromium.org, Aug 10 2017

Labels: merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b58a07543e5a8ad794546f9faf17f85d54b9db0a

commit b58a07543e5a8ad794546f9faf17f85d54b9db0a
Author: Wenzhao Zang <wzang@chromium.org>
Date: Thu Aug 10 18:29:26 2017

Remove redundant dependency of components/user_manager/user.h

As a follow-up of CL 572144, some redundant dependency needs to be
removed.
Please see the bug and the error log for more details.

TBR=wzang@chromium.org

(cherry picked from commit b2ee6c753532130c7b80770265c6f38dbd50e373)

Bug:  750260 
Change-Id: I00ef044160cd90aac8e39b23144eef21f29a7d31
Reviewed-on: https://chromium-review.googlesource.com/592328
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Oliver Chang <ochang@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#490673}
Reviewed-on: https://chromium-review.googlesource.com/610921
Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#443}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/b58a07543e5a8ad794546f9faf17f85d54b9db0a/ash/public/cpp/wallpaper_struct_traits.h
[modify] https://crrev.com/b58a07543e5a8ad794546f9faf17f85d54b9db0a/ash/public/interfaces/wallpaper.typemap
[modify] https://crrev.com/b58a07543e5a8ad794546f9faf17f85d54b9db0a/chrome/browser/extensions/api/autotest_private/autotest_private_api.cc
[modify] https://crrev.com/b58a07543e5a8ad794546f9faf17f85d54b9db0a/chrome/browser/extensions/api/easy_unlock_private/easy_unlock_private_api.cc
[modify] https://crrev.com/b58a07543e5a8ad794546f9faf17f85d54b9db0a/chrome/browser/extensions/extension_system_impl.cc
[modify] https://crrev.com/b58a07543e5a8ad794546f9faf17f85d54b9db0a/chrome/browser/extensions/external_provider_impl.cc
[modify] https://crrev.com/b58a07543e5a8ad794546f9faf17f85d54b9db0a/chrome/browser/ui/webui/net_internals/net_internals_ui.cc
[modify] https://crrev.com/b58a07543e5a8ad794546f9faf17f85d54b9db0a/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

Labels: -Merge-Review-61 Merge-Approved-61
Approving merge for M61 and M62.
Project Member

Comment 17 by sheriffbot@chromium.org, Sep 11 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 18 by wzang@chromium.org, Sep 11 2017

Labels: -Merge-Approved-61

Comment 19 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment