Re: [BuildStream] Proposal: Workspace related DX features & design



Hi Sander,

On 24/08/18 18:02, Sander Striker via BuildStream-list wrote:
Hi,

On Fri, Aug 24, 2018 at 2:17 PM William Salmon via BuildStream-list
<buildstream-list gnome org <mailto:buildstream-list gnome org>> wrote:

    Hi all,

    As recently raised by Tristan [1], there are numerous related issues
    & hanging 
    merge requests revolving around improvements & new functionality for
    BuildStream 
    workspaces. As such it make sense to address them in a common
    proposal which 
    hopefully addresses & highlights any overlapping or common changes
    needed to 
    meet the respective goals. Tackling & grouping related requests
    together also 
    means that if we have to conclude that a CLI change is needed [2],
    then it can 
    be done once to minimise & hopefully mitigate further changes. It's
    also 
    important to understand how any changes to the CLIcould be consumed,
    in the 
    sense of a user (porcelain) or in automation (machine readable) so
    any feedback 
    from those perspectives is welcome.

    Overview
    ========

    We want to enhance the usability of workspaces in BuildStream. This
    includes 
    increasing the flexibility of the existing implementation (such as
    supporting 
    new defaults for workspace location & names [3]), QoL changes (such
    as the 
    ability to use relevant bst commands inside workspaces, and bst
    commands 
    targeting multiple elements, [4][5]) and new additions for aiding
    development
    & debugging (such as cached workspaces & multiple workspaces in
    shell, [6][7]). 
    Some of the issues raised in the call for proposal are months old
    and as such 
    some of the included comments have now been rectified or made
    redundant;as 
    such consolidation is needed.

    When reviewing the state of the tickets that extend the support for
    debugging 
    & development within the shell, it has been highlighted that cached
    build trees 
    may be the more appropriate option instead of extending workspaces.
    As such it 
    may be more appropriate to create a new bst command for this
    workflow & consider 
    it outside of this scope. Remote execution should also take
    advantage of build 
    trees (for failed build artifact debugging [8]) and not need any new
    API changes 
    to workspaces, however if there is disagreement we should capture
    those changes 
    in this proposal. Once a change has been developed in a workspace it
    would be 
    desirable to have an easy way to get this information into the
    respective 
    upstream/vcs (or even just locally for the projects consuming it);
    we think this 
    may need to expand the workspace API but not change any existing
    features e.g. 
    `bst workspace patch create` (or a documented patch/quilt workflow)
    or similar.

    Proposal
    ========

    These are the workspace related features to be considered together:

     * run bst commands in workspace directories, including defaulting
    command to 
       said workspace element.
     * Make workspace creation easier.
       - Automatic names.
       - Default location.
       - From cached build.
       - Make multiple workspaces.
       - Relative paths outside of project.

 

     * Debug, workspace/cached build may be used to preserve the
    required artifacts 
       so including this here.

    Implementation
    ==============

    Comands in workspaces
    ~~~~~~~~~~~~~~~~~~~~

    This was started by chandan/cs-shadow in [9] however it leaves some
    questions.

    [9] uses a file in ~/.local/share to achieve this but then it cannot
    easily 
    implement relative workspaces. We propose using a `fragment` file
    that we will 
    create in a .bst/project.yml of a workspace's root directory. This
    is similar 
    to how git submodules identify which directories in the main project
    are 
    subdirectories.


Makes sense.  Not sure about the terminology, but let's leave that as an
implementation detail :).
 

    Support for multiple projects pulling a workspace is not allowed by
    default but 
    it can be forced with known side-effects. Default behaviour is to say a 
    workspace already exists at a given location with a given name. If
    forced to be 
    a workspace for multiple projects then the original parent project
    takes 
    precedence (i.e. is the default resolution unless overridden) (bst
    commands 
    again need to be forced & warn the user that it may affect other
    projects 
    consuming the shared workspace).


I don't know how common this use case of having multiple projects refer
to the same workspace is going to be, but it doesn't sound too expensive
to support it regardless.

Yes to be frank it's a situation that I'd like to avoid, however I can
see some benefits when working say on a crucial dependency that is
in-flight which is being actively consumed by many projects and as such
debugging from a single source point may be more efficient (of course,
this relies on the base source ref being the same). As stated it would
be something that a user would have to force, I think giving a warning
is about as expensive as it should get.


    We will add a `fragment` to workspaces that points back to the
    project which
    created it. This will allow for commands run in workspaces to find
    the project
    that they should run from. Also it allows for projects to check for
    two projects 
    using the same workspace.


The latter being important for I assume for refcounting purposes so that
a bst workspace close doesn't end up cleaning up a workspace in use by
another project?

Correct, also as stated later on where the default resolution of none
'destructive' commands associate to at a project level.


    The fragment will also be relative if the workspace was created as
    relative to 
    allow for moving workspaces.


