Running build shells on non-build-elements [Was: Updated 2.0 milestone]

[Tristan Van Berkom <tr...@codethink.co.uk>]

Hi again Abderrahim,

On Mon, 2022-01-10 at 18:20 +0100, Abderrahim Kitouni wrote:
> Hi Tristan,
> 
> Le lun. 10 janv. 2022 à 06:23, Tristan Van Berkom
> <tr...@codethink.co.uk> a écrit :
> > > I'd add to the list
> > > https://github.com/apache/buildstream/issues/1263
> > > I think this is potentially (Caching buildtrees only works with BuildElement)
> > 
> > This, I think what we're really talking about here is script elements,
> > or similar concoctions, and we probably don't want to cache anything
> > for a variety of elements (like compose or filter elements).
> 
> As is probably clear from my message, I pressed send without finishing
> my sentence.
> 
> The issue isn't really about caching buildtrees, it's about running a
> shell in a failed sandbox. As such elements that don't have a sandbox
> (BST_RUN_COMMANDS = False) like compose or filter are not concerned by
> this at all. Indeed, this is mainly about the script element and
> possibly other custom elements (whether implemented using
> ScriptElement or not).
> 
> The issue is currently that this core functionality that works fine
> with bst 1 simply doesn't work with anything that isn't a
> BuildElement, which would be a regression. I'm not advocating for
> supporting build trees for more elements, but rather to have an
> alternative for elements that don't have a build tree to still be able
> to shell into a failed sandbox, especially for local builds.

So what is unclear to me, is how serious exactly this regression is and
what important use case is not covered by what I proposed, which I
consider to be a pretty straight forward approach to solving the issue
(and doesn't require any real changes afaics except for a better error
message).

For instance, it's clearly not important to run a build shell on a
stack, filter or compose element.

For a script element, we *could* set `%{build-root}` to "/", but that
is probably better off being a choice which a user can make, since
doing so implies caching a potentially huge build tree (probably a good
choice though when configured to only cache failed builds).

The discussion on the issue at:

    https://github.com/apache/buildstream/issues/1263#issuecomment-773013851

...suggests a complicated approach which requires some careful
recording and consideration of "marked" directories in the sandbox, and
what purpose they are marked for, which strikes me both as overkill and
more fragile than requiring plugins to store working data in the
`%{build-root}`.


On regressions in general... we have to consider that bst-2 is a
different beast with a different feature set.

Some features just aren't there, we no longer support `bst build --
track` or tracking in conjunction with any other command.

Also it's possible for the user to decide to never cache build trees
(even failed ones), in which case I think it's perfectly sane to just
say that caching of failed builds *must* be enabled if you want to use
build shells at all.

Cheers,
    -Tristan