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

Issue 663843 link

Starred by 3 users
Status: Duplicate
Merged: issue 789809
Owner:
jbudorick@chromium.org
Closed: Jun 2018
Cc:
█ jparent@chromium.org
█ iannucci@chromium.org
vadimsh@chromium.org
dpranke@chromium.org
dpa...@chromium.org
aga...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 662541


Sign in to add a comment

Allow chromium/src to take dependencies on CIPD for binary dependencies

Project Member Reported by iannu...@google.com, Nov 9 2016 
Currently we have a bunch of ad-hoc mechanisms for pulling binaries into chromium/src:
  * download_from_google_storage
  * https://chromium.googlesource.com/chromium/buildtools/
  * custom downloader scripts:
    * build/download_nacl_toolchains.py
    * build/android/play_services/update.py
    * build/android/update_deps/update_third_party_deps.py
    * build/linux/sysroot_scripts/install-sysroot.py
    * build/vs_toolchain.py
    * build/mac_toolchain.py
    * third_party/binutils/download.py
    * tools/clang/scripts/update.py
    * build/get_syzygy_binaries.py
    * third_party/instrumented_libraries/scripts/download_binaries.py
    * build/android/download_doclava.py
    * tools/clang_format_merge_driver/install_git_hook.py
  * hard-coded binaries that float @ HEAD in depot_tools (which means that patching them in a CL is not possible):
    * ninja
  * manually-run scripts which are hopefully 'good enough'. These aren't even run on the bots and so patching them in a CL is not possible:
    * build/install-build-deps.sh (and friends)
  * probably some others that I missed

There are a bunch of issues with each of these:
  * They're run serially (gclient hooks have no way of knowing that they're just, essentially, doing downloads)
  * each has their own "caching" mechanism
  * each has their own integrity mechanism (or none)
  * each has their own upload, some of which are obtuse/slow.
  * each has their own pinning/rolling process
  * each needs to be inspected differently; there's no common basis for reasoning about them
  * each may need its own credentials (though most of these are public/anonymous credentials today)

Furthermore, src-internal is currently used to pull binaries that are stashed in a variety of repos (which is unfortunate because pulling git repos with binaries in them is generally considered a very painful experience); there's a long-standing bug (396073) to move the contents of src-internal which may be technically assisted by allowing CIPD support in chromium/src.

This recently came to a point when we realized that in order to enable some new javascript-based tests in chromium/src, we actually need node.js. Infra already successfully deploys node via CIPD. The CIPD distribution has many advantages, including:
  * parallel fetching of all dependencies
  * high-integrity binary verification and caching mechanism
  * zipfile deployment for efficiently downloading a bundle of files
  * canaryable upload/roll process (can upload new versions without breaking old versions, can tag by ref or by e.g. git commit hash of the origin)
  * easy to produce a report about what binaries were included in a build
  * can apply ACLs to certain packages if need be e.g. if some src-internal dependencies moved to CIPD we could filter them based on the user's credentials at a per-package level.

The good news is that we're not actually far off from being able to use CIPD. It is already used in production on all bots, so we're confident that the system works correctly already. 

At the high-level we need to:
  * enable cipd on developer machines (integrate it into depot_tools at bash/batch/powershell level, requires adding download_client endpoint to service to minimize bootstrap code)
  * add script/manifest to chromium/src and a single gclient hook for it (or, alternately, build the manifest directly into DEPS, per dpranke@)
  * migrate adhoc downloads to CIPD packages

Comment 1 by iannu...@google.com, Nov 9 2016

Blockedon: -663091
Blocking: 663091

Comment 2 by iannu...@google.com, Nov 9 2016

Cc: aga...@chromium.org
Components: Infra>SDK

Comment 3 by mmoss@chromium.org, Nov 9 2016 

I'm not sure build/install-build-deps.sh fits the same pattern as the other tools, since that's really about system configuration and distro package installs, and not about distributing anything that we provide.

Comment 4 by iannu...@google.com, Nov 9 2016 

It fits the same pattern of build/vs_toolchain.py and build/mac_toolchain.py. The fact that it modifies system packages is a (very longstanding) bug. CIPD packages are for containing binary dependencies; the origin of the binaries doesn't matter.

Comment 5 by mmoss@chromium.org, Nov 9 2016 

Actually, the sysroot scripts seem more equivalent to the toolchain scripts, and I agree those definitely fit the pattern, and your proposal. Perhaps it's time install-build-deps dies and we use the sysroots everywhere (I think they're currently only used for official builds).