I am having a bit of trouble parsing this sentence.  Could you expand on
this a little?

This is covered in the workspace creation commands section. The idea
here is that if the workspace was generated with a relative path (either
at a default/conf level or at a cli level) then the path stored in the
fragment for the project location should be stored as relative

 

    Workspace creation commands
    ~~~~~~~~~~~~~~~~~~~~~~~~~

    Note: Defaults already exist at a project and system level so we
    will just store 
    the default workspace location information alongside it.

 

    We will change from:
    bst workspace open -element.bst workspacename
    to:
    bst workspace open --directory /path/to/workspaces element.bst
    [workspacename]


I'm assuming that the Note you started with regarding defaults makes the
--directory optional?  The main reason for double checking is because I
want the command to be as simple as possible:
  bst workspace open element.bst

Yes in this example both '--directory' & 'workspacename' are to be
optional if defaults have been set (be that .config or project.conf)


    When opening a single element then you can:

    bst workspace open --directory /path/to/workspaces --multi
    elementA.bst elementB.bst


Why do we need the --multi argument?  That doesn't seem intuitive.  If
it is to get around the fact that otherwise elementB.bst would be
interpreted as the workspacename, then I don't think we should do that. 
Otherwise you end up with an open workspace carrying the name of the
second element you wanted a workspace open for.

Yes it is to mitigate the situation you've mentioned. Will has some
thoughts about handling this with click in alternative ways, however as
it stands the most sane thing seemed to be the use of an additional flag.

 

    The directory option specifies the directory, in which the folder 
    containing/(the root of) the workspace will be created. The folder 
    containing/(the root of) the workspace is either the "workspacename"
    from the 
    command line or if not given then it is the element name without the
    trailing
    ".bst".

    eg. `bst workspace open --directory /path/to/workspaces elementA.bst`

    would create:
    path/to/workspaces/elementA/
    as the root or the workspace

    and `bst workspace open --directory /path/to/workspaces element.bst
    workspacename`

    would create
    path/to/workspaces/workspacename/

    If the directory is relative, e.g.not starting with a "/" then we
    treat it as
    relative to the project and the workspaces fragment will be in
    relative terms,
    so you can move both or, more likely, the folder that contains both.


Makes sense.

What is the sensible default location used when no workspace location is
defined?  'workspaces' as a subdirectory of the project directory?  Or
will not having it set result in a helpful failure message?

