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



Hi Tristan,

Thanks for the quick reply, please find my comments inline.

On Thu, Sep 20, 2018 at 8:36 AM Tristan Van Berkom
<tristan vanberkom codethink co uk> 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".

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.

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".


- 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.

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.


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.


  * 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 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.

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.


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`

Cheers,
    -Tristan

[0]: https://mail.gnome.org/archives/buildstream-list/2018-August/msg00043.html



Regards,
Chandan
On Thu, Sep 20, 2018 at 8:36 AM Tristan Van Berkom
<tristan vanberkom codethink co uk> 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".

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.

- 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.

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.

  * 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 ?).

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`

Cheers,
    -Tristan

[0]: https://mail.gnome.org/archives/buildstream-list/2018-August/msg00043.html



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]