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

Issue 650833 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 833010



Sign in to add a comment

SimpleChromeWorkflow should use --internal on -chrome-pfq builders

Project Member Reported by steve...@chromium.org, Sep 27 2016

Issue description

SimpleChromeWorkflow tests cros chrome-sdk with the latest chromeos prebuild and the same chrome source used to build the chromeos-chrome ebuild.

cros chrome-sdk relies on '--internal' to configure args.gn correctly (e.g. with is_chrome_branded = True).

We should pass --interna to cros chrome-sdk on internal (-chrome-pfq) builders.

See issue 612248 for additional discussion.

 
Labels: -Pri-3 Pri-1
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/fd2b317271784288eeb6f5d8f17419d6ed85fca1

commit fd2b317271784288eeb6f5d8f17419d6ed85fca1
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Wed Jun 21 00:24:19 2017

crome_stages.py: Pass --internal to cros chrome-sdk

For chrome PFQ builds, we should be passing --internal
to cros chrome-sdk to test the internal build.

BUG= chromium:650833 
TEST=TestSimpleChrome stage uses --internal on chrome-pfq builders

Change-Id: I5cae2edd5f51fb53f96dd8190c5c6de4deaeadc4
Reviewed-on: https://chromium-review.googlesource.com/537875
Commit-Ready: Steven Bennetts <stevenjb@chromium.org>
Tested-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/fd2b317271784288eeb6f5d8f17419d6ed85fca1/cbuildbot/stages/chrome_stages.py

Cc: dgarr...@chromium.org
Originally I was using the following to identify chrome vs. chromium builders:

self._run.config.build_type == constants.CHROME_PFQ_TYPE

It was suggested that instead I use:

self._run.config.internal

However that turns out to be true on chromium builders, i.e.:
veyron_jerry-chromium-pfq, daisy-chromium-pfq, arm-generic-chromium-pfq,
amd64-generic-chromium-pfq

Is that expected? Would the original logic be correct or not? (We specifically want to run on -chrome-pfq builders and not -chromium-pfq builders).

+dgarrett@

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/e1c02c004a5dee99d70605e58bbae777b1e8c64b

commit e1c02c004a5dee99d70605e58bbae777b1e8c64b
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Wed Jun 21 17:11:39 2017

Revert "crome_stages.py: Pass --internal to cros chrome-sdk"

This reverts commit fd2b317271784288eeb6f5d8f17419d6ed85fca1.

Reason for revert: Logic is incorrect, --internal is getting passed to chromium
(i.e. non chrome) builders, specifically:

veyron_jerry-chromium-pfq, daisy-chromium-pfq, arm-generic-chromium-pfq,
amd64-generic-chromium-pfq

Original change's description:
> crome_stages.py: Pass --internal to cros chrome-sdk
> 
> For chrome PFQ builds, we should be passing --internal
> to cros chrome-sdk to test the internal build.
> 
> BUG= chromium:650833 
> TEST=TestSimpleChrome stage uses --internal on chrome-pfq builders
> 
> Change-Id: I5cae2edd5f51fb53f96dd8190c5c6de4deaeadc4
> Reviewed-on: https://chromium-review.googlesource.com/537875
> Commit-Ready: Steven Bennetts <stevenjb@chromium.org>
> Tested-by: Steven Bennetts <stevenjb@chromium.org>
> Reviewed-by: Don Garrett <dgarrett@chromium.org>

Bug:  chromium:650833 
Change-Id: I67d79b25148fb59e0b98769f6e57fba5840591ba
Reviewed-on: https://chromium-review.googlesource.com/542939
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Tested-by: Steven Bennetts <stevenjb@chromium.org>

[modify] https://crrev.com/e1c02c004a5dee99d70605e58bbae777b1e8c64b/cbuildbot/stages/chrome_stages.py

