New issue
Advanced search Search tips

Issue 705072 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Apr 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Remove dependence on GYP_DEFINES when downloading instrumented libraries

Project Member Reported by thomasanderson@chromium.org, Mar 24 2017

Issue description

There's currently a gotcha step when running msan builds where you have to run 'gclient sync' with the right GYP_DEFINES to actually download the prebuilt instrumented libraries.  This should be made into a gn target that runs automatically instead.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 25 2017

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

commit 5adb8c047852670be6c9d42114f3a17d44b6c56e
Author: thomasanderson <thomasanderson@google.com>
Date: Sat Mar 25 02:45:41 2017

Instrumented libraries: Remove dependence on GYP_DEFINES

This CL ports download_binaries.py and unpack_binaries.py from using
GYP_DEFINES to gn args.  Previously, there was a gotcha step where you
had to run 'gclient sync' with the right GYP_DEFINES to actually download
the prebuilt instrumented libraries.  This CL removes that step.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_msan_rel_ng;master.tryserver.chromium.linux:linux_chromium_chromeos_msan_rel_ng
R=eugenis@chromium.org
BUG= 705072 

Review-Url: https://codereview.chromium.org/2775913002
Cr-Commit-Position: refs/heads/master@{#459630}

[modify] https://crrev.com/5adb8c047852670be6c9d42114f3a17d44b6c56e/DEPS
[modify] https://crrev.com/5adb8c047852670be6c9d42114f3a17d44b6c56e/third_party/instrumented_libraries/BUILD.gn
[modify] https://crrev.com/5adb8c047852670be6c9d42114f3a17d44b6c56e/third_party/instrumented_libraries/scripts/download_binaries.py
[modify] https://crrev.com/5adb8c047852670be6c9d42114f3a17d44b6c56e/third_party/instrumented_libraries/scripts/unpack_binaries.py

Cc: dpranke@chromium.org
Is this work in line with  issue 570091 ? Could you mark it as blocked by this if the issues are connected?
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/459b881c233557f5da9ca9e4779fc1696c785a1c

commit 459b881c233557f5da9ca9e4779fc1696c785a1c
Author: thomasanderson <thomasanderson@google.com>
Date: Wed Mar 29 22:07:43 2017

Remove download_binaries.py from DEPS

This step is no longer necessary after https://codereview.chromium.org/2775913002/

BUG= chromium:705072 

Review-Url: https://codereview.chromium.org/2774043002
Cr-Commit-Position: refs/heads/master@{#44252}

[modify] https://crrev.com/459b881c233557f5da9ca9e4779fc1696c785a1c/DEPS

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/3b72184d40abde9c70149783189eb77c7438dc4f

commit 3b72184d40abde9c70149783189eb77c7438dc4f
Author: thomasanderson <thomasanderson@chromium.org>
Date: Fri Mar 31 07:33:57 2017

Revert of Remove download_binaries.py from DEPS (patchset #2 id:20001 of https://codereview.chromium.org/2774043002/ )

Reason for revert:
https://codereview.chromium.org/2775913002/ needs to be reverted, so reverting all dependent patch sets

Original issue's description:
> Remove download_binaries.py from DEPS
>
> This step is no longer necessary after https://codereview.chromium.org/2775913002/
>
> BUG= chromium:705072 
>
> Review-Url: https://codereview.chromium.org/2774043002
> Cr-Commit-Position: refs/heads/master@{#44252}
> Committed: https://chromium.googlesource.com/v8/v8/+/459b881c233557f5da9ca9e4779fc1696c785a1c

TBR=machenbach@chromium.org,thomasanderson@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= chromium:705072 

Review-Url: https://codereview.chromium.org/2786173004
Cr-Commit-Position: refs/heads/master@{#44289}

[modify] https://crrev.com/3b72184d40abde9c70149783189eb77c7438dc4f/DEPS

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 4 2017

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

commit aa441f50eecca13f2d3490a89705cb00cc0d5920
Author: thomasanderson <thomasanderson@chromium.org>
Date: Tue Apr 04 22:35:39 2017

Revert of Instrumented libraries: Remove dependence on GYP_DEFINES (patchset #2 id:60001 of https://codereview.chromium.org/2775913002/ )

Reason for revert:
Reverting now that all of the third_party changes have been reverted

Original issue's description:
> Instrumented libraries: Remove dependence on GYP_DEFINES
>
> This CL ports download_binaries.py and unpack_binaries.py from using
> GYP_DEFINES to gn args.  Previously, there was a gotcha step where you
> had to run 'gclient sync' with the right GYP_DEFINES to actually download
> the prebuilt instrumented libraries.  This CL removes that step.
>
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_msan_rel_ng;master.tryserver.chromium.linux:linux_chromium_chromeos_msan_rel_ng
> R=eugenis@chromium.org
> BUG= 705072 
>
> Review-Url: https://codereview.chromium.org/2775913002
> Cr-Commit-Position: refs/heads/master@{#459630}
> Committed: https://chromium.googlesource.com/chromium/src/+/5adb8c047852670be6c9d42114f3a17d44b6c56e

TBR=eugenis@chromium.org,dpranke@chromium.org,machenbach@chromium.org,thomasanderson@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 705072 

Review-Url: https://codereview.chromium.org/2792343003
Cr-Commit-Position: refs/heads/master@{#461871}

[modify] https://crrev.com/aa441f50eecca13f2d3490a89705cb00cc0d5920/DEPS
[modify] https://crrev.com/aa441f50eecca13f2d3490a89705cb00cc0d5920/third_party/instrumented_libraries/BUILD.gn
[modify] https://crrev.com/aa441f50eecca13f2d3490a89705cb00cc0d5920/third_party/instrumented_libraries/scripts/download_binaries.py
[modify] https://crrev.com/aa441f50eecca13f2d3490a89705cb00cc0d5920/third_party/instrumented_libraries/scripts/unpack_binaries.py

Status: WontFix (was: Started)
For the record: the reason this was reverted / wontfixed is because we do not ever want to download things (or, more generally require network connectivity) during the build. Developers should be able to generally assume that if they've run `gclient sync` they can then do all their development offline (assuming they're not using goma, of course).

Sign in to add a comment