New issue
Advanced search Search tips

Issue 794764 link

Starred by 12 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Feature


Sign in to add a comment

CIPD ensure needs a flag to tell it to verify the integrity of packages that it thinks are already installed

Project Member Reported by iannucci@chromium.org, Dec 14 2017

Issue description

For use on dev machines where people delete random files.
 
This will likely have performance implications so I don't think we can make this behavior default (though maybe vadim has thoughts on this?)
What about packages installed via "copy" mode? Once a file installed this way is deleted it is gone...

Are you perhaps proposing to add a paranoid ensure mode that does all it can to rebuild the correct state of the root (including redownloading and reinstalling packages if it has too)?
yeah I think that would need to be the case. I forget, do we keep a hashes manifest in the .cipd directory these days?
Vadim suggests in chat to name the flag '-paranoid' so that it would look like `cipd ensure -paranoid -ensure-file <thing>`.

I propose that the behavior of this flag would be to compare the hashes of every file to their hashes in a manifest stored in .cipd (I think we have this manifest already, but I don't recall). This will mean that the performance of cipd in the noop case will go from O(1) to O(number_of_installed_bytes).
(An alternative to consider might be adjusting the layout of the packages so that developers don't have a temptation to mess with them...)
Cc: jbudorick@chromium.org
Summary: CIPD ensure needs a flag to tell it to verify the integrity of packages that it thinks are already installed (was: CIPD ensure needs a flag to tell it to recreate all symlinks)
Status: Available (was: Untriaged)
Anyone mind if I take this? It's one of the sharper edges in our use of CIPD in src...
Cc: phosek@chromium.org
This is affecting Fuchsia as well so we would like to see this addressed.
This is occasionally frustrating for us downstream and generally surprising,  I'd also like to see this addressed. From our perspective it is a downgrade from the old DEPS downloads which would re-check things when re-run; cipd seems to currently need a rm -rf .cipd to "be safe".
Cc: -jbudorick@chromium.org
Owner: jbudorick@chromium.org
Status: Assigned (was: Available)
I'm going to assume that the lack of response to #9 in the last 3 months means that no one minds if I take this.
This feature is not trivial. Please outline your plan before starting implementing it.
#13: ack.
Blocking: 841266
John, do you mind if I take this? There're breakages happening that could be fixed if this is implemented (see  Issue 841266 ).
Cc: -vadimsh@chromium.org jbudorick@chromium.org
Owner: vadimsh@chromium.org
Go for it; I'd need to find a couple of days to stare at this and am unlikely to get those too soon. Would like to be CC'd on CLs for it, though.
While this is not ready, is there a recommended practice to recover from a "git clean" other than a complete clobber or a rm -rf .cipd?
#18: `rm -rf .cipd` or `gclient revert` should both work, though beware that the latter will reset the state of everything in your checkout.

Comment 20 by digit@google.com, Jun 13 2018

Blocking: 845405
Cc: scottmg@chromium.org
I found this very confusing too, in working on bug 855791. At least some sort of detection/error/warning would be really nice if possible.
Project Member

Comment 22 by bugdroid1@chromium.org, Jun 27 2018

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

commit 0895c7977a5c312aca132b23a3a9d46f78f77ee4
Author: Scott Graham <scottmg@chromium.org>
Date: Wed Jun 27 16:10:36 2018

Pull GN via CIPD package

The gn binary will be downloaded into third_party/gn. This CL rolls
buildtools forward to a revision where gn is no longer in buildtools.

Because buildtools has no way to cleanup after itself, add
build/util/gn_cleanup.py to delete the old binaries so that we don't
accidentally depend on them.

Update mb.py to use the new location.

Update explicit location in ios/web_view/BUILD.gn for the new location.

Update explicit location in
tools/traffic_annotation/auditor/traffic_annotation_auditor.cc for the new
location.

Update explicit location in tools/licenses.py with for the new location.

Bug: 855791
Bug:  794764 
Bug: 856883
Bug:  856884 
Bug: 856878
Bug:  856899 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: I92c908faf4f868850eafa1b4adf6e7c33c365116
Reviewed-on: https://chromium-review.googlesource.com/1112840
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570792}
[modify] https://crrev.com/0895c7977a5c312aca132b23a3a9d46f78f77ee4/DEPS
[add] https://crrev.com/0895c7977a5c312aca132b23a3a9d46f78f77ee4/build/util/gn_cleanup.py
[modify] https://crrev.com/0895c7977a5c312aca132b23a3a9d46f78f77ee4/ios/web_view/BUILD.gn
[modify] https://crrev.com/0895c7977a5c312aca132b23a3a9d46f78f77ee4/tools/licenses.py
[modify] https://crrev.com/0895c7977a5c312aca132b23a3a9d46f78f77ee4/tools/mb/mb.py
[modify] https://crrev.com/0895c7977a5c312aca132b23a3a9d46f78f77ee4/tools/mb/mb_unittest.py
[modify] https://crrev.com/0895c7977a5c312aca132b23a3a9d46f78f77ee4/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Jun 27 2018

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

commit 35383539aa83f27185a661215773ece946535676
Author: Scott Graham <scottmg@chromium.org>
Date: Wed Jun 27 16:27:35 2018

Revert "Pull GN via CIPD package"

This reverts commit 0895c7977a5c312aca132b23a3a9d46f78f77ee4.

Reason for revert: Missed a hardcoded gn location: https://crbug.com/857107.

Original change's description:
> Pull GN via CIPD package
> 
> The gn binary will be downloaded into third_party/gn. This CL rolls
> buildtools forward to a revision where gn is no longer in buildtools.
> 
> Because buildtools has no way to cleanup after itself, add
> build/util/gn_cleanup.py to delete the old binaries so that we don't
> accidentally depend on them.
> 
> Update mb.py to use the new location.
> 
> Update explicit location in ios/web_view/BUILD.gn for the new location.
> 
> Update explicit location in
> tools/traffic_annotation/auditor/traffic_annotation_auditor.cc for the new
> location.
> 
> Update explicit location in tools/licenses.py with for the new location.
> 
> Bug: 855791
> Bug:  794764 
> Bug: 856883
> Bug:  856884 
> Bug: 856878
> Bug:  856899 
> Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
> Change-Id: I92c908faf4f868850eafa1b4adf6e7c33c365116
> Reviewed-on: https://chromium-review.googlesource.com/1112840
> Commit-Queue: Scott Graham <scottmg@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570792}