OK, I found the problem:

  # TODO(davidjames): Convert this to an external config once the unified master
  # logic is ready.
  site_config.AddTemplate(
      'chromium_pfq',
      site_config.templates.internal,
      ...

So, unless someone understands how to fix that TODO, I think we will need to use a different mechanism than 'internal', at least for now.

dgarett@ - WDYT of using 'self._run.config.build_type == constants.CHROME_PFQ_TYPE'? I am open to alternatives that will differentiate between 'chrome' and 'chromium' PFQ builders.

Project Member

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

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/9c4f2232102ca5d705f9296f4bc89b5a677e5ed7

commit 9c4f2232102ca5d705f9296f4bc89b5a677e5ed7
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Tue Jul 11 17:47:05 2017

crome_stages.py: Pass --internal to chrome-sdk (Take 2)

For chrome PFQ builds, we should be passing --internal
to cros chrome-sdk to test the internal build.

BUG= chromium:650833 
TEST=TestSimpleChrome stage uses --internal on chrome-pfq builders

Change-Id: Ibd943d542074baa7664ff48151963484a0f9305b
Reviewed-on: https://chromium-review.googlesource.com/545220
Tested-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>

[modify] https://crrev.com/9c4f2232102ca5d705f9296f4bc89b5a677e5ed7/cbuildbot/stages/chrome_stages.py

Comment 8 by glevin@chromium.org, Jul 12 2017

Cc: derat@chromium.org glevin@chromium.org
Based on last night's pfq runs, I don't think the
  self._run.config.build_type == constants.CHROME_PFQ_TYPE
check worked as intended.

veyron_jerry-chromium-pfq, daisy-chromium-pfq, amd64-generic-chromium-pfq and arm-generic-chromium-pfq (the Chromium builders called out in Comment #3) all failed TestSimpleChromeWorkflow, which appears to have run with the --internal flag and failed with

IOError: [Errno 2] No such file or directory: '../../chrome/app/theme/google_chrome/BRANDING'

See, for example,
https://uberchromegw.corp.google.com/i/chromeos/builders/amd64-generic-chromium-pfq/builds/10339/steps/TestSimpleChromeWorkflow/logs/stdio


Project Member

Comment 9 by bugdroid1@chromium.org, Jul 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/03be27b1287fd688a4efb97a4b5f47eeda9fa995

commit 03be27b1287fd688a4efb97a4b5f47eeda9fa995
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Wed Jul 12 16:08:51 2017

Revert "crome_stages.py: Pass --internal to chrome-sdk (Take 2)"

This reverts commit 9c4f2232102ca5d705f9296f4bc89b5a677e5ed7.

Reason for revert: This doesn't work either :(

Original change's description:
> crome_stages.py: Pass --internal to chrome-sdk (Take 2)
> 
> For chrome PFQ builds, we should be passing --internal
> to cros chrome-sdk to test the internal build.
> 
> BUG= chromium:650833 
> TEST=TestSimpleChrome stage uses --internal on chrome-pfq builders
> 
> Change-Id: Ibd943d542074baa7664ff48151963484a0f9305b
> Reviewed-on: https://chromium-review.googlesource.com/545220
> Tested-by: Steven Bennetts <stevenjb@chromium.org>
> Reviewed-by: Don Garrett <dgarrett@chromium.org>

Bug:  chromium:650833 
Change-Id: I93717688403881da8ab68fbc6f2886133b3e3c70
Reviewed-on: https://chromium-review.googlesource.com/568221
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Tested-by: Steven Bennetts <stevenjb@chromium.org>

[modify] https://crrev.com/03be27b1287fd688a4efb97a4b5f47eeda9fa995/cbuildbot/stages/chrome_stages.py

Components: Infra>Client>ChromeOS>CI
Components: -Infra>Client>ChromeOS
Blocking: 833010

Comment 13 by rcui@chromium.org, May 24 2018

Cc: rcui@chromium.org
Project Member

Comment 14 by bugdroid1@chromium.org, Jun 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/3a51f2a80a387b940b3c6166b50a85060b510ce4

commit 3a51f2a80a387b940b3c6166b50a85060b510ce4
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Fri Jun 15 16:36:31 2018

Pass --internal to simple chrome for chrome-pfq

This tests for 'chrome_internal' in useflags, which should be reliable.

BUG= chromium:650833 
TEST=SimpleChromeWorkflow stage runs with --internal on chrome-pfq builders

Change-Id: I4d2b623f26537b5e7a1ce2953eae1d748f47439b
Reviewed-on: https://chromium-review.googlesource.com/1101502
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Tested-by: Steven Bennetts <stevenjb@chromium.org>
Trybot-Ready: Steven Bennetts <stevenjb@chromium.org>

[modify] https://crrev.com/3a51f2a80a387b940b3c6166b50a85060b510ce4/cbuildbot/stages/chrome_stages.py

Project Member

Comment 15 by bugdroid1@chromium.org, Jun 15 2018

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

commit 64e587b560822112c2c00a6cdf65a0b5632d01e8
Author: Chromite Chromium Autoroll <chromite-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Jun 15 18:20:33 2018

Roll src/third_party/chromite ce952677d647..3a51f2a80a38 (1 commits)

https://chromium.googlesource.com/chromiumos/chromite.git/+log/ce952677d647..3a51f2a80a38


git log ce952677d647..3a51f2a80a38 --date=short --no-merges --format='%ad %ae %s'
2018-06-15 stevenjb@chromium.org Pass --internal to simple chrome for chrome-pfq


Created with:
  gclient setdep -r src/third_party/chromite@3a51f2a80a38

The AutoRoll server is located here: https://chromite-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.



BUG= chromium:650833 
TBR=chrome-os-gardeners@chromium.org

Change-Id: I361c35be91d66d477d91b8b5ef3b95e6f1286f06
Reviewed-on: https://chromium-review.googlesource.com/1102737
Reviewed-by: Chromite Chromium Autoroll <chromite-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: Chromite Chromium Autoroll <chromite-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#567733}
[modify] https://crrev.com/64e587b560822112c2c00a6cdf65a0b5632d01e8/DEPS

Status: Fixed (was: Started)
Hooray! This seems to have worked!

CL was in this PFQ run:
https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/buildDetails?id=2668000

Sample successful chrome-pfq run:

https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/buildDetails?builderName=tricky-chrome-pfq&buildNumber=5597

Confirmed --internal is in the cros chrome-sdk command:

14:28:24: INFO: RunCommand: cros --log-level debug --cache-dir /tmp/cbuildbot-tmp9jC0ok/chrome-sdk-cachexgCaBs/cache chrome-sdk --board tricky --cwd /b/c/cbuild/repository/.cache/distfiles/target-master/chrome-src-internal/src --sdk-path /b/c/cbuild/repository/buildbot_archive/tricky-chrome-pfq/R69-10786.0.0-rc1 --nogn-gen --nostart-goma --gomadir /b/c/goma_cache/client --internal --chrome-src /b/c/cbuild/repository/.cache/distfiles/target-master/chrome-src-internal/src -- /b/c/cbuild/repository/chromite/bin/deploy_chrome --build-dir /b/c/cbuild/repository/.cache/distfiles/target-master/chrome-src-internal/src/out_tricky/Release --staging-only --staging-dir /tmp/cbuildbot-tmp9jC0ok/chrome-sdk-stagelZ2aUh in /b/c/cbuild/repository

Sample successful chromium-pfq run:

https://cros-goldeneye.corp.google.com/chromeos/healthmonitoring/buildDetails?builderName=amd64-generic-chromium-pfq&buildNumber=12360

Confirmed no '--internal' in the cros chrome-sdk command:

13:51:17: INFO: RunCommand: cros --log-level debug --cache-dir /tmp/cbuildbot-tmpfVINiy/chrome-sdk-cachehYkFYT/cache chrome-sdk --board amd64-generic --cwd /b/c/cbuild/repository/.cache/distfiles/target-master/chrome-src/src --sdk-path /b/c/cbuild/repository/buildbot_archive/amd64-generic-chromium-pfq/R69-10786.0.0-rc1 --nogn-gen --nostart-goma --gomadir /b/c/goma_cache/client --chrome-src /b/c/cbuild/repository/.cache/distfiles/target-master/chrome-src/src -- true in /b/c/cbuild/repository

Sign in to add a comment