Comment 6 by iannu...@google.com, Nov 9 2016 

It's not my immediate plan to kill install-build-deps, but anything I can do to hasten its demise is a plus. I'll be focusing on the rest of the scripts first. The toolchain/sysroot scripts are in a pretty decent place at the moment (though there's still some room for improvement).

Comment 7 by mmoss@chromium.org, Nov 9 2016 

sgtm

Comment 8 by dpranke@chromium.org, Nov 10 2016

Cc: andyb...@chromium.org
We actually use the sysroots everywhere by default nowadays, but I think install-build-deps installs other things like tools that we probably need as well. It would be a good exercise to see what we actually still needed.

Apart from that, I think the idea of moving to CIPD consistently seems like a good one (as we've discussed), but I also don't think upgrading the existing scripts is a very high priority thing. Let's get something w/ CIPD working, but I don't want to block anything on migrating the existing stuff over. 

Does that sound reasonable?

Also, there's probably cases where we might want telemetry to download things as well so we should talk to the speed-infra folks about this. Their needs are probably more complicated than anything else we'd be looking at.

Comment 9 by tansell@chromium.org, Nov 10 2016

Cc: mcgreevy@chromium.org jparent@chromium.org djd@chromium.org
+ djd@, mcgreevy@, jparent@

There is an problem not mentioned here and is kind of hard to describe. It basically comes down to the idea that we care somewhat about Chrome "history", not just the state of current head. I'm however very unsure how much we do care (I don't actually know many people who end up building older versions of Chrome?).

There are two competing thorny problems here though;

 * Older revisions of the code based may depend on older versions of the tools to produce results. Examples might include;
   - Older version of Chrome might use something which is no longer available in the Android SDK.
   - It feels important to be able to build an older revision of Chrome with the clang / binutils that was in use at that time. 
   - This feeds into the "build determinism" problem.

 * Some tools won't work if you go "back in time". Example might include;
    - Tools which depend on stuff like oauth or other remote mechanisms which no longer work. In these cases you probably want to use a newer version of the tool even though you are on an older revision.
    - Tools which don't impact the build output but are involved in the build process. Things like isolate, swarming and goma shouldn't change the output - so using the newer faster versions even when building old stuff kind of makes sense.

I think both things can probably be satisfied with using CIPD for storage of the dependencies but this issue does have some possible flow on effects. Random comments along these lines;
 - Some tools will reference explicit versions while others will reference floating versions.
 - Any explicit version that is referenced must never be garbaged collected, removed or otherwise modified.

Comment 10 by dpranke@chromium.org, Nov 10 2016 

@tansell - 

You raise good points.

We absolutely need to support people building old versions, a pretty large number of people do need to do it pretty often. We also have the obvious need to support things across trunk, beta, and stable, and we have the need to bisect large ranges of revisions looking for regressions.
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 14 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra.git/+/7f0902c9dce1d801b52d18451ad62e33e2f7fdb4

commit 7f0902c9dce1d801b52d18451ad62e33e2f7fdb4
Author: Robert Iannucci <iannucci@chromium.org>
Date: Mon Nov 14 19:06:27 2016

Add file_name to fetchClientBinary endpoint.

BUG= 663843 

Change-Id: I0514996c458f79babbc396e5371fafce736acae5
Reviewed-on: https://chromium-review.googlesource.com/410943
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>

[modify] https://crrev.com/7f0902c9dce1d801b52d18451ad62e33e2f7fdb4/appengine/chrome_infra_packages/cipd/api.py
[modify] https://crrev.com/7f0902c9dce1d801b52d18451ad62e33e2f7fdb4/appengine/chrome_infra_packages/cipd/test/api_test.py
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 14 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/a300394a1ae1e7cbcc77b016ec3124a0b5142ccf

commit a300394a1ae1e7cbcc77b016ec3124a0b5142ccf
Author: iannucci <iannucci@chromium.org>
Date: Mon Nov 14 21:41:22 2016

Hide advanced commands from cipd client help.

R=dnj@chromium.org, vadimsh@chromium.org
BUG= 663843 

Review-Url: https://codereview.chromium.org/2503583002

[modify] https://crrev.com/a300394a1ae1e7cbcc77b016ec3124a0b5142ccf/cipd/client/cmd/cipd/friendly.go
[modify] https://crrev.com/a300394a1ae1e7cbcc77b016ec3124a0b5142ccf/cipd/client/cmd/cipd/main.go
[modify] https://crrev.com/a300394a1ae1e7cbcc77b016ec3124a0b5142ccf/client/authcli/authcli.go
[modify] https://crrev.com/a300394a1ae1e7cbcc77b016ec3124a0b5142ccf/client/cmd/isolate/main.go
[modify] https://crrev.com/a300394a1ae1e7cbcc77b016ec3124a0b5142ccf/client/cmd/isolated/main.go
[modify] https://crrev.com/a300394a1ae1e7cbcc77b016ec3124a0b5142ccf/client/cmd/swarming/main.go
[modify] https://crrev.com/a300394a1ae1e7cbcc77b016ec3124a0b5142ccf/deploytool/cmd/luci_deploy/main.go
[modify] https://crrev.com/a300394a1ae1e7cbcc77b016ec3124a0b5142ccf/grpc/cmd/rpc/main.go
[modify] https://crrev.com/a300394a1ae1e7cbcc77b016ec3124a0b5142ccf/logdog/client/cli/main.go
[modify] https://crrev.com/a300394a1ae1e7cbcc77b016ec3124a0b5142ccf/logdog/client/cmd/logdog_butler/main.go
[modify] https://crrev.com/a300394a1ae1e7cbcc77b016ec3124a0b5142ccf/logdog/common/storage/archive/logdog_archive_test/main.go

Comment 14 by iannu...@google.com, Nov 19 2016 

re comments #10 and #9: I absolutely agree, and I intend to only add CIPD support in a way that we can facilitate building historical versions easily (and routinely. this is very important for bisect). If that's not possible for some reason, then I would consider that a P0 blocker.

In particular, I would propose that (by policy, maybe by PRESUBMIT) we only ever refer to CIPD packages in chromium/src via tags (which are immutable) or by explicit instance ids (content-addresses), not by refs (which mutate over time and are things like "latest"). This would be similar to the way that we disallow DEPS'ing in repos via branch name.

For convenience, it would also be feasible to have a roll_cipd tool which updated tags based on some refs (e.g. roll_cipd.py some/package/name latest would update the tag to git_revision:deadbeef). In any event, this should be much easier to work with than download_from_google_storage, or trying to land binaries in the buildtools repo. This will also allow us to use the trybots for binaries too:
  * upload new version to cipd
  * CQ/dryrun CL which updates pins in src

But packages are independent from each other (unlike buildtools), and easily inspectable (unlike download_from_google_storage and other custom downloader scripts).
Project Member

Comment 20 by bugdroid1@chromium.org, Nov 29 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra.git/+/926e65f4a3cd788521e8c8b8c54c66360db34d14

commit 926e65f4a3cd788521e8c8b8c54c66360db34d14
Author: Robert Iannucci <iannucci@chromium.org>
Date: Tue Nov 29 21:16:34 2016

Bump luci-go.

06da8fd Add HTTP User Agent flag for cipd.
b6eff57 Make selfupdate maintain client version as well.
dd65e8b Add base128 encoding library.
3e06bc0 Regenerate internal/messages.
4f96867 client/isolate: add Checker to coalesce Contains calls
0366bd8 Fix reader sizes.
8950fc7 Log events from exparchive.
b16ae2a Add the new monitoring endpoint protos to the Go tsmon.
c6ecfb2 client/isolate: expand deps into individual files
74934df client/isolate: add skeleton exparchive command
c34ebd4 luci-go: Basic support for event logging in Go.
14213b9 Add cipd selfupdate command.
ca8863e client: use helper funcs to construct isolated.File
7d92ecc Milo: Add ts_mon metrics for master json datastore success
427d9ab Milo: Basic nested steps support

BUG= 663843 

Change-Id: I90b9512cfb349402dcdd8c9c0ebac25988794b2a
Reviewed-on: https://chromium-review.googlesource.com/414591
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/926e65f4a3cd788521e8c8b8c54c66360db34d14/DEPS

Comment 24 by dpa...@chromium.org, Dec 8 2016

Cc: dbeam@chromium.org dpa...@chromium.org
Project Member

Comment 25 by bugdroid1@chromium.org, Dec 9 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/307e15deab37650e9627fe3d06ee88a2dd82e1cd

commit 307e15deab37650e9627fe3d06ee88a2dd82e1cd
Author: iannucci <iannucci@chromium.org>
Date: Fri Dec 09 04:04:48 2016

Allow cipd ensure file to contain package name templates.

This will allow ensure files to be generically specified regardless of
platform, e.g.

  path/to/package/${platform}  git_revision:deadbeef....

R=dnj@chromium.org, vadimsh@chromium.org
BUG= 663843 

Review-Url: https://codereview.chromium.org/2559893003

[modify] https://crrev.com/307e15deab37650e9627fe3d06ee88a2dd82e1cd/cipd/client/cipd/client.go
[modify] https://crrev.com/307e15deab37650e9627fe3d06ee88a2dd82e1cd/cipd/client/cipd/client_test.go
Project Member

Comment 26 by bugdroid1@chromium.org, Dec 10 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/3f4c0a36feb6ff528e1734eabc2b7033799fb26e

commit 3f4c0a36feb6ff528e1734eabc2b7033799fb26e
Author: iannucci <iannucci@chromium.org>
Date: Sat Dec 10 00:33:07 2016

Add SIMPLE conditional package name expansion.

This will allow ensure files to have lines like:

  infra/tools/git/${platform=windows}-${arch}  git_revision:deadbeef....

To support cipd-ification of e.g. depot_tools without wrapper scripts.

R=dnj@chromium.org, vadimsh@chromium.org
BUG= 663843 

Review-Url: https://codereview.chromium.org/2550413008

[modify] https://crrev.com/3f4c0a36feb6ff528e1734eabc2b7033799fb26e/cipd/client/cipd/client.go
[modify] https://crrev.com/3f4c0a36feb6ff528e1734eabc2b7033799fb26e/cipd/client/cipd/client_test.go
Project Member

Comment 27 by bugdroid1@chromium.org, Dec 13 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/2188fe944f28a13369460a7b9285e407824acd9d

commit 2188fe944f28a13369460a7b9285e407824acd9d
Author: Robert Iannucci <iannucci@chromium.org>
Date: Fri Dec 02 19:15:57 2016

Add cipd bootstrap scripts to depot_tools.

This takes advantage of powershell on windows for a cleanish duplicate of the
posix version.

R=dnj@chromium.org, vadimsh@chromium.org
BUG= 663843 

Change-Id: Ib23a044ff912e3239b58848a26143eb6575826d5
Reviewed-on: https://chromium-review.googlesource.com/414228
Commit-Queue: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/2188fe944f28a13369460a7b9285e407824acd9d/.gitignore
[add] https://crrev.com/2188fe944f28a13369460a7b9285e407824acd9d/cipd
[add] https://crrev.com/2188fe944f28a13369460a7b9285e407824acd9d/cipd.bat
[add] https://crrev.com/2188fe944f28a13369460a7b9285e407824acd9d/cipd.ps1
[add] https://crrev.com/2188fe944f28a13369460a7b9285e407824acd9d/cipd_client_version
Project Member

Comment 28 by bugdroid1@chromium.org, Jan 3 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/15f122ee64fe3f1e39a1a1ba3a822b6d6962d520

commit 15f122ee64fe3f1e39a1a1ba3a822b6d6962d520
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Jan 03 21:48:58 2017

Allow cipd.ps1 execution even if it was fetched from depot_tools.zip.

Windows keeps track of files downloaded from the Internet and powershell by
default refuses to execute them (even with RemoteSigned policy). We need to
explicitly unblock the file first.

Note: it requires Powershell >= 3.0.

R=dpranke@chromium.org, iannucci@chromium.org
BUG= 663843 

Change-Id: Id681f4058f906fa4782a360be184d132d837ae78
Reviewed-on: https://chromium-review.googlesource.com/424327
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/15f122ee64fe3f1e39a1a1ba3a822b6d6962d520/cipd.bat
Project Member

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

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/d3cee645d7868a841597cb5bf3038010ecbf382f

commit d3cee645d7868a841597cb5bf3038010ecbf382f
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Jan 03 23:39:32 2017

Revert "Allow cipd.ps1 execution even if it was fetched from depot_tools.zip."

This reverts commit 15f122ee64fe3f1e39a1a1ba3a822b6d6962d520.

Reason for revert: lots of bots are running Powershell 2. the 
new step fails on them (non-fatally though, just generating 
errors in the log, e.g. https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/357219/steps/ensure%20git%20tooling%20on%20windows/logs/stdio)

Original change's description:
> Allow cipd.ps1 execution even if it was fetched from depot_tools.zip.
> 
> Windows keeps track of files downloaded from the Internet and powershell by
> default refuses to execute them (even with RemoteSigned policy). We need to
> explicitly unblock the file first.
> 
> Note: it requires Powershell >= 3.0.
> 
> R=dpranke@chromium.org, iannucci@chromium.org
> BUG= 663843 
> 
> Change-Id: Id681f4058f906fa4782a360be184d132d837ae78
> Reviewed-on: https://chromium-review.googlesource.com/424327
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>
> 

TBR=iannucci@chromium.org,vadimsh@chromium.org,dpranke@chromium.org,chromium-reviews@chromium.org
BUG= 663843 
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Change-Id: Ibdc58e60d3f35a760de45bc1f71c88c1c4d2f2df
Reviewed-on: https://chromium-review.googlesource.com/424872
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>
Reviewed-by: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/d3cee645d7868a841597cb5bf3038010ecbf382f/cipd.bat

Comment 30 by dpranke@chromium.org, Jan 16 2017

Labels: -Pri-3 OS-All Pri-2
@iannucci - can we get an update / next steps on this?

Comment 31 by andyb...@chromium.org, Jan 18 2017

Cc: -andyb...@chromium.org
Project Member

Comment 41 by bugdroid1@chromium.org, Jan 27 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/c08a552ab25cc1d8d543dbbe48f83aa0c968205b

commit c08a552ab25cc1d8d543dbbe48f83aa0c968205b
Author: iannucci <iannucci@chromium.org>
Date: Fri Jan 27 04:33:30 2017

[cipd] Rename new Root terminology to Subdir.

The new term is confusing with the existing 'root' meaning. Better to
nip this one in the bud before it gets messy.

R=vadimsh@chromium.org
BUG= 663843 

Review-Url: https://codereview.chromium.org/2663473002

[modify] https://crrev.com/c08a552ab25cc1d8d543dbbe48f83aa0c968205b/cipd/client/cipd/common/common.go
[modify] https://crrev.com/c08a552ab25cc1d8d543dbbe48f83aa0c968205b/cipd/client/cipd/common/common_test.go
[modify] https://crrev.com/c08a552ab25cc1d8d543dbbe48f83aa0c968205b/cipd/client/cipd/ensure/bad_test.go
[modify] https://crrev.com/c08a552ab25cc1d8d543dbbe48f83aa0c968205b/cipd/client/cipd/ensure/doc.go
[modify] https://crrev.com/c08a552ab25cc1d8d543dbbe48f83aa0c968205b/cipd/client/cipd/ensure/file.go
[modify] https://crrev.com/c08a552ab25cc1d8d543dbbe48f83aa0c968205b/cipd/client/cipd/ensure/file_test.go
[modify] https://crrev.com/c08a552ab25cc1d8d543dbbe48f83aa0c968205b/cipd/client/cipd/ensure/good_test.go
[modify] https://crrev.com/c08a552ab25cc1d8d543dbbe48f83aa0c968205b/cipd/client/cipd/ensure/item_parsers.go
[modify] https://crrev.com/c08a552ab25cc1d8d543dbbe48f83aa0c968205b/cipd/client/cmd/cipd/main.go
Project Member

Comment 42 by bugdroid1@chromium.org, Jan 27 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/88f5293a10b2bcbd534e4e7a28a851b38585f3b1

commit 88f5293a10b2bcbd534e4e7a28a851b38585f3b1
Author: iannucci <iannucci@chromium.org>
Date: Fri Jan 27 04:53:09 2017

[cipd] Add AssertOnlyDefaultSubdir.

This transitional function will end up being called by a bunch of things
in subsequent CLs.

R=vadimsh@chromium.org
BUG= 663843 

Review-Url: https://codereview.chromium.org/2663483002

[modify] https://crrev.com/88f5293a10b2bcbd534e4e7a28a851b38585f3b1/cipd/client/cipd/common/common.go
[modify] https://crrev.com/88f5293a10b2bcbd534e4e7a28a851b38585f3b1/cipd/client/cmd/cipd/main.go

Comment 44 by mmoss@chromium.org, Jan 27 2017

Cc: -mmoss@chromium.org

Comment 49 by iannu...@google.com, Mar 2 2017

Cc: iannucci@chromium.org
Owner: ----
Status: Available (was: Started)
I discussed with dpranke a couple weeks ago, and this is paused, pending additional work on gclient.

Comment 50 by dpranke@chromium.org, Jun 30 2017

Blocking: -663091

Comment 51 by dpranke@chromium.org, Jun 30 2017

Blockedon: 662541

Comment 52 by mar...@chromium.org, Jun 11 2018

Cc: -dbeam@chromium.org -mcgreevy@chromium.org -djd@chromium.org
Mergedinto: 789809
Owner: jbudorick@chromium.org
Status: Duplicate (was: Available)
This was fixed earlier this year! \o/
►

Sign in to add a comment