TBR=dpranke@chromium.org,scottmg@chromium.org

Change-Id: I89e180710b5ce483106fa3b4ff6e8dec06780c07
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 855791,  794764 , 856883,  856884 , 856878,  856899 
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Reviewed-on: https://chromium-review.googlesource.com/1117219
Reviewed-by: Scott Graham <scottmg@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570799}
[modify] https://crrev.com/35383539aa83f27185a661215773ece946535676/DEPS
[delete] https://crrev.com/d0ed5774569fc56f4a0c74affafda8a7ad2a02aa/build/util/gn_cleanup.py
[modify] https://crrev.com/35383539aa83f27185a661215773ece946535676/ios/web_view/BUILD.gn
[modify] https://crrev.com/35383539aa83f27185a661215773ece946535676/tools/licenses.py
[modify] https://crrev.com/35383539aa83f27185a661215773ece946535676/tools/mb/mb.py
[modify] https://crrev.com/35383539aa83f27185a661215773ece946535676/tools/mb/mb_unittest.py
[modify] https://crrev.com/35383539aa83f27185a661215773ece946535676/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Jun 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/ce027be4609c3fe99f3040c62eb7f6783be813ae

commit ce027be4609c3fe99f3040c62eb7f6783be813ae
Author: Scott Graham <scottmg@chromium.org>
Date: Wed Jun 27 18:58:00 2018

Add 'remove .cipd' step to ios builders

This is the same as
https://chromium.googlesource.com/chromium/tools/build/+/ba55b88ad560966554ee82224eec00e1d9daa5b3,
but ios isn't handled in the same code path.

Bug:  794764 ,  849374 ,  857110 
Change-Id: Ib4c60fffc975b04ef7907112095209c4d79c00cf
Reviewed-on: https://chromium-review.googlesource.com/1117359
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Scott Graham <scottmg@chromium.org>

