"ensure" is named this way since it ensures the local state matches whatever is in .ensure file. If it starts leaving stuff behind, it is no longer "ensure", but something else...

If this is exclusively for local development (i.e not for bots), I guess we can add an env var or maybe some magic "lock" command or something that can alter the behavior of "ensure" in a particular CIPD root. It will make caller responsible for manually cleaning up stuff.

Will this be sufficient?

Something like that. Here’s what I was thinking: if I could write:

@Subdir clang/${os}-${arch}
fuchsia/clang/${os}-${arch} latest
@Preserve clang/mac-amd64
@Preserve clang/linux-amd64

meaning “don’t remove the things listed in @Preserve, even if ${os}-${arch} is something different”, that’d be good. It’s finer-grained than just saying “don’t remove anything” which is what I think you’re suggesting. It’d still be nice to have “cipd ensure” remove things that are no longer relevant, have moved, etc., without forcing that to be the caller’s responsibility.

I think that this feature request is probably better handled by the "DevX" folks (working on gclient and related things). This request essentially boils down to having gclient hooks work in a mode where it doesn't clean things up. CIPD is the implementation detail of the day, but I suspect that the ability to leak files like this is not a known requirement of gclient. Even if we fix this in cipd, the behavior could diverge again tomorrow because this is not a known requirement for gclient.

On a related note, I'm looking into changing how gclient handles CIPD integration (see related bug).

+dpranke for fyi

gclient already operates in a "leave other stuff behind" mode. It doesn't know anything about the outputs of the hooks it runs, so if a hook stops running (due to a config change, or removal of the hook) it doesn't remove the last set of outputs produced by that hook. When a dep stops being synced (due to a config change, or removal of the dep) it just prints a warning saying "you probably don't need this anymore" but doesn't automatically delete it.

+liaoyuke, +jbudorick ...

With the conditionals changes, gclient itself is now fairly agnostic about how this stuff works. It is currently possible to check out everything you need to build multiple platforms from a single host; however, I don't think we've really considered the cast where you might want to fetch stuff for multiple host architectures/os'es (e.g., two different GN binaries). It should be easy enough to add that.

The larger problem is indeed probably CIPD, which we may not currently have set up in a way to do either of these two dimensions.

And, as agable@ says, we don't have a concept of cleaning up after hooks, either. We should think about how to get the equivalent of data/data_deps for hooks, because this would be useful for `analyze` as well.

I would prefer to avoid the concept of something like "preserve", apart from how we currently rely on .gitignores.

So I think if gclient wants this behavior, it should invoke `cipd install` instead of `cipd ensure`. `cipd install` will instruct cipd to add-or-update various packages, but will never clean them up. It sounds like this is the desired mode of operation for gclient?

(I would also mention that if we fix  crbug.com/789809 , it would be totally possible for gclient to run `cipd ensure` normally, but have a flag or something to tell it to do `cipd install` instead. I think on bots we specifically DON'T want to have leftovers? I'd also expect that most developers don't want leftovers either, but I don't have data for that).

I am using “cipd install” as a workaround now.

The thing I like about “cipd ensure” that “cipd install” doesn’t give me is the ability to automatically clean up obsolete packages.

In my example, I want to keep both mac-amd64 and linux-amd64 around, but only worry about installing/updating the one that applies to the build host. On the other hand, if they get moved about in the directory structure or obsoleted entirely, “cipd ensure” should be able to figure out, from my updated .ensure file, that they’re no longer necessary. It should be able to remove them.

So in this particular case, different subdirectories are used depending on the platform (pardon the tweaked syntax, which is closer to how this might actually look if it were implemented):

   @Subdir clang/${os}-${arch}
   fuchsia/clang/${os}-${arch} latest
     ^Preserve os=mac,arch=amd64
     ^Preserve os=linux,arch=amd64

If the (also entirely sensible) ensure file were used:
   
   @Subdir clang
   fuchsia/clang/${os}-${arch} latest
     ^Preserve os=mac,arch=amd64
     ^Preserve os=linux,arch=amd64

What would you expect to happen? What about if windows is thrown into the mix (which presumably has some differently named files, like `cc.exe` instead of `cc`)?

I don't think that this is something that cipd can generically support with ensure as an option inside the ensure file; it pretty radically alters the entire purpose of what ensure is meant to do (by weakening its guarantees). The purpose of the ensure file is to describe the state of the package tree in the event that `cipd ensure` exits cleanly... adding this option would make ensure a function of the ensure file as well as (sometimes!) the filesystem.

I think that ensure could MAYBE support a command line flag like `-append-only`, which would skip package deletion (only update/add them). This would still weaken ensure, but only in a way that's controlled by the caller. Bots, for example, would never pass this flag, and so would never need to worry about accumulating cruft because someone checked in an ensure file with Preserve statements. This would solve your immediate problem at the cost of leaving turds in the checkout if/when the folders move (so maybe ~1/year? less?). The remedy would be to run ensure once without the flag to get to a clean state, and then continue to run without the flag. gclient could similarly be configured to pass this flag to cipd ensure when the user instructs it to (also not something we'd configure on the bots). Maybe this could be an option in the .gclient file or something like that so developers who want this only need to set it once.

I still believe that the right thing to do here is to have a way to configure gclient (via conditions) in a way that says "I would like both the mac and linux toolchains in my checkout." and then having gclient tell cipd to ensure that this is the case. It would be more explicit, debuggable, wouldn't compromise/complicate cipd's semantics, would always leave the checkout in a reproducible state, and would be, on the whole, simpler.

> If the (also entirely sensible) ensure file were used:
>    
>    @Subdir clang
>    fuchsia/clang/${os}-${arch} latest
>      ^Preserve os=mac,arch=amd64
>      ^Preserve os=linux,arch=amd64
>
> What would you expect to happen?

That’s a bogus configuration, but I’ll take a stab at it. I’d expect cipd to try to install or update into clang from fuchsia/clang/${os}-${arch}, based on the current build host’s ${os}-${arch}. That means replacing (removing and reinstalling) an existing package if some other ${os}-${arch} was previously used. In my mind, “preserve” would be about hanging on to things that would be removed because they didn’t match the current configuration, while things that the current configuration defines in a way that obsolete what’s already on disk should still be honored by updating/replacing. But, like I said, it’s a bogus configuration and I don’t really have any expectations around it. I don’t think anyone would write this except in error, and it’d be fine by me if this raised an error or resulted in brokenness.

> I think that ensure could MAYBE support a command line flag like `-append-only`,
> which would skip package deletion (only update/add them). This would still weaken
> ensure, but only in a way that's controlled by the caller.

That doesn’t really help me, it’s really no better than saying “cipd install”. That’s what I was clarifying in comment 11.

> I still believe that the right thing to do here is to have a way to configure gclient
> (via conditions) in a way that says "I would like both the mac and linux toolchains
> in my checkout." and then having gclient tell cipd to ensure that this is the case. 

That’s fine for my use case too.

This seems to be gclient+CIPD-only issue, which seems better served by Infra>SDK component. Removing Infra>Client>Chrome. It may also be related to  issue 789809 , as an additional requirement for better CIPD support.

AFAICT,  issue 789809  should solve this use case.