Re: Proposed changes to CLI pipeline specification



On Mon, Nov 6, 2017 at 6:30 PM Tristan Maat <tristan maat codethink co uk> wrote:
Hi all,

As you may have noticed, we recently merged a branch that allows
specifying multiple targets for most BuildStream commands - in the
wake of this we had a closer look at how pipelines should be specified
on the command line.

Since this is expected to be fairly static after the 1.0 release, we
are blocking 1.0 on a few issues regarding this - namely #113 and
#117, which revolve around `bst build --track` and #131, which
currently blocks #117.

The discussion of these changes has spread over multiple issues and
IRC, so to make sure we keep track of the whole picture here is an
overview of the related issues and all discussion on them:

Overview
--------

I am proposing to change the following:

- `--except` behavior should change as follows:
   - Load the excepted element as part of the pipeline
   - Either:
     - Recursively remove all children of the specified element,
       regardless of other dependent elements
     - Recursively remove the /intersection/ of the specified element
       with normal targets, making sure that dependencies not included
       through the intersection remain part of the pipeline
- `bst build --track` should be split into the following options:
   - `bst build --track <element.bst>` to specify individual elements
   - `bst build --track-recurse <element.bst>` to specify an element and
its dependencies
   - `bst build --track-except <element.bst>` to except elements
included using the previous commands
   - `bst build --track-save` to enable saving tracked references

Modifications to the `--except` argument
----------------------------------------

Current Implementation
======================

`--except` is currently not particularly useful, as the example given
in #113 shows:

`bst track --deps all --except core/meta-gnome-core.bst
apps/meta-gnome-apps-tested.bst`

Since `apps/meta-gnome-apps-tested.bst` does not depend on
`core/meta-gnome-core.bst`, the command fails with `No element named
core/meta-gnome-core.bst`.

This is not at all what the user would expect - for one, that element
does indeed exist, it is just not part of the current pipeline.

The user also probably intends to remove any elements of that stack
from the track pipeline - to do so, however, they would have to
specify each element that is actually in the pipeline
individually. This is obviously not an option for large projects.

Proposed Changes
================

First off, BuildStream should first load elements specified using
`--except`. This avoids the problem that --except targets may not be
part of the pipeline.

To resolve the other issue, that excepting stack targets does not
currently do anything, there are two options:

- The excepted element and **all** its child nodes are removed
   recursively from the pipeline, unless they are an explicit target
   element.

   This has the advantage of being fairly simple to understand, and
   being slightly safer - since the `--except` targets will always be
   **exempt** from whatever action is performed on them, this results
   in the least accidental operations if the user makes a mistake.

   The above example would then work /as is/, but some elements may
   have to be re-included as additional targets.

- The /intersection/ between the excepted and target elements'
   children become the "excepted" elements, and recursively remove
   their children, except when those children are depended on by
   elements that are not children of the "excepted" elements.

   /intersection/ here refers to the topmost elements at which the
   excepted dependencies and the normal target dependencies cross.

   This has the advantage of neatly sorting out dependencies where
   problems could arise. If say we want to build an element that
   depends on 'zlib', which happens to also be in the toolchain, this
   approach would track zlib with the parent element even if the
   toolchain is excluded, whereas the other approach might end up with
   an outdated zlib.

   In this case the above example would work /as is/, with no further
   additions to the command line.

I like the intersection option better.  I have a feeling that without that, you either end up not doing exceptions because it is too tedious to add everything back that you want to build, or, you end up with a long list of elements to explicitly build.

I see the use case of the stack element, and the desire to except it and all its descendants.  However, I also see the case of excepting just a single element.  Should we introduce the same --except and --except-recurse to allow for that flexibility?  Or is that too much?

Another path could be to introduce an option that takes a file containing all elements to exclude.  That file could be produced by taking the output of an incantation of `bst show` on an element to exclude, if you want to exclude its descendants.
 
More granular selection of elements to track with `bst build --track`
---------------------------------------------------------------------

Current Implementation
======================

Currently, `bst build --track` will simply track all dependencies of
the built element, and write new refs to their element files.

This may be an issue when we don't want to download a new base runtime
to simply build a new version of a (local) element - for example just
attempting to run `bst build --track core/cheese.bst` to run CI on a
new version of cheese in gnome modulesets.

It also leaves debris in the project files, forcing the user to run `git
--reset`
before building the latest version of a project, which can be mildly
annoying.

Proposed Changes
================

To make the `bst build --track` functionality more flexible it was
proposed to add more of the `bst track` functionality to `bst build`,
namely:

- Allow specifying an element as an argument, such as: `bst build
--track <element.bst>`
- Allow specifying multiple track targets
- Make `bst build --track <element.bst>` only track the specified element
- Add `bst build --track-recurse <element.bst>` which tracks an element
and its dependencies
- Add `bst build --track-except <element.bst>` to except certain
elements from the tracking pipeline

Additionally, we would add a `bst build --track-save` flag, which simply
causes BuildStream to save tracked
references, while making the default behavior to only use new refs for
the current build.

To stay consistent with `bst track`, perhaps `bst build
--track-recurse` should simply be `bst build --track`, since it is
already possible to exclude unwanted children.

While it would be possible to exclude unwanted descendants, I can see the use case where all descendants are unwanted.  If we want fully symmetry, then have `bst track` grow a --recurse option and don't recurse by default?
 
Some neat commands that would now be possible:

- Run CI on an element whose git repo just changed, avoiding downloads
   and ensuring dependencies don't show unrelated breakage:
   `bst build --track core/cheese.bst core/cheese.bst`

- "Build, fixing my pipeline, please":
   `bst build --track-save --track-recurse core/cheese.bst core/cheese.bst`

- "Build, fixing my pipeline, but please don't download 3GB for base again":
   `bst build --track-save --track-recurse core/cheese.bst
--track-except base.bst core/cheese.bst`

- "Build, fixing my pipeline, but please don't download 3GB for base
again, though I need a new ninja":
   `bst build --track-save --track-recurse core/cheese.bst --track
base/ninja.bst --track-except base.bst core/cheese.bst`

- "Build the latest version of this project for now, but don't override
my refs, I'm debugging something weird":
   `bst build --track-recurse core/cheese.bst core/cheese.bst`

Another possible change that was brought up was the addition of a flag
which only updates refs "when necessary", i.e. a flag that allows only
tracking sources that don't have a ref in the first place. (see
https://gitlab.com/BuildStream/buildstream/issues/117)

Since this is perhaps better captured by the also proposed concept
of *tracking domains* it seems like it would later be deprecated. It
also looks like the change would only **add** API, so if this becomes
necessary it can always be added later.

===

Hopefully this is not laid out too confusingly, it covers a wide range
of changes...

My main questions are:
- Do the proposed changes seem sensible and future-proof?
- Am I missing any use cases for `bst build --track`?
- How should `--except` resolve?

Please let me know your comments!

Tristan Maat
_______________________________________________
Buildstream-list mailing list
Buildstream-list gnome org
https://mail.gnome.org/mailman/listinfo/buildstream-list


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