-
Couldn't load subscription status.
- Fork 36
docs: Add 'Execution of BuildStream Elements' page #2057
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
docs: Add 'Execution of BuildStream Elements' page #2057
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments. This document needs a rewrite as it has a lot of factual inaccuracies and wrong terminology.
| Sources are fetched | ||
| ------------------- | ||
|
|
||
| The hidden first step is actually validating the ``yaml``. This includes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This "hidden" step needs to be "unhidden" in the documentation.
|
|
||
| The hidden first step is actually validating the ``yaml``. This includes | ||
| resolving includes, options, appends, which are denoted by ``(@)``, | ||
| ``(?)`` and ``(>)`` respectively. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense to link to the rest of the documentation, having a document like this feels very weird.
| ---------------------------- | ||
|
|
||
| Now that all dependencies and sources are staged in a temporary | ||
| filesystem, this filesystem is started up by BuildBox in a sandbox. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"this filesystem is started" <- what does that mean?
| config files and other “configure” related actions into this section. | ||
| These should be the commands that can only be run once (for example a | ||
| folder can only be moved once), this is due to `BuildStream | ||
| workspaces <https://docs.buildstream.build/2.4/developing/workspaces.html>`__. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to link to the appropriate document, not to the 2.4 version of the documentation.
But if you want to talk about workspaces, you should split configure commands from the rest of the commands above.
| pulling in fresh sources. Builds using workspaces only run configure | ||
| commands once, and any subsequent builds using the same workspace will skip | ||
| the configure commands step, therefore steps of the build that aren't | ||
| repeatable (without re-staging sources) should be added to configure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
repeatable
It seems this word is using in a different meaning here than usual.
| should mainly be comprised of moving the built artifacts from the | ||
| ``${build-root}`` to the ``${install-root}``. | ||
|
|
||
| There is no need for Install commands to clean up any of the sources |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say it's the reverse. It's not that there is no need, it's more that you shouldn't. The sources can be stored as a build tree, and they need to stay around if you want to inspect the buildtree after the fact.
| Directories can be created under the install location, for example | ||
| ``%{install-root}/example/``, and these will be maintained when another | ||
| element depends on this one, for example this will become | ||
| ``./example/``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You meant /example
28300de to
cf8d4e4
Compare
cf8d4e4 to
0efc86e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this document contains useful information, however, the scope doesn't seem completely clear to me.
Looking at the current BuildStream documentation, I think we're indeed missing a good overview of the different stages for a given element during bst build, so it would be nice to have that.
However, this document includes a lot of details that I don't consider useful as part of an overview. We should link to other sections of the documentation for that (and expand other sections for details that aren't documented anywhere else right now).
I.e., I would define the scope of this page to be an overview of the build process of an element, as I think that's what we're missing most. Or did you have a different scope or target audience in mind? I'm open to other possibilities but I think we should be reasonably clear about what does and what does not belong on this new page.
WIP!