[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/explain.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/ios/unified_builder_tester.expected/goma_compilation_failure.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/timed_out.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/errors.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/clobber_build.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/ios/try.expected/icu_patch.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/expiration_test.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/expired.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/target_cpu_missing.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/is_debug_missing.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/ios/unified_builder_tester.expected/basic_experimental.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/ios/try.expected/patch_failure.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/xcode_build_version.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/api.py
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/basic.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/no_exit_code.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/ios/try.expected/gn.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/test_failure.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/infra_failure.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/ios/try.expected/basic.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/xcode_build_version_luci.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/ios/try.expected/goma_compilation_failure.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/deprecate_xcode_version.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/ios/try.expected/parent.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/ios/unified_builder_tester.expected/basic.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/ios/unified_builder_tester.expected/goma_canary.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/fyi.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/ios/unified_builder_tester.expected/goma.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/clobber_checkout.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/perf_test.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/ios/try.expected/additional_compile_targets.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/webrtc/chromium_ios.expected/basic_goma_build.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/README.recipes.md
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipe_modules/ios/examples/full.expected/device_check_false.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/ios/try.expected/no_tests.json
[modify] https://crrev.com/ce027be4609c3fe99f3040c62eb7f6783be813ae/scripts/slave/recipes/ios/try.expected/no_compilation.json

Blocking: 860251
Issue 860263 has been merged into this issue.
Labels: -Pri-2 Pri-1
vadimsh@, how long will you take to fix this issue?

Removing .cipd in every build on buildbot causes slow build time, especially for android builder. Because some toolchains (Desugar.jar, d8.jar etc) necessary to build android chrome are managed by CIPD, and recreation of files makes timestamp of toolchains fresh. That drops incrementability from compile step on builders.

If you need to take some time to fix this issue, can we use 'gclient revert' instead of removing .cipd dir in buildbot?
Or can we use timestamp preserving copy when files are not updated instaed of symlink in CIPD?
I actually haven't started yet (beyound just thinking). I'm starting today. I expect it will take several days at least.

I don't think 'gclient revert' will help. In fact, I have a feeling it's probably the reason cipd-deployed files get removed in the first place.

Two alternative ways forward while I work on the paranoid mode:
1. Figure out what's deleting cipd-deployed files on bots and make it STOP doing that. The paranoid mode is actually just a workaround. gclient shoulnd't be routinely deleting parts of its checkout.
2. You can rebuild packages with toolchains with '-preserve-mtime' option. It will preserve modification timestamps of files, i.e. they will always have their modification time set to the one stored in the package after they are extracted.
Note that gclient revert explicitly nukes .cipd as an escape hatch. That behavior can be revised once this bug is fixed.
oof, this looks like a lot of piled up workarounds. Has anyone identified what's deleting files that it doesn't own?
e.g. if some process was randomly touching source files that it didn't own, leading to slow build times, that would be fixed, however a process deleting files it doesn't own (and thus requiring cipd paranoid mode) doesn't get fixed?
Even if we identified that (which I don't think we fully have, though I have guesses), I think we'd still want this behavior available for the case where a developer deletes a symlink installed by cipd (intentionally or not) and wants to restore the state of their tree.
which is to say, I disagree with the premise of 28.1, that this paranoid mode is a workaround.
I understand the use for this in a not-primary-flow "oops I did a bad thing" path, but it seems like we'd be running this 100% of the time on the bots (and so any badly-behaving scripts wouldn't get fixed because this would pave over them)

I guess it can't be worse than running `git checkout` all the time (and scanning over ~6GB of files on disk)
As for what could be bad: I'm wondering if gclient should be running `cipd ensure` whether it has a cipd dependency configured or not? I don't think we currently do, and I think that could cause problems when the only packages used by a checkout get reverted & relanded.
Project Member

Comment 36 by bugdroid1@chromium.org, Jul 10

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/677cf543bc8149b70169d8d9437e984950820d73

commit 677cf543bc8149b70169d8d9437e984950820d73
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Jul 10 02:44:28 2018

[cipd] High level structure for the paranoid mode.

The approach:
  1. Deployer's CheckDeployed method now accepts ParanoiaMode argument
     specifying it should verify everything is actually deployed, and return
     a list of files that should be reinstalled if not.
  2. buildActionPlan learned to optionally use this to detect broken packages,
     and setup "Needs repair" action for them (with a plan what to repair). This
     allows 'puppet-check-updates' subcommand to figure out that something needs
     to be reinstalled without actually reinstalling anything yet.
  3. EnsurePackages then proceeds to repairing/reinstalling broken packages.

Nothing is actually implemented yet, this CL just sets up the general high
level structure. Also, there's no tests yet. They'll be added later.

R=iannucci@chromium.org
CC=​jbudorick@chromium.org
BUG= 794764 

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

[modify] https://crrev.com/677cf543bc8149b70169d8d9437e984950820d73/cipd/client/cipd/action_plan.go
[modify] https://crrev.com/677cf543bc8149b70169d8d9437e984950820d73/cipd/client/cipd/client.go
[modify] https://crrev.com/677cf543bc8149b70169d8d9437e984950820d73/cipd/client/cipd/local/deployer.go
[modify] https://crrev.com/677cf543bc8149b70169d8d9437e984950820d73/cipd/client/cli/friendly.go
[modify] https://crrev.com/677cf543bc8149b70169d8d9437e984950820d73/cipd/client/cli/main.go
[modify] https://crrev.com/677cf543bc8149b70169d8d9437e984950820d73/vpython/cipd/cipd.go

Project Member

Comment 37 by bugdroid1@chromium.org, Jul 10

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/fb1c5a269a6baa6116c6a21745ebb7b45831d5fd

commit fb1c5a269a6baa6116c6a21745ebb7b45831d5fd
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Tue Jul 10 21:18:09 2018

[cipd] Add $ParanoidMode directive to the ensure file.

It can be set to 'CheckPresence' to enable a mode in which CIPD verifies all
supposedly installed files are indeed present.

Effectively noop currently.

R=iannucci@chromium.org
CC=​jbudorick@chromium.org
BUG= 794764 

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

[modify] https://crrev.com/fb1c5a269a6baa6116c6a21745ebb7b45831d5fd/cipd/client/cipd/client.go
[modify] https://crrev.com/fb1c5a269a6baa6116c6a21745ebb7b45831d5fd/cipd/client/cipd/ensure/bad_test.go
[modify] https://crrev.com/fb1c5a269a6baa6116c6a21745ebb7b45831d5fd/cipd/client/cipd/ensure/doc.go
[modify] https://crrev.com/fb1c5a269a6baa6116c6a21745ebb7b45831d5fd/cipd/client/cipd/ensure/file.go
[modify] https://crrev.com/fb1c5a269a6baa6116c6a21745ebb7b45831d5fd/cipd/client/cipd/ensure/file_test.go
[modify] https://crrev.com/fb1c5a269a6baa6116c6a21745ebb7b45831d5fd/cipd/client/cipd/ensure/good_test.go
[modify] https://crrev.com/fb1c5a269a6baa6116c6a21745ebb7b45831d5fd/cipd/client/cipd/ensure/item_parsers.go
[modify] https://crrev.com/fb1c5a269a6baa6116c6a21745ebb7b45831d5fd/cipd/client/cipd/local/deployer.go
[modify] https://crrev.com/fb1c5a269a6baa6116c6a21745ebb7b45831d5fd/cipd/client/cli/friendly.go
[modify] https://crrev.com/fb1c5a269a6baa6116c6a21745ebb7b45831d5fd/cipd/client/cli/main.go
[add] https://crrev.com/fb1c5a269a6baa6116c6a21745ebb7b45831d5fd/cipd/common/paranoia.go

Project Member

Comment 38 by bugdroid1@chromium.org, Jul 11

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/96fd48259f804e69b25a18b96467f93d42b403cd

commit 96fd48259f804e69b25a18b96467f93d42b403cd
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Wed Jul 11 20:40:11 2018

[cipd] Implement detection of files that need redeployment.

Checks only presence. Doesn't check content or file mode.

This also introduces two flavors of repairs:
  * Redeploy: to repair such files, the client will need to fetch the original
    package file.
  * Relink: to repair such files, the client will only need to restore symlinks
    from the .cipd/* guts to the site root. This is a local operation.

The distinction will be important in the follow up CL, where cipd.Client will
decide whether it needs to fetch the package or not.

R=iannucci@chromium.org
CC=​​jbudorick@chromium.org
BUG= 794764 

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

[modify] https://crrev.com/96fd48259f804e69b25a18b96467f93d42b403cd/cipd/client/cipd/action_plan.go
[modify] https://crrev.com/96fd48259f804e69b25a18b96467f93d42b403cd/cipd/client/cipd/client.go
[modify] https://crrev.com/96fd48259f804e69b25a18b96467f93d42b403cd/cipd/client/cipd/local/deployer.go
[modify] https://crrev.com/96fd48259f804e69b25a18b96467f93d42b403cd/cipd/client/cipd/local/deployer_test.go
[modify] https://crrev.com/96fd48259f804e69b25a18b96467f93d42b403cd/cipd/client/cipd/local/fs.go

Project Member

Comment 39 by bugdroid1@chromium.org, Jul 12

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/80af58173c58a2306257a762e26fcfe13d0bca29

commit 80af58173c58a2306257a762e26fcfe13d0bca29
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Thu Jul 12 21:10:53 2018

[cipd] Fetch the package if it is needed to repair a deployment.

R=iannucci@chromium.org
CC=​​jbudorick@chromium.org
BUG= 794764 

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

[modify] https://crrev.com/80af58173c58a2306257a762e26fcfe13d0bca29/cipd/client/cipd/client.go
[modify] https://crrev.com/80af58173c58a2306257a762e26fcfe13d0bca29/cipd/client/cipd/local/deployer.go

Project Member

Comment 40 by bugdroid1@chromium.org, Jul 15

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/6266207973c4abf1ae84405aa3b453f3ec9c7185

commit 6266207973c4abf1ae84405aa3b453f3ec9c7185
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Sun Jul 15 01:16:49 2018

[cipd] Refactor Destination interface to support extracting into existing dirs.

What was called Destination before is now called TransactionalDestination. It
supports extracting packages into a new directory with transactional semantics
(all or nothing).

New Destination interface is the remaining non-transactional part. It can be
used to write files into an existing directory (which will be needed during
repairs). Since atomically writing a bunch of files is not really possible,
extracting into an existing directory is not transactional (extracting each
individual file is still is though).

R=iannucci@chromium.org
CC=​​jbudorick@chromium.org
BUG= 794764 

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

[modify] https://crrev.com/6266207973c4abf1ae84405aa3b453f3ec9c7185/cipd/client/cipd/local/deployer.go
[modify] https://crrev.com/6266207973c4abf1ae84405aa3b453f3ec9c7185/cipd/client/cipd/local/files.go
[modify] https://crrev.com/6266207973c4abf1ae84405aa3b453f3ec9c7185/cipd/client/cipd/local/files_test.go
[modify] https://crrev.com/6266207973c4abf1ae84405aa3b453f3ec9c7185/cipd/client/cipd/local/reader.go
[modify] https://crrev.com/6266207973c4abf1ae84405aa3b453f3ec9c7185/cipd/client/cipd/local/reader_test.go

Project Member

Comment 41 by bugdroid1@chromium.org, Jul 15

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/eab8d0a7adf5cc24a1244b1febe156698ff8c732

commit eab8d0a7adf5cc24a1244b1febe156698ff8c732
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Sun Jul 15 01:25:29 2018

[cipd] Refactor CheckDeployed/RemoveDeployed to share code.

This is in preparation for RepairDeployed to share this code too.

Pure refactoring, no new logic.

R=iannucci@chromium.org
CC=​​jbudorick@chromium.org
BUG= 794764 

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

[modify] https://crrev.com/eab8d0a7adf5cc24a1244b1febe156698ff8c732/cipd/client/cipd/client.go
[modify] https://crrev.com/eab8d0a7adf5cc24a1244b1febe156698ff8c732/cipd/client/cipd/local/deployer.go
[modify] https://crrev.com/eab8d0a7adf5cc24a1244b1febe156698ff8c732/cipd/client/cipd/local/deployer_test.go
[modify] https://crrev.com/eab8d0a7adf5cc24a1244b1febe156698ff8c732/cipd/client/cli/friendly.go

Project Member

Comment 42 by bugdroid1@chromium.org, Jul 15

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/76fd7050ddba11c4a915e5e3c4706c29c1c3d603

commit 76fd7050ddba11c4a915e5e3c4706c29c1c3d603
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Sun Jul 15 01:37:39 2018

[cipd] Rename ExtractInstance to ExtractFiles, make it accept []File.

When repairing instances, we have []File, but may not have the PackageInstance
(if we didn't fetch anything). But ExtractInstance doesn't really need
PackageInstance. It only uses its Files() method to get []File.

This change necessitates moving .cipd/* gut filtering to Deployer, which is
probably a better place for it anyway.

R=iannucci@chromium.org
CC=jbudorick@chromium.org
BUG= 794764 

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

[modify] https://crrev.com/76fd7050ddba11c4a915e5e3c4706c29c1c3d603/cipd/client/cipd/local/deployer.go
[modify] https://crrev.com/76fd7050ddba11c4a915e5e3c4706c29c1c3d603/cipd/client/cipd/local/deployer_test.go
[modify] https://crrev.com/76fd7050ddba11c4a915e5e3c4706c29c1c3d603/cipd/client/cipd/local/reader.go
[modify] https://crrev.com/76fd7050ddba11c4a915e5e3c4706c29c1c3d603/cipd/client/cipd/local/reader_test.go

Project Member

Comment 43 by bugdroid1@chromium.org, Jul 15

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/f1385cd7d606a42887b2f4ec48903579e8184bd3

commit f1385cd7d606a42887b2f4ec48903579e8184bd3
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Sun Jul 15 01:44:30 2018

[cipd] Make manifest extraction in ExtractFiles optional.

It is not needed (and harmful), when extracting only subset of files when
repairing a package.

Also promote related constants from private to public, since manifests and
structure of packages is public stuff, at least within the CIPD client
codebase. Constants look more consistent that way too.

R=iannucci@chromium.org
CC=jbudorick@chromium.org
BUG= 794764 

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

[modify] https://crrev.com/f1385cd7d606a42887b2f4ec48903579e8184bd3/cipd/client/cipd/local/builder.go
[modify] https://crrev.com/f1385cd7d606a42887b2f4ec48903579e8184bd3/cipd/client/cipd/local/builder_test.go
[modify] https://crrev.com/f1385cd7d606a42887b2f4ec48903579e8184bd3/cipd/client/cipd/local/deployer.go
[modify] https://crrev.com/f1385cd7d606a42887b2f4ec48903579e8184bd3/cipd/client/cipd/local/deployer_test.go
[modify] https://crrev.com/f1385cd7d606a42887b2f4ec48903579e8184bd3/cipd/client/cipd/local/json_descs.go
[modify] https://crrev.com/f1385cd7d606a42887b2f4ec48903579e8184bd3/cipd/client/cipd/local/reader.go
[modify] https://crrev.com/f1385cd7d606a42887b2f4ec48903579e8184bd3/cipd/client/cipd/local/reader_test.go

Project Member

Comment 44 by bugdroid1@chromium.org, Jul 16

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/9449d182d0131f0eb5d859c8f777bde4aca82f9e

commit 9449d182d0131f0eb5d859c8f777bde4aca82f9e
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Mon Jul 16 01:31:10 2018

[cipd] Implement Deployer.RepairDeployed.

This concludes the implementation of CheckPresence paranoid mode.

R=iannucci@chromium.org
CC=jbudorick@chromium.org
BUG= 794764 

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

[modify] https://crrev.com/9449d182d0131f0eb5d859c8f777bde4aca82f9e/cipd/client/cipd/client.go
[modify] https://crrev.com/9449d182d0131f0eb5d859c8f777bde4aca82f9e/cipd/client/cipd/local/deployer.go
[modify] https://crrev.com/9449d182d0131f0eb5d859c8f777bde4aca82f9e/cipd/client/cipd/local/deployer_test.go

Project Member

Comment 45 by bugdroid1@chromium.org, Jul 16

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/521b9b64770d3b43a221b1e2bd277687616adacf

commit 521b9b64770d3b43a221b1e2bd277687616adacf
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Mon Jul 16 01:43:47 2018

Roll infra/go/src/go.chromium.org/luci/ 12258214c..9449d182d (5 commits)

https://chromium.googlesource.com/infra/luci/luci-go/+log/12258214cf61..9449d182d013

$ git log 12258214c..9449d182d --date=short --no-merges --format='%ad %ae %s'
2018-07-16 vadimsh [cipd] Implement Deployer.RepairDeployed.
2018-07-15 vadimsh [cipd] Make manifest extraction in ExtractFiles optional.
2018-07-15 vadimsh [cipd] Rename ExtractInstance to ExtractFiles, make it accept []File.
2018-07-15 vadimsh [cipd] Refactor CheckDeployed/RemoveDeployed to share code.
2018-07-15 vadimsh [cipd] Refactor Destination interface to support extracting into existing dirs.

Created with:
  roll-dep infra/go/src/go.chromium.org/luci

TBR=iannucci@chromium.org
BUG= 794764 

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

[modify] https://crrev.com/521b9b64770d3b43a221b1e2bd277687616adacf/DEPS

Project Member

Comment 46 by bugdroid1@chromium.org, Jul 16

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

commit fb734036f4b5ae6d5afc63cbfc41d3a5d1c29a82
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Mon Jul 16 22:08:14 2018

[cipd] Update CIPD client to 2.1.0 (b9c4670197 -> 521b9b6477).

The new version supports '$ParanoidMode CheckPresence' in ensure files.

R=iannucci@chromium.org
BUG= 794764 

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

[modify] https://crrev.com/fb734036f4b5ae6d5afc63cbfc41d3a5d1c29a82/cipd_client_version

Project Member

Comment 47 by bugdroid1@chromium.org, Jul 16

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/infradata/config/+/8d8e006858d13e04a07eec1339f2c9bd1509b8df

commit 8d8e006858d13e04a07eec1339f2c9bd1509b8df
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Mon Jul 16 22:21:34 2018

Project Member

Comment 48 by bugdroid1@chromium.org, Jul 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/3ca64e74397fd04c31b3b43004071618ef3696b1

commit 3ca64e74397fd04c31b3b43004071618ef3696b1
Author: Vadim Shtayura <vadimsh@chromium.org>
Date: Mon Jul 16 22:25:00 2018

[cipd] Update CIPD client to 2.1.0 (b9c4670197 -> 521b9b6477).

The new version supports '$ParanoidMode CheckPresence' in ensure files.

R=iannucci@chromium.org
BUG= 794764 

Change-Id: I79531b18e378a4b14ae2c45256319591b9a664c4
Reviewed-on: https://chromium-review.googlesource.com/1138735
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>

[modify] https://crrev.com/3ca64e74397fd04c31b3b43004071618ef3696b1/scripts/slave/cipd_bootstrap_v2.py

Project Member

Comment 49 by bugdroid1@chromium.org, Jul 17

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

commit 20666af345b4dce7f1846d734cdbde9393be513e
Author: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Tue Jul 17 01:01:39 2018

Roll src/third_party/depot_tools 8d3925b16482..fb734036f4b5 (1 commits)

https://chromium.googlesource.com/chromium/tools/depot_tools.git/+log/8d3925b16482..fb734036f4b5


git log 8d3925b16482..fb734036f4b5 --date=short --no-merges --format='%ad %ae %s'
2018-07-16 vadimsh@chromium.org [cipd] Update CIPD client to 2.1.0 (b9c4670197 -> 521b9b6477).


Created with:
  gclient setdep -r src/third_party/depot_tools@fb734036f4b5

The AutoRoll server is located here: https://depot-tools-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:794764 
TBR=agable@chromium.org

Change-Id: I7f7b32f6845b0f30c32d1fe411c8c8e70a0ac705
Reviewed-on: https://chromium-review.googlesource.com/1138995
Reviewed-by: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: depot-tools-chromium-autoroll <depot-tools-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#575502}
[modify] https://crrev.com/20666af345b4dce7f1846d734cdbde9393be513e/DEPS

Can we remove 'remove .cipd' step now?
#50: should be ok to do so given vadimsh's work in this bug + https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1132345; will write up the CLs & land tomorrow.
#51: I see. Thank you for prioritizing this bug.
Status: Fixed (was: Assigned)
Project Member

Comment 54 by bugdroid1@chromium.org, Nov 2

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

commit 74121ab3d156d9152da03f484fa07b6b1bbbe845
Author: Stephen Martinis <martiniss@chromium.org>
Date: Fri Nov 02 23:41:50 2018

Remove old cipd clobber gclient hook

The bug it relied on is fixed now.

Bug:  794764 
Change-Id: Ifb52b474590966e19768a122a042fa708569a2d2
Reviewed-on: https://chromium-review.googlesource.com/c/1308907
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Stephen Martinis <martiniss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605096}
[modify] https://crrev.com/74121ab3d156d9152da03f484fa07b6b1bbbe845/DEPS

Sign in to add a comment