My preference would for be the command to behave as is to not break any
compatibility with any existing automated workspaces. As such I'd
propose to keep the existing helpful failure message, all be it reworded
to include the availability of defaults. Having workspaces as
sub-directories of a project is also a workflow in which Tristan has
been an advocate of not recommending (see issue #191), so making this
default behavior would be somewhat challenging that.

 

    In anticipation of a feature to allow a workspace to be created from
    a build 
    tree we would also add a pair of exclusive options (--no-cache
    --use-cache) that 
    would override the buildstream/system/project default option for, if
    a cached 
    build should be used. The open command will only accept a single one
    of these 
    options and so if you want some of the elements to be cached and
    some not, you 
    will needto call the command twice - once with the elements to be 
    cached and once for those to be created without.  As we propose to
    land all the 
    CLI changes to `workspace open` command in one MR, we may need to
    land the 
    option before the feature is ready.In this case we will have bst
    issue a "not 
    implemented" error if the --use-cache option was used.


Makes sense to me.
 

    Moving projects
    ~~~~~~~~~~~~~~~

    This will only be supported if the workspaces are defined with a
    relative path.

    Note: you could fudge by manually changing the fragment or we could
    add a
     utility to automate this.

    This is because otherwise the fragment will get lost and you cannot
    run bst 
    commands from the workspace. If run in the project then the fragment
    check will 
    fail.

    /projects/Stuff/ProjectA/
                            buildstreamfolder/project.conf
                            workspaceA/source/etc
                            workspaceB/source/etc

    could be moved to:

    /projects/better/ProjectA/
                            buildstreamfolder/project.conf
                            workspaceA/source/etc
                            workspaceB/source/etc


What is the recovery mode for a moved workspace that can't find its
project?  Can it be re-registered?

Will mentioned that a user could 'fudge' the path given in a workspace
fragment which would allow for manual intervention in the case of
re-registering. Ultimately that's a bit messy so going forward with a
utility (a new flag for bst workspace?) for the re-association of
workspaces to projects is probably the right thing to do. How 'smart'
this command would be, especially when trying to generate a list of
potential parent projects, is something that would need to be more
fleshed out if desired.

 

    Debug
    ~~~~~

    Note: while the scope of this task is workspaces, they are clearly
    an important 
    factor in debugging especially when debugging something you are
    using as a 
    workspace, so we need to catch any potental API changes to
    workspaces. This 
    therefore requires some detail of how to debug to ensure that no API
    changes 
    are needed.

    We will add an example of using GDBfrom a binary package to show how
    a generic
    debugger could be used from within the bst shell. However it may
    well be that 
    BuildStream could build the debugger, and it could be added to the
    shell as a 
    runtime dependency. There has been relevant talk surrounding the
    perceived 
    specifics of how to utilise 'SwankyDebug' inside a bst produced
    shell here [10].

    This will then form the reference implementation that we can then
    talk around 
    to implement other debuggers that may have other requirements.

    This example will include debugging a library from an app (getting
    break 
    points/messages) from the library when running `gdb app` from the
    app's shell.

    Mounting workspaces vs cached build trees was brought up in [7] for
    this 
    perceived usecase and as such it needs to be clarified whether the
    commitment 
    to implement this feature with the current workspaces is 'worth it',
    or if the 
    work to use cached build trees is ultimately the best long-term
    path. One other 
    point of consideration is do we want to differentiate a flag for
    runtime 
    dependency debugging vs build debugging (which is currently -b), or
    would 
    runtime debugging be the default functionality of 'shell'?

    The following must be implemented without affecting the project
    files (we dont 
    want debug options to be accidentally added to the project's vcs)
    but parts will 
    affect the cache key:

    - Get list of runtime/build time dependencies
    - Reference against open workspaces for the project
    - Mount them into the respective buildroots, provide feedback if
    there are 
      clashes
    - Environment is set up for target element, user has to be aware of
    this if 
      they want to tinker with dependencies within shell.
    - Mount in debugger (local element type if not to be built by
    BuildStream, 
      either as dependency or debug flag)
    - Debug, this can include making source changes & recompiling within
    shell
    - Drop out of shell & carry on with bst workflow as appropriate to the 
      debugging outcome.


I feel that half of the proposal is non-contentious and the other half
is around Debug.  From the above I am not clear there are going to be
any conflicting workspace CLI changes needed, it seems not.  Can we
separate out Debug on that basis?

I agree here that the majority of cli changes needed (or as we currently
see as needed) here are for bst shell. However there is definitely
overlaps & the point of using cached build trees (& especially if they
are to become by default what is currently checked out into a workspace)
are places that we feel benefit from being in the overarching design
proposal. If deemed necessary I'd take no issue with the core design &
implementation of extending debugging with bst shell to be separated out.

Cheers,

Tom

Cheers,

Sander
 

    Outstanding issues
    ==================

    Issues to close
    ~~~~~~~~~~~~~~~

    The description has been implemented so this can be closed: 
    https://gitlab.com/BuildStream/buildstream/issues/191

    Creating workspaces should be one ticket setting out a concise API,
    so we will 
    close the related ones and create a new one with a clear description
    and link 
    to this list:

    https://gitlab.com/BuildStream/buildstream/issues/229 
    https://gitlab.com/BuildStream/buildstream/issues/362 

    Issues to Update
    ~~~~~~~~~~~~~~~~

    If we accept the fragment idea, then we will add the details and
    submit a MR 
    to: https://gitlab.com/BuildStream/buildstream/issues/222

    Issues to create
    ~~~~~~~~~~~~~~~~

     - A new 'epic' issue which will contain the outcomes of this
    proposal as well 
       as a link to the original mailing list post, to act as a related
    ticket for
       all new work and to serve as a point of reference/origin.
    - A new ticket for relative workspaces that covers "out of project"
    workspaces.
    - An issue to cover all the changes to the command line CLI for the
    workspace 
      open command.
    - An issue to track the proposed PoC for debugging an app & related
    libraries 
      from within bst shell with GDB as a generic tool.

    Many Thanks
    Tom & Will

    # Links
    [1]
    https://mail.gnome.org/archives/buildstream-list/2018-August/msg00033.html
    [2]
    https://mail.gnome.org/archives/buildstream-list/2018-August/msg00023.html
    [3] https://gitlab.com/BuildStream/buildstream/issues/229
    [4] https://gitlab.com/BuildStream/buildstream/issues/222
    [5] https://gitlab.com/BuildStream/buildstream/issues/362
    [6] https://gitlab.com/BuildStream/buildstream/issues/311
    [7] https://gitlab.com/BuildStream/buildstream/issues/413
    [8] https://gitlab.com/BuildStream/buildstream/issues/539
    [9] https://gitlab.com/BuildStream/buildstream/merge_requests/510
    [10]
    https://mail.gnome.org/archives/buildstream-list/2018-August/msg00034.html


    _______________________________________________
    BuildStream-list mailing list
    BuildStream-list gnome org <mailto:BuildStream-list gnome org>
    https://mail.gnome.org/mailman/listinfo/buildstream-list

-- 

Cheers,

Sander


_______________________________________________
BuildStream-list mailing list
BuildStream-list gnome org
https://mail.gnome.org/mailman/listinfo/buildstream-list


-- 
https://www.codethink.co.uk/privacy.html


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