search

earthly / lib

Mozilla Public License 2.0
7 stars 11 forks source link

`rust+CARGO` build should assume output? #27

Closed adamgordonbell closed 9 months ago

adamgordonbell commented 9 months ago

The rust+CARGO UDC is great. One thing that tripped me up was --output="release/[^\./]+"

If no output is supplied could we just assume everything in /target/ should be the output? At least if doing cargo build, I think I would usually want the output of it somewhere accessible.

idelvall commented 9 months ago

Hi Adam, thanks! The original implementation worked that that way, but we soon realized that the target cache grew significantly even with cargo-sweep installed (more than 10GB in some cases), so we were not adding significant runtime overhead by copying all the files (most of then not needed), we were also significantly polluting the build layer cache. Another thing I considered was instead of adding an "--output" option add a "--save-artifact" one, so no need for redundant information, but I think this is cleaner

adamgordonbell commented 9 months ago

Makes sense. I'm just talking about a default. So you could still use the output flag, but if you didn't, you wouldn't be surprised that you ended up with no files. Keep in mind the user of the UDC has no idea how it works internally and probably just wants the output of their cargo call.

Maybe its too early to say. If other people use it and get tripped up by this it might make sense to consider.

idelvall commented 9 months ago

The output is an ARG in the UDC, so it's not an implementation detail, but even if a user ignores what it does, I think it's better to force them to read the API documentation by don't outputting anything, rather than giving them the output without being aware of the side effects of that as the cache grows. At least that is what I would prefer, but yes, let's wait to see how people react to all of this.

Thanks Adam!