cli: Add --buildtree and --buildroot to bst artifact checkout

[ownik]

This PR adds a "--use-buildtrees" option to "bst artifacts checkout", similar to the existing option in "bst shell".

The goal is to make it possible to check out build artifacts from the sandbox build tree when it is available.

Would be okay with adding this functionality, or are there reasons not to expose it through "bst artifacts checkout"? Or is there another way to get build artifacts from sandbox now?

Fixes #1822

[ownik]

@BenjaminSchubert @abderrahim @cs-shadow @gtristan @juergbi I've made changes to verify that buildtree exists in the cache, similar to the shell command. Could you please review, please?

[abderrahim]

Looks like this is fixing #1822, please link the issue in your PR. Also please add a test for this behaviour.

I'll try to review it today.

[abderrahim]

A few things need to be considered in the design of this feature:

• Someone recently suggested to implement this as a flag to source checkout rather than artifact checkout. It kinda makes sense that the build tree is related to the source, but it's also stored as part of the artifact. Maybe it makes sense to move it to a separate toplevel command? something like bst buildtree checkout?
• This is currently checking out the buildroot rather than what is traditionally considered a "build tree". This means it can work with both build elements and non-build-elements, but it also means we get a whole sandbox with the dependencies, the build tree, and even the artifact (if the build succeeded) which is probably not what people asking for the build tree are looking for.
• if we go with a new toplevel command, we might want to expose options to checkout either the build tree or the build root.

[ownik]

@abderrahim Thanks for the feedback! I've reworked the implementation into a new bst buildtree checkout command. By default it checks out the buildtree, and a --buildroot option can be used to export the entire buildroot instead.

I've also added an integration test covering the new command and link the issue in PR.

[ownik]

@abderrahim Any chance to review this PR?

[abderrahim]

Thanks for the contributions. See my comments below.

[abderrahim]

Does this make sense for the build tree? Either we're checking out just the buildtree, and then dependencies don't make sense, or we check out the whole buildroot and it already contains the build dependencies.

Am I missing something?

[ownik]

Sorry for the delayed response, and thanks for pointing this out.

Yes, you're right. I've changed this to element: Element = elements[0], is it ok?

[juergbi]

Someone recently suggested to implement this as a flag to source checkout rather than artifact checkout. It kinda makes sense that the build tree is related to the source, but it's also stored as part of the artifact. Maybe it makes sense to move it to a separate toplevel command? something like bst buildtree checkout?

I think bst artifact checkout was actually a reasonable place for this feature. The build tree is part of the artifact. While the source is normally also part of the build tree in BuildStream, the buildtree checkout still operates solely on the artifact and there is no direct involvement of the source (plugin). The build tree is (optionally) pushed/pulled as part of bst artifact push/pull, not bst source push/pull and I assume we also don't intend to add bst buildtree push/pull. Or are there any ideas for further bst buildtree subcommands, which could make the new buildtree command group more useful?

If most people feel a separate top-level command makes sense, I won't object, but I'm not personally convinced so far. While this is not a democracy, feel free to 👍️ on this comment if you prefer this feature to be in bst artifact checkout or 👎️ if you think bst buildtree checkout is a better fit.

[ownik]

@juergbi Agreed, I've moved it to bst artifact checkout as --buildtree and --buildroot options and dropped the bst buildtree group. This also lets it reuse the existing --tar/--compression/--hardlinks/--directory handling. --deps and --integrate are rejected with these options, as only the target artifact is used.