GNOME.org
 

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

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/
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]