New issue
Advanced search Search tips

Issue 903460 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Compiling problem for chrome_public_apk (another issue with Jumbo?)

Project Member Reported by mar...@mwiacek.com, Nov 8

Issue description

I receive compilation error. I strongly believe that it should be captured by builder https://bugs.chromium.org/p/chromium/issues/detail?id=896410.

Error:

In file included from gen/content/browser/browser_jumbo_4.cc:28:
In file included from ./../../content/browser/cache_storage/cache_storage_manager.cc:30:
In file included from ../../content/browser/service_worker/service_worker_context_core.h:21:
In file included from ../../content/browser/service_worker/service_worker_info.h:15:
In file included from ../../content/browser/service_worker/service_worker_version.h:30:
In file included from ../../content/browser/service_worker/embedded_worker_instance.h:25:
In file included from ../../content/browser/service_worker/service_worker_metrics.h:15:
../../content/browser/service_worker/service_worker_database.h:398:5: error: declaration shadows a variable in namespace 'content::(anonymous)' [-Werror,-Wshadow]
    UNINITIALIZED,
    ^
./../../content/browser/browser_thread_impl.cc:36:3: note: previous declaration is here
  UNINITIALIZED = 0,
  ^
In file included from gen/content/browser/browser_jumbo_4.cc:28:
In file included from ./../../content/browser/cache_storage/cache_storage_manager.cc:30:
In file included from ../../content/browser/service_worker/service_worker_context_core.h:25:
../../content/browser/service_worker/service_worker_storage.h:566:5: error: declaration shadows a variable in namespace 'content::(anonymous)' [-Werror,-Wshadow]
    UNINITIALIZED,
    ^
./../../content/browser/browser_thread_impl.cc:36:3: note: previous declaration is here
  UNINITIALIZED = 0,
  ^

Compilation settings:

target_os="android"
target_cpu="arm"
enable_nacl=false
symbol_level=0
remove_webcore_debug_symbols=true
is_debug=false
dcheck_always_on=false
is_component_build=false
use_jumbo_build=true


 
The builder appears to be fine:
https://luci-milo.appspot.com/p/chromium/builders/luci.chromium.ci/android-jumbo-rel

Is your checkout up to date? Have you ran "gclient sync" recently?
Yes and yes, I'm working on fix...
Owner: mar...@mwiacek.com
Status: Started (was: Unconfirmed)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 9

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

commit d630de0987d6548a684ad742dad51ff8fb06fa55
Author: Marcin Wiacek <marcin@mwiacek.com>
Date: Fri Nov 09 07:54:05 2018

Fix Jumbo builds with UNINITIALIZED values

Error:

../../content/browser/service_worker/service_worker_database.h:398:5: error: declaration shadows a variable in namespace 'content::(anonymous)' [-Werror,-Wshadow]
    UNINITIALIZED,
    ^
../../content/browser/service_worker/service_worker_storage.h:566:5: error: declaration shadows a variable in namespace 'content::(anonymous)' [-Werror,-Wshadow]
    UNINITIALIZED,
    ^

./../../content/browser/browser_thread_impl.cc:36:3: note: previous declaration is here
  UNINITIALIZED = 0,
  ^

Example compilation settings:

target_os="android"
target_cpu="arm"
enable_nacl=false
symbol_level=0
remove_webcore_debug_symbols=true
is_debug=false
dcheck_always_on=false
is_component_build=false
use_jumbo_build=true

BUG= 903460 

Change-Id: Ie8c6ef88ac299e02724bcbbeb1db69e67515f62b
Reviewed-on: https://chromium-review.googlesource.com/c/1327213
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Marcin WiÄ…cek <marcin@mwiacek.com>
Cr-Commit-Position: refs/heads/master@{#606763}
[modify] https://crrev.com/d630de0987d6548a684ad742dad51ff8fb06fa55/content/browser/service_worker/service_worker_database.cc
[modify] https://crrev.com/d630de0987d6548a684ad742dad51ff8fb06fa55/content/browser/service_worker/service_worker_database.h
[modify] https://crrev.com/d630de0987d6548a684ad742dad51ff8fb06fa55/content/browser/service_worker/service_worker_database_unittest.cc
[modify] https://crrev.com/d630de0987d6548a684ad742dad51ff8fb06fa55/content/browser/service_worker/service_worker_storage.cc
[modify] https://crrev.com/d630de0987d6548a684ad742dad51ff8fb06fa55/content/browser/service_worker/service_worker_storage.h

Cc: dpranke@chromium.org brat...@opera.com
The CQ jumbo bot runs with goma optimized jumbo chunks (~8 files per translation unit) which is much less than for everyone else (~50 files per translation unit). That makes it miss things. Could be time to look at changing it.

Thanks for taking care of this!
Oh, and the direct trigger was a new clang version with improved checks for shadowed enums. It looks like they have been working for a couple of weeks fixing enough of the source to use the new clang (see  bug 895475 ) but missed a handful of problems in jumbo builds.


Yeah, I think I'm fine with changing the defaults at this point.

We should also double-check the other GN args, to make sure they otherwise exactly match the other configs (e.g., linux-jumbo-rel should otherwise match linux_chromium_rel_ng); otherwise it makes comparing build speeds impossible.
who could take this bug in such case?
Status: Fixed (was: Started)
I've opened 905588 for this. The particular problem you've fixed so I'm closing it.
Bug 905588 (making it clickable)
Status: Verified (was: Fixed)

Comment 13 Deleted

Sign in to add a comment