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

[Sander Striker <s striker striker nl>]

Reviving this thread in light of the bst artifact subcommand group thread.  Mostly from a consistency perspective, if we are creating:

    bst artifact pull ...

    bst artifact push ...

    bst artifact checkout ...

    bst artifact log ...

Should we not also settle on:

    bst source fetch ...

    bst source checkout ...

?

Cheers,

Sander

On Sat, Sep 22, 2018 at 10:47 PM Chandan Singh <chandan chandansingh net> wrote:

Hi all,

Since we were mostly in agreement, I thought it would be a good idea
to send a short summary email.

- Merge request !819 has been created to add `--deps build` option to
`bst checkout`. [1]

- Merge request !820 has been created to add `bst source-checkout`
command. This command currently comes with minimal options (`--deps`
and `--except`) but options like `--tar` can be added later. [2]

- Issue !672 has been created to handle retirement of `souce-bundle`
command since its functionality is very limited in scope and is also
not used very often. Its functionality can e supported by extending
`source-checkout` command via additional options. [3]

- Once !819 and !820 are merged, I can create a separate merge request
to add `checkout --build` option that will basically be syntactic
sugar for  doing `bst checkout --deps build <some-element>
<some-directory>` + `bst source-checkout [--deps none] <some-element>
<some-directory>`.


[1] https://gitlab.com/BuildStream/buildstream/merge_requests/819
[2] https://gitlab.com/BuildStream/buildstream/merge_requests/820
[3] https://gitlab.com/BuildStream/buildstream/issues/672

Cheers,
Chandan

