Allow not persisting CACHE mount

[ingwarsw]

Adds option to disable persist cache.
See #3509 for details.

Im not sure if by default should be persist or not-persist.
Persisting data seems bad idea in most "global" caches, cause they will grow and it takes ages to persist them.
And caches are well, caches and I assume in most cases are not needed in image..
And if someone wants to persist it it can enable it.

[alexcb]

Thanks for the PR; it should be not-persist, as we try to avoid backwards-breaking changes. We could potentially flip the default behaviour when we bump VERSION 0.7 to 0.8.

[vladaionescu]

it should be not-persist, as we try to avoid backwards-breaking changes

Hey @alexcb - note that currently, the behavior is to always persist. So then the default should remain persist, right?

[vladaionescu]

Either way, the existence of this new option still requires a feature flag. Something like VERSION --cache-not-persist-option 0.7.

[alexcb]

it should be not-persist, as we try to avoid backwards-breaking changes

Hey @alexcb - note that currently, the behavior is to always persist. So then the default should remain persist, right?

That's what I tried to say, but words are hard.... thanks for clarifying this :)

I understood the question as which of the two boolean flag names do we want:

CACHE --persisted ... or CACHE --non-persisted ....

I think we'll want CACHE --non-persisted ..., so that users can continue to use CACHE ... which will give them a persisted cache by default.

Something like VERSION --cache-not-persist-option 0.7

I wonder if we really need that in this case, because old users of earthly will get an error such as CACHE --non-persisted contains an unsupported flag name, so we'll prevent cases where a feature is ignored by older earthly clients. Thoughts @vladaionescu ?

[vladaionescu]

It's best to add a feature flag for new commands and new options, even if they're backwards compatible. The reasoning is that every 0.7 Earthly should be able to build any VERSION 0.7 Earthfile.

[vladaionescu]

BTW should be call the flag --no-persist? It's a little more consistent

[ingwarsw]

Adding extra flag seems ok..
But since we will be adding flag maybe then default can be reverted (it will not be breaking change then)?
I dont see when --persist option is needed (but i assume there are few cases), but in all options that I can think about its harmful, and thats why default should be in my opinion to not persist.

So to summarise.

When build with:

VERSION 0.7

test:
    CACHE /some

Would persist cache as currently (so would not be a breaking change).

If build with:

VERSION --cache-not-persist-option 0.7

test:
    CACHE /some

Would not persist it (not a breaking change because guarded by option).

If build with:

VERSION --cache-not-persist-option 0.7

test:
    CACHE --persist /some

Would persist it.

[alexcb]

But since we will be adding flag maybe then default can be reverted (it will not be breaking change then)?
I dont see when --persist option is needed (but i assume there are few cases), but in all options that I can think about its harmful, and thats why default should be in my opinion to not persist.

This would mean when users upgrade to VERSION 0.8, it would break the behaviour, which would suprise users who simply update the number without reading the CHANGELOG -- I think I'm ok with that change, since persisting the cache leads to really large images; thoughts on this change @vladaionescu ?

[vladaionescu]

Yeah, it feels like a good decision. It is sometimes surprising to users that this auto-persisting is taking place, and sometimes it affects performance due to the size of the cache.

[ingwarsw]

This would mean when users upgrade to VERSION 0.8, it would break the behaviour, which would suprise users who simply update the number without reading the CHANGELOG -- I think I'm ok with that change, since persisting the cache leads to really large images; thoughts on this change @vladaionescu ?

Thats something I didn't take into account..
But that should still not break much..

ps. what are the use cases when saving cache is essential?

[ingwarsw]

Wrong button :)

[vladaionescu]

what are the use cases when saving cache is essential?

Sometimes users will want to populate some cache in one target, and reuse most of it in another target. For example: download dependencies in one target, but then have those dependencies around when compiling in another target. The pattern is very common in some languages.

[ingwarsw]

what are the use cases when saving cache is essential?

Sometimes users will want to populate some cache in one target, and reuse most of it in another target. For example: download dependencies in one target, but then have those dependencies around when compiling in another target. The pattern is very common in some languages.

But as far as its in earthly its way faster to just mount cache in that other target.
Only place I need it may be needed its maybe python or other languages like that where you would need cache folder in final image.. but then I would never use shared cache cause it may be waaaay bigger than I actually need.

[alexcb]

Thanks for the changes @ingwarsw ; I just opened #3527 (with a git commit --allow-empty) to trigger our tests, which unfortunately depend on some secrets that are only available to members of the earthy repo. If they pass we'll be able to merge this PR.