Re: [BuildStream] Feature proposal: Add --build option to bst checkout

[Tristan Van Berkom <tristan vanberkom codethink co uk>]

Hi Sander,

On Thu, 2018-09-20 at 12:02 +0100, Sander Striker wrote:
> Hi,
>
> > On Thu, Sep 20, 2018 at 9:36 AM Tristan Van Berkom via BuildStream-list <buildstream-list gnome org> 
> > wrote:
> > Hi Chandan,
> >
> > On Wed, 2018-09-19 at 22:14 +0100, Chandan Singh wrote:
> > > Hi,
> > >  
> > > I was considering adding a `--build` option to `bst checkout` that would alter
> > > the behavior of this command similar to how `--build` works in `bst shell`.
> > > So, instead of checking out an element (and its runtime dependencies),
> > > BuildStream would checkout its build dependencies and sources instead. This
> > > checkout will look the same as the file system tree of the build sandbox.
> > >  
> > > `bst shell --build` is very useful in debugging a build but there are some
> > > scenarios where it is not quite suitable. Here are some examples where
> > > `checkout --build` might be more useful:
> > >  
> > > - if BuildStream is running as a remote execution client, `shell --build` will
> > >   be unusable
> >
> > How is this a foregone conclusion ?
> >
> > If we can setup a shell on a remote worker for the purpose of launching
> > a build, certainly connecting the user's terminal to that remote shell
> > is not immensely onerous - I would frankly expect this to "just work".
>
> Your expectation is not true.  The Remote Execution API is meant for feeding it async jobs.
> Just think of remote execution of helping by supplying more cores during bst build.
>
> I fully expect bst shell to not use the RemoteSandbox.

I have a real concern here that we are allowing implementation details
to drive design, instead of the other way around.

That said, this is a rather minor limitation, but will be more annoying
on platforms which cannot support running the remotely built binary
format locally.

We should not discard this as impossible just for the sake of the
implementation currently dictating this to us, and at least keep a
close eye on how to tackle the problem later on.

> > That said, I would also not expect to be able to launch graphical
> > applications remotely which access my own display server, or audio;
> > however with a bit of tinkering (forwarding the correct DISPLAY etc),
> > it should be still possible with some performance degradation.
> >
> > Also, if you are using remote execution on a host which is compatible
> > with the target you are building (currently only linux binaries but
> > this should change with time), you should be able to download the
> > artifacts and sources from this build network as an initialization step
> > of `bst shell`, and have the shell execute in a local sandbox even if
> > the build executed remotely.
>
> Right, but that is with the local sandbox.  I think what Chandan was
> saying is, if a local sandbox is not available, bst shell won't
> work.  And in the scenario of a remote execution only client, that is
> the case.

As a limitation, which we should keep an eye on possibly fixing, one
approach might be that we support spinning up sandboxes in VMs locally
on platforms which cannot support the remotely built binary format, and
download the remote artifacts, shell into the VM - there are several
ways to look at it the problem.

We should not write off the problem completely, nor should we consider
the problem to be a blocker.

That said, limitations of shelling into a remote sandbox will still
exist, and exporting data for users is something BuildStream should do,
nobody can argue against that :)

> > > - such a checkout can easily be imported into other systems, like Docker, which
> > >   can be useful for debugging purposes
> > >  
> > > - in certain cases, it will allow users to involve host tools for debugging
> >
> > For the above two points, as already outlined in this email[0], my view
> > here is that you have this entirely backwards.
> >
> > The shell is an integral part of BuildStream, we need to work on making
> > it perform better (for which I have another proposal brewing), instead
> > of resorting to circumventing it.
>
> I think we still need to do proper analysis of what is more
> accessible to users.  The user experience is key here; if people find
> easier methods that achieve the same, then that is what they'll use. 
> This is my major concern.

We can always export things and users can always do what they want with
what they export, yes.

That said, currently the motivating factor for users to chose a
different technique, is due to `bst shell` not being performant enough,
we should be able to offer a solution for edit / compile / test cycles
which does not involve an export / import dance at all.

As I mentioned in my reply to Chandan, that is a separate proposal
which is still brewing.

> > > Proposed changes
> > > ================
> > >  
> > > - Add `--build` option to `bst checkout`
> > >  
> > > I think this option should also be mutually exclusive with `--deps [none,run]`
> > > as that makes sense in the context of checking out built artifacts, whereas
> > > `--build` makes sense in the context of checking out the sources. We can also
> > > consider adding `--deps build` option instead but in my opinion that may be
> > > more confusing for the user.
> >
> > On the one hand, I feel that your arguments for proposing this in the
> > first place do not hold much water.
> >
> > On the other hand, it *seems* that this proposal is very easy to
> > implement, and should not impose onerous complexity in the tooling
> > moving forward.
> >
> > I think I have a counter proposal that I would prefer here, actually:
> >
> >   * Let's add `--deps build` to `bst checkout`
> >
> >     This does what you would expect, it only checks out the build
> >     dependencies.
>
> Didn't we propose to have checkout become a subcommand of the bst
> artifact group?  Would this still work without the element context?

This is a very good point, I had overlooked this.

I think that this can and should indeed still work with checkout as an
artifact subgroup command indeed, however it might be limited to what
you have in your artifact cache.

In other words:

  * It should not be a problem to discover what the build dependencies
    are by traversing artifacts and reading their metadata.

  * It might bail out and ask you to pull in some build-only
    dependencies or build them, if those are not accessible.

However, it will not make sense at this point to checkout the sources
of an artifact, unless perhaps we are checking out a cached build tree.

> >   * Let's add a *separate* command for checking out sources.
> >
> >     This would have the advantage of clearer symmetry with
> >     `bst checkout`, and could offer the same `--deps` options
> >     and `--except` options which we do in `bst show` and such.
> >
> >     This way we offer the same API as `bst show` for expressing
> >     the multiple element from which you might want to checkout
> >     sources.
> >
> > With this approach, you would get some added flexibility - there are
> > other reasons one might want to only checkout the sources (maybe we
> > want to ship all of the exact sources which went into an appliance,
> > except for a few proprietary bits, to a customer, for license
> > compliance ?).
>
> That does seem like a reasonable feature.
>  
> > Even if you want to checkout sources in side a `bst checkout` directory,
> > you might want more sources than just the target of a `bst checkout --build`
>
> I could see it be more convenient to have one command though.

Understood, I also feel like it could be convenient added sugar, and
have highlighted my concerns regarding this getting out of control in
my other reply to Chandan.

Cheers,
    -Tristan