On Thu, Sep 20, 2018 at 12:23 PM Tristan Van Berkom
<tristan vanberkom codethink co uk> wrote:
>
> On Thu, 2018-09-20 at 10:17 +0100, Chandan Singh wrote:
> > Hi Tristan,
> [...]
> > > > - if BuildStream is running as a remote execution client, `shell --build` will
> > > >   be unusable
> > >
> > > How is this a foregone conclusion ?
> [...]
> > Sorry for jumping the gun on this one. I should have been more careful
> > with the phrasing. Let me say why I am unsure about this - 1. I am not
> > sure how interactive shells fit with the remote execution API, and 2.
> > providing interactive shell access to the remote execution farm may
> > not be desired in some cases as it can very easily increase the load
> > heavily on the farm.
> >
> > Even if we worked around (1) and implemented something of our own to
> > support remote interactive shells, I think there is still need for
> > something like this when one is working on such a project offline.
> > Without such an option, it becomes difficult to see "what the build
> > sandbox exactly looks like".
>
> I think we are mostly in agreement, I just want to add here that
> basically, our implementation needs to continue to be design driven,
> not the other way around.
>
> We have a tool which `bst shell` is an integral part of it's workflow,
> and we are adding remote execution to the mix. This means that we
> should be designing for all the BuildStream use cases which already
> exist, with the new possibility that a sandbox might now be remote.
>
> Of course there are going to be limitations on remote sandboxes.
>
> > > > - 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 am not saying that we shouldn't make shell better, but I am asking
> > about allowing people to use whatever tools they want to debug when
> > things are not working. Being able to layout the files in the same way
> > as they would be inside the build sandbox allows one to do things that
> > would otherwise not be possible in vanilla bst shell.
> >
> > For example, if I import the entire build sandbox filesystem in a
> > Docker image, other things can easily be layered on top of it, the
> > image can be persisted for longer periods of time so that one could
> > back to it later, and also shared with other people. This is one
> > example but I am sure different host tools will allow us to different
> > kinds of things.
>
> This is partly touching on the other proposal which I have brewing, I
> have been exploring some options to make `bst shell` performant enough
> so as to not lose the developers attention, I am edging towards a
> situation where we *maintain* an already staged sysroot which is
> updated in real time as a consequence of running builds; such that
> startup time of `bst shell` is reduced to almost nothing.
>
> But this is a separate topic.
>
> > So, while I totally agree that we should make the shell experience
> > better, I also think that if we can enable other ways of debugging
> > without much cost to us, we should consider doing that.
>
> I completely agree that users should be able to extract what they need
> from BuildStream, yes.
>
> > >
> > > > 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.
> >
> > This could work but it will create an inconsistency with how other
> > choices to `--deps` work, i.e. `run` and `none`. The `--deps` option
> > currently means - which dependencies should be checked out _in
> > addition_ to output of the specified element. `--deps build` would be
> > different from the existing options as it would checkout the output of
> > its build dependencies _instead_ of its output. This is the
> > inconsistency I was initially trying to avoid, but if you think it's
> > okay then we can go with this option.
>
> This is not an inconsistency for the `--deps` option as far as I can
> see.
>
> The `--deps` option is symmetric across all commands where it appears,
> however not all options are allowed for all commands. For a succinct
> description, I refer to the output of `bst show --help`:
>
>       none:  No dependencies, just the element itself
>       plan:  Dependencies required for a build plan
>       run:   Runtime dependencies, including the element itself
>       build: Build time dependencies, excluding the element itself
>       all:   All dependencies
>
> >
> > >
> > >   * 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 ?).
> >
> > Adding a different command for sources make sense. I was trying to
> > avoid in order to keep the number of bst commands minimal but I don't
> > have any objections to it as it may also be useful in other scenarios.
>
> I am a bit uncomfortable with that as well, but I think consistency in
> how our commands work is better than lumping in more options onto one
> command, to save us from another command.
>
> > I still think that `checkout --build` would create a nice symmetry
> > with `shell --build` as both shell and checkout would otherwise focus
> > on the built artifacts and not the sources. `bst checkout` essentially
> > checks out what you get inside `bst shell` so it would have made sense
> > if `checkout --build` checked out the same thing as what you would get
> > inside `shell --build`. While I think this symmetry would be nice to
> > have, if you think other option is clearer, I don't have issues with
> > that.
>
> What I don't like about this is:
>
>   * We are lumping in more functionality into the checkout command
>
>   * The new functionality is limited in how it can be used, while
>     there are other use cases for checking out the exact sources
>     for a given project / element state.
>
> That said, while I am more interested in a more flexible / useful
> command, I would not object to this addition of `--build` as an
> addition, which uses the same underlying codepaths.
>
> The point here is that; people will keep wanting more, if you give them
> `bst checkout --build` without giving them `bst source-checkout`, then
> the next proposal will naturally be to extend `bst checkout` more and
> more (can I now have --build-all-sources please ? can I now have a
> --build-sources-except option too ?).
>
> We should nip that ugly possibility at the bud first, and just provide
> something flexible on the outset, in that light a `--build` option is
> just sugar added to `bst checkout`.
>
> > Sidenote: If we add a new command for sources, should we also
> > re-evaluate the source-bundle command? Perhaps that could be folded in
> > the new command for source. For instance, if we could have something
> > like `bst source --deps build --build-script` that does what
> > souce-bundle currently does. Just a thought.
>
> The source-bundle is a bit of an ugly, underused, undertested beast.
>
> It includes support from the Elements which each in turn create build
> scripts, and it outputs a tarball.
>
> I don't know what to do about source-bundle, I think we need to leave
> it as it is for now, but as it is underused, we might rather:
>
>   * Create bst source-checkout
>
>   * Have a more uniform / symmetric output for it, allowing directory
>     or tarball or tar streaming as we do with checkout
>
>   * Add an option as you describe, to allow build script generation
>     in the output
>
>   * Deprecate `bst source-bundle`, removing it from the default help
>     output, and supporting it only as a matter of backwards
>     compatibility.
>
>     Maybe even remove it entirely, along with the other CLI breakages
>     we've been discussing for 1.4 regarding workspace commands.
>
> Cheers,
>     -Tristan
>


--
Regards,
Chandan Singh
http://chandansingh.net/

--

Cheers,

Sander