Only download the build tree if absolutely necessary

added Important label

changed the description

mentioned in issue #21 (closed)

added Blocker label

changed milestone to %BuildStream_v1.4

marked this issue as related to #21 (closed)

mentioned in merge request !570 (closed)

mentioned in issue #566

removed Important label

assigned to @tpollard

I'm currently taking a look at this, the work it could lead to for #311 is of interest for the wider workspace DX changes

As a test I'm building core-deps/gtk+-3.bst from gnome-build-meta, pulling from a pre-populated remote cas server. A fully pulled build creates a local artifact dir of 32G, whilst by explicitly emitting the 'buildtree' directory from the pulled in artifact object & populating a new cache it seems to create an artifact dir of 7G, whilst claiming to successfully build the element

I'm currently looking at adding a helper function '_fetch_buildroot' to cascache.py. Through the use of _get_subdir & _fetch_directory it should be possible to add the buildroot digest object to the local cache

Opening workspaces (although to actually be implemented in #311, through the use of a helper function) & pushing to a new remote cache server (as to not push incomplete artifacts) are the two agreed situations in which bst should automatically know to fetch a missing workspace. The user requested for a buildtree from bst pull & bst build could be implemented with a flag or option to the respective commands, or by a default 'pull/include buildtree' at a user or project configuration level. It also needs to be able support already pulled 'partial' artifacts that require the buildroot object adding to them, to avoid doing a full refetch of the whole artifact tree.

I'm currently looking at adding a helper function '_fetch_buildroot' to cascache.py.

cascache.py does not currently know the directory structure within an artifact and I'd like to keep it that way for proper separation of concerns. Do you see an issue keeping _fetch_buildroot in a different file and only implementing the generic subdir functions in cascache.py?

Opening workspaces (although to actually be implemented in #311, through the use of a helper function) & pushing to a new remote cache server (as to not push incomplete artifacts) are the two agreed situations in which bst should automatically know to fetch a missing workspace. The user requested for a buildtree from bst pull & bst build could be implemented with a flag or option to the respective commands, or by a default 'pull/include buildtree' at a user or project configuration level. It also needs to be able support already pulled 'partial' artifacts that require the buildroot object adding to them, to avoid doing a full refetch of the whole artifact tree.

Agree to an explicit --pull-buildtrees like option to pull and build
- Note that bst pull --pull-buildtrees needs to still pull the build tree of an already downloaded artifact which does not yet have it's build tree
Agree that we can have a user configuration option to opt into pulling all build trees by default, although I have my doubts that it would be used much in practice
More importantly, we need to have an interactive experience in the commands where a build tree is finally needed, or optionally needed, like the workspace opening for example
- In these cases we need to optionally run the scheduler if the user has decided at this moment to download the build tree they never needed until now
- This is the mode of operation I expect to be most frequently used, the user doesnt want this data until they actually want this data

by a default 'pull/include buildtree' at a user or project configuration level.

This user/project distinction is not arbitrary by any means; the project should not decide this, only the user should decide what they want to be downloading locally.

This user/project distinction is not arbitrary by any means; the project should not decide this, only the user should decide what they want to be downloading locally.

I've been adding support from a user config level as the arfitact server (+ if to push to them) can be defined there, so it made sense to add to the existing structure parsed by ArtifactCacheSpec()

I'm hoping to push some WIP soon that can at-least demonstrate so of the functionality for this issue

I've been adding support from a user config level as the arfitact server (+ if to push to them) can be defined there, so it made sense to add to the existing structure parsed by ArtifactCacheSpec()

Two things:

Optionality of push is #566, it's important to not make push support optional off the bat, please don't jump the gun on #566, this issue comes first and #566 is strategically postponed until we make things work well enough without it.
I understand that ArtifactCacheSpec() itself captures a lot of config and currently works for project config and user config, but implementing this optionality at the project level is wrong.

The fact that the project provides a recommendation of which artifact server to use is an exception to the rule of project data and user configuration separation.

It is convenient for the project to provide the url to it's artifact server; it is inappropriate for the project to inform the user of whether they should be pulling build trees or not.

Optionality of push is #566, it's important to not make push support optional off the bat, please don't jump the gun on #566, this issue comes first and #566 is strategically postponed until we make things work well enough without it.

Sorry misunderstanding there, I meant the optionality of pulling buildtrees from a remote set in project.conf, with push being the unchanged current implementation

mentioned in merge request !786 (merged)

I've pushed a WIP !786 (merged) with the work I have so far on this issue

Sorry misunderstanding there, I meant the optionality of pulling buildtrees from a remote set in project.conf, with push being the unchanged current implementation

Right, and I'm trying to clarify that a project.conf, I.e. an upstream project / group; does not get to decide the default of whether the user wants to pull the build trees or not.

The default is the user only ever wants the least as possible, at the last minute, when they want it; if the user configuration buildstream.conf allows the user to decide to pull everything all the time (which might be desirable in say, a CI setting), that is fine, but the project does not decide.

I've now had chance to rebase !786 (merged) which has brought along with it some changes to cascache.py for remote execution support. This brings along with it a couple of interesting functions which may prove useful in implementing this issue. These include the pull_tree & push/send directory functions along with verification helpers, which could if deemed appropriate be used for the handling of the workspace directory artifact/blobs

added Doing label and removed To Do label

Hi @tristanvb I've been making some progress & refactoring (taking into account your point on project not setting this config) on !786 (merged). I'm at the point that I need to handle pushing to remotes when the local artifact does not include the buildtree object. As it stands the push queue will skip the element artifact if it does not exist at all locally. Would you want this behaviour to extend to when the artifact exists without the buildtree, or would you expect a pull (be it via pull queue or directly) to happen automatically to bring in the missing dir (all be it dependant on an alternative remote existing to fetch it from). Cheers

Hi @juergbi I've been making progress of !786 (merged) & I've taken into account your comment on not explicitly defining the artifact dir structure within cascache.py. At this point it would be helpful if you could spare a chance to go over some of the existing logic, so I can do any refactoring before moving to tests. Cheers

@tpollard

Would you want this behaviour to extend to when the artifact exists without the buildtree, or would you expect a pull (be it via pull queue or directly) to happen automatically to bring in the missing dir (all be it dependant on an alternative remote existing to fetch it from).

Skip the push if the build tree does not exist locally is easier as a first step.

Once we have this working and have a good test scenario with one of the existing sample projects, we may change this approach, and consider whether we want to accept artifact caches which don't contain build trees at all, as a part of #566.

added To Do label and removed Doing label

added Doing label and removed To Do label

marked the checklist item Ensure the CAS client/server layer has support for downloading partial artifacts as completed

marked the checklist item Configurable PullQueue, allow specifying whether you want to download the main artifact, the build tree, or both as completed

marked the checklist item If BuildStream is going to push artifacts to different artifact server than the one it downloads from, it must download the build trees in order to upload the new artifacts; incomplete artifact uploads are not allowed. as completed

mentioned in merge request !869 (merged)

When !869 (merged) is merged (happening now), there will be a new entry point where a build tree can be optionally downloaded.

!786 (merged) has now been rebased and modified to handle @jonathanmaw landing !869 (merged). As pointed out this has added another situation in which an optional partial pull job for the buildtree could be attempted. As with the push logic, if the buildtree is not available, then it will be skipped/not processed. !873 (closed) will bring new api to cascache to handle subdir extraction from an artifact, when that lands it should be used inplace of the full extract when staging.

mentioned in issue #539 (closed)

marked this issue as related to #740 (closed)

mentioned in issue #311

I've commented in #311 with regards to !786 (merged) landing, and how it leaves the final point of this issue ticket open. As with trying to push a partial element, I think the path of least resistance and the least disruptive from a UX view is to just skip any action (warning the user where appropriate) which is expecting to consume a missing buildtree initially. An issue/MR linked to 494 to close the final point should be acted upon to create a global 'utility/helper' method that can be used on demand via the scheduler to attempt to download the buildtree (or be it future required partial subdir objects) for this usecase and other none/existing scenarios. Once this is available it can be consumed throughout the codebase as a single path, without limiting work elsewhere.

mentioned in issue #774 (closed)

marked this issue as related to #774 (closed)

I've added #774 (closed) to track the final requirement of this issue, if this is not as expected @tristanvb then I'm happy to discuss/rejig.

changed the description

mentioned in merge request !978 (merged)

!978 (merged) has been merged and shown to be used on bst push for 'buildtree-less' artifacts, the same method can/should be used where a missing buildtree may be required given the current context/config, i.e workspace buildtrees. Is this enough to tick the final point of this ticket and close it?

assigned to @tristanvb

!1050 (merged) Has also been merged, which gives the option for bst build shell attempt to download missing buildtrees via the pullqueue

marked this issue as related to #566

Only download the build tree if absolutely necessary

Background

Task description

Acceptance Criteria

Child items ...

Activity