abderrahim commented on code in PR #2057:
URL: https://github.com/apache/buildstream/pull/2057#discussion_r2337342483
##########
doc/source/using_execution_of_elements.rst:
##########
@@ -0,0 +1,112 @@
+Execution of BuildStream Elements
+=================================
+
+This page provides a guide on how BuildStream elements are converted
+into executable build steps, including how element types may affect
Review Comment:
I feel this goes against the terminology used by buildstream. You don't
*execute* an element, you probably *build* it. Also talking about "converting a
buildstream element into executable steps" feels wrong as well.
##########
doc/source/using_execution_of_elements.rst:
##########
@@ -0,0 +1,112 @@
+Execution of BuildStream Elements
+=================================
+
+This page provides a guide on how BuildStream elements are converted
+into executable build steps, including how element types may affect
+this.
+
+When a BuildStream element is being built, it can roughly be broken down
+into these individual stages:
+
+- Sources are fetched
+- The sandbox is prepared
+- Configure and Run commands
+- Install Commands and Caching Artifacts
+
+Sources are fetched
+-------------------
+
+The hidden first step is actually validating the ``yaml``. This includes
+resolving includes, options, appends, which are denoted by ``(@)``,
+``(?)`` and ``(>)`` respectively.
+
+The first step once the ``yaml`` has been validated is that BuildStream
+will fetch the sources. This is dependent on the source ``kind`` as to
+how these are fetched. After the sources are fetched they may need to
+checked. For example a ``kind: tar`` would need to check the ``sha256``
+matches, and a ``git_repo`` source would need to switch to the specified
+commit sha.
+
+The Sandbox is prepared
+-----------------------
+
+The Sandbox is prepared as a temporary filesystem, where build
+dependencies (``build-depends:``) and general dependencies (``depends``)
+are staged, along with their own runtime dependencies. This happens in
+an abstract state, which can quickly spot repeated files and overlaps.
+
+In general when a dependency is being staged, the produced artifact is
+added from the root (``/``). This can sometimes be changed using the
Review Comment:
If we're talking about elements in general, they can be staged anywhere the
plugin wants (and `location` isn't a generic configuration that all elements
support). If we're talking about build elements, then "In general" isn't needed.
##########
doc/source/using_execution_of_elements.rst:
##########
@@ -0,0 +1,112 @@
+Execution of BuildStream Elements
+=================================
+
+This page provides a guide on how BuildStream elements are converted
+into executable build steps, including how element types may affect
+this.
+
+When a BuildStream element is being built, it can roughly be broken down
+into these individual stages:
+
+- Sources are fetched
+- The sandbox is prepared
+- Configure and Run commands
+- Install Commands and Caching Artifacts
+
+Sources are fetched
+-------------------
+
+The hidden first step is actually validating the ``yaml``. This includes
Review Comment:
This "hidden" step needs to be "unhidden" in the documentation.
##########
doc/source/using_execution_of_elements.rst:
##########
@@ -0,0 +1,112 @@
+Execution of BuildStream Elements
+=================================
+
+This page provides a guide on how BuildStream elements are converted
+into executable build steps, including how element types may affect
+this.
+
+When a BuildStream element is being built, it can roughly be broken down
+into these individual stages:
+
+- Sources are fetched
+- The sandbox is prepared
+- Configure and Run commands
+- Install Commands and Caching Artifacts
Review Comment:
This is wrong, install commands run together with other commands. Caching
artifacts is indeed a separate step.
##########
doc/source/using_execution_of_elements.rst:
##########
@@ -0,0 +1,112 @@
+Execution of BuildStream Elements
+=================================
+
+This page provides a guide on how BuildStream elements are converted
+into executable build steps, including how element types may affect
+this.
+
+When a BuildStream element is being built, it can roughly be broken down
+into these individual stages:
+
+- Sources are fetched
+- The sandbox is prepared
+- Configure and Run commands
+- Install Commands and Caching Artifacts
+
+Sources are fetched
+-------------------
+
+The hidden first step is actually validating the ``yaml``. This includes
+resolving includes, options, appends, which are denoted by ``(@)``,
+``(?)`` and ``(>)`` respectively.
+
+The first step once the ``yaml`` has been validated is that BuildStream
+will fetch the sources. This is dependent on the source ``kind`` as to
+how these are fetched. After the sources are fetched they may need to
+checked. For example a ``kind: tar`` would need to check the ``sha256``
+matches, and a ``git_repo`` source would need to switch to the specified
+commit sha.
+
+The Sandbox is prepared
+-----------------------
+
+The Sandbox is prepared as a temporary filesystem, where build
+dependencies (``build-depends:``) and general dependencies (``depends``)
+are staged, along with their own runtime dependencies. This happens in
+an abstract state, which can quickly spot repeated files and overlaps.
+
+In general when a dependency is being staged, the produced artifact is
+added from the root (``/``). This can sometimes be changed using the
+``location:`` attribute. In most cases dependencies are marked by
+BuildStream as immutable.
+
+After the dependencies are staged BuildStream stages the sources in the
+``build-root`` location. The actual location of this differs slightly
+depending on the project file structure, which is why it is common to
+see elements use the variable ``%{build-root}`` which resolves to the
+correct location.
+
+Configure and Build commands
+----------------------------
+
+Now that all dependencies and sources are staged in a temporary
+filesystem, this filesystem is started up by BuildBox in a sandbox.
+
+The first commands to be run are configure commands and it is
+recommended to include things like moving the sources about, generating
+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>`__.
+
+!!! tip “BuildStream Workspaces and Configure Commands”
+
+::
+
+ When a
[workspace](https://docs.buildstream.build/2.4/developing/workspaces.html)
+ is opened, it stages all the sources for the indicated element locally, then
+ when doing a build of that element it uses these local sources instead of
+ 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
+ commands.
+
+After Configure commands are run, then Build commands are next. Build
+commands are intended to contain the actual build process, for example
+if the build uses ``make`` then this stage should run the
+``make target`` command.
+
+Install Commands and Caching Artifacts
+--------------------------------------
+
+Install commands are the final commands that are run before BuildStream
+collects the artifacts and closes the build sandbox. Install commands
+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
Review Comment:
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.
##########
doc/source/using_execution_of_elements.rst:
##########
@@ -0,0 +1,112 @@
+Execution of BuildStream Elements
+=================================
+
+This page provides a guide on how BuildStream elements are converted
+into executable build steps, including how element types may affect
+this.
+
+When a BuildStream element is being built, it can roughly be broken down
+into these individual stages:
+
+- Sources are fetched
+- The sandbox is prepared
+- Configure and Run commands
+- Install Commands and Caching Artifacts
+
+Sources are fetched
+-------------------
+
+The hidden first step is actually validating the ``yaml``. This includes
+resolving includes, options, appends, which are denoted by ``(@)``,
+``(?)`` and ``(>)`` respectively.
+
+The first step once the ``yaml`` has been validated is that BuildStream
+will fetch the sources. This is dependent on the source ``kind`` as to
+how these are fetched. After the sources are fetched they may need to
+checked. For example a ``kind: tar`` would need to check the ``sha256``
+matches, and a ``git_repo`` source would need to switch to the specified
+commit sha.
+
+The Sandbox is prepared
+-----------------------
+
+The Sandbox is prepared as a temporary filesystem, where build
+dependencies (``build-depends:``) and general dependencies (``depends``)
+are staged, along with their own runtime dependencies. This happens in
+an abstract state, which can quickly spot repeated files and overlaps.
+
+In general when a dependency is being staged, the produced artifact is
+added from the root (``/``). This can sometimes be changed using the
+``location:`` attribute. In most cases dependencies are marked by
+BuildStream as immutable.
+
+After the dependencies are staged BuildStream stages the sources in the
+``build-root`` location. The actual location of this differs slightly
+depending on the project file structure, which is why it is common to
+see elements use the variable ``%{build-root}`` which resolves to the
+correct location.
+
+Configure and Build commands
+----------------------------
+
+Now that all dependencies and sources are staged in a temporary
+filesystem, this filesystem is started up by BuildBox in a sandbox.
+
+The first commands to be run are configure commands and it is
+recommended to include things like moving the sources about, generating
+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>`__.
Review Comment:
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.
##########
doc/source/using_execution_of_elements.rst:
##########
@@ -0,0 +1,112 @@
+Execution of BuildStream Elements
+=================================
+
+This page provides a guide on how BuildStream elements are converted
+into executable build steps, including how element types may affect
+this.
+
+When a BuildStream element is being built, it can roughly be broken down
+into these individual stages:
+
+- Sources are fetched
+- The sandbox is prepared
+- Configure and Run commands
+- Install Commands and Caching Artifacts
+
+Sources are fetched
+-------------------
+
+The hidden first step is actually validating the ``yaml``. This includes
+resolving includes, options, appends, which are denoted by ``(@)``,
+``(?)`` and ``(>)`` respectively.
+
+The first step once the ``yaml`` has been validated is that BuildStream
+will fetch the sources. This is dependent on the source ``kind`` as to
+how these are fetched. After the sources are fetched they may need to
+checked. For example a ``kind: tar`` would need to check the ``sha256``
+matches, and a ``git_repo`` source would need to switch to the specified
+commit sha.
+
+The Sandbox is prepared
+-----------------------
+
+The Sandbox is prepared as a temporary filesystem, where build
+dependencies (``build-depends:``) and general dependencies (``depends``)
+are staged, along with their own runtime dependencies. This happens in
+an abstract state, which can quickly spot repeated files and overlaps.
+
+In general when a dependency is being staged, the produced artifact is
+added from the root (``/``). This can sometimes be changed using the
+``location:`` attribute. In most cases dependencies are marked by
+BuildStream as immutable.
+
+After the dependencies are staged BuildStream stages the sources in the
+``build-root`` location. The actual location of this differs slightly
+depending on the project file structure, which is why it is common to
+see elements use the variable ``%{build-root}`` which resolves to the
+correct location.
+
+Configure and Build commands
+----------------------------
+
+Now that all dependencies and sources are staged in a temporary
+filesystem, this filesystem is started up by BuildBox in a sandbox.
+
+The first commands to be run are configure commands and it is
+recommended to include things like moving the sources about, generating
+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>`__.
+
+!!! tip “BuildStream Workspaces and Configure Commands”
+
+::
+
+ When a
[workspace](https://docs.buildstream.build/2.4/developing/workspaces.html)
+ is opened, it stages all the sources for the indicated element locally, then
+ when doing a build of that element it uses these local sources instead of
+ 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
+ commands.
+
+After Configure commands are run, then Build commands are next. Build
+commands are intended to contain the actual build process, for example
+if the build uses ``make`` then this stage should run the
+``make target`` command.
+
+Install Commands and Caching Artifacts
+--------------------------------------
+
+Install commands are the final commands that are run before BuildStream
+collects the artifacts and closes the build sandbox. Install commands
+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
+that aren’t going to moved, as the BuildStream handles this when the
+sandbox is closed.
+
+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/``.
Review Comment:
You meant `/example`
##########
doc/source/using_execution_of_elements.rst:
##########
@@ -0,0 +1,112 @@
+Execution of BuildStream Elements
+=================================
+
+This page provides a guide on how BuildStream elements are converted
+into executable build steps, including how element types may affect
+this.
+
+When a BuildStream element is being built, it can roughly be broken down
+into these individual stages:
+
+- Sources are fetched
+- The sandbox is prepared
+- Configure and Run commands
+- Install Commands and Caching Artifacts
+
+Sources are fetched
+-------------------
+
+The hidden first step is actually validating the ``yaml``. This includes
+resolving includes, options, appends, which are denoted by ``(@)``,
+``(?)`` and ``(>)`` respectively.
Review Comment:
I think it makes sense to link to the rest of the documentation, having a
document like this feels very weird.
##########
doc/source/using_execution_of_elements.rst:
##########
@@ -0,0 +1,112 @@
+Execution of BuildStream Elements
+=================================
+
+This page provides a guide on how BuildStream elements are converted
+into executable build steps, including how element types may affect
+this.
+
+When a BuildStream element is being built, it can roughly be broken down
+into these individual stages:
+
+- Sources are fetched
+- The sandbox is prepared
+- Configure and Run commands
+- Install Commands and Caching Artifacts
+
+Sources are fetched
+-------------------
+
+The hidden first step is actually validating the ``yaml``. This includes
+resolving includes, options, appends, which are denoted by ``(@)``,
+``(?)`` and ``(>)`` respectively.
+
+The first step once the ``yaml`` has been validated is that BuildStream
+will fetch the sources. This is dependent on the source ``kind`` as to
+how these are fetched. After the sources are fetched they may need to
+checked. For example a ``kind: tar`` would need to check the ``sha256``
+matches, and a ``git_repo`` source would need to switch to the specified
+commit sha.
+
+The Sandbox is prepared
+-----------------------
+
+The Sandbox is prepared as a temporary filesystem, where build
+dependencies (``build-depends:``) and general dependencies (``depends``)
Review Comment:
build dependencies is enough, "general dependencies" as this document calls
them are "both" build and runtime dependencies in buildstream terminology.
##########
doc/source/using_execution_of_elements.rst:
##########
@@ -0,0 +1,112 @@
+Execution of BuildStream Elements
+=================================
+
+This page provides a guide on how BuildStream elements are converted
+into executable build steps, including how element types may affect
+this.
+
+When a BuildStream element is being built, it can roughly be broken down
+into these individual stages:
+
+- Sources are fetched
+- The sandbox is prepared
+- Configure and Run commands
+- Install Commands and Caching Artifacts
+
+Sources are fetched
+-------------------
+
+The hidden first step is actually validating the ``yaml``. This includes
+resolving includes, options, appends, which are denoted by ``(@)``,
+``(?)`` and ``(>)`` respectively.
+
+The first step once the ``yaml`` has been validated is that BuildStream
+will fetch the sources. This is dependent on the source ``kind`` as to
+how these are fetched. After the sources are fetched they may need to
+checked. For example a ``kind: tar`` would need to check the ``sha256``
+matches, and a ``git_repo`` source would need to switch to the specified
+commit sha.
+
+The Sandbox is prepared
+-----------------------
+
+The Sandbox is prepared as a temporary filesystem, where build
+dependencies (``build-depends:``) and general dependencies (``depends``)
+are staged, along with their own runtime dependencies. This happens in
+an abstract state, which can quickly spot repeated files and overlaps.
+
+In general when a dependency is being staged, the produced artifact is
+added from the root (``/``). This can sometimes be changed using the
+``location:`` attribute. In most cases dependencies are marked by
+BuildStream as immutable.
+
+After the dependencies are staged BuildStream stages the sources in the
+``build-root`` location. The actual location of this differs slightly
+depending on the project file structure, which is why it is common to
+see elements use the variable ``%{build-root}`` which resolves to the
+correct location.
+
+Configure and Build commands
+----------------------------
+
+Now that all dependencies and sources are staged in a temporary
+filesystem, this filesystem is started up by BuildBox in a sandbox.
Review Comment:
"this filesystem is started" <- what does that mean?
##########
doc/source/using_execution_of_elements.rst:
##########
@@ -0,0 +1,112 @@
+Execution of BuildStream Elements
+=================================
+
+This page provides a guide on how BuildStream elements are converted
+into executable build steps, including how element types may affect
+this.
+
+When a BuildStream element is being built, it can roughly be broken down
+into these individual stages:
+
+- Sources are fetched
+- The sandbox is prepared
+- Configure and Run commands
+- Install Commands and Caching Artifacts
+
+Sources are fetched
+-------------------
+
+The hidden first step is actually validating the ``yaml``. This includes
+resolving includes, options, appends, which are denoted by ``(@)``,
+``(?)`` and ``(>)`` respectively.
+
+The first step once the ``yaml`` has been validated is that BuildStream
+will fetch the sources. This is dependent on the source ``kind`` as to
+how these are fetched. After the sources are fetched they may need to
+checked. For example a ``kind: tar`` would need to check the ``sha256``
+matches, and a ``git_repo`` source would need to switch to the specified
+commit sha.
+
+The Sandbox is prepared
+-----------------------
+
+The Sandbox is prepared as a temporary filesystem, where build
+dependencies (``build-depends:``) and general dependencies (``depends``)
+are staged, along with their own runtime dependencies. This happens in
+an abstract state, which can quickly spot repeated files and overlaps.
+
+In general when a dependency is being staged, the produced artifact is
+added from the root (``/``). This can sometimes be changed using the
+``location:`` attribute. In most cases dependencies are marked by
+BuildStream as immutable.
+
+After the dependencies are staged BuildStream stages the sources in the
+``build-root`` location. The actual location of this differs slightly
+depending on the project file structure, which is why it is common to
+see elements use the variable ``%{build-root}`` which resolves to the
+correct location.
+
+Configure and Build commands
+----------------------------
+
+Now that all dependencies and sources are staged in a temporary
+filesystem, this filesystem is started up by BuildBox in a sandbox.
+
+The first commands to be run are configure commands and it is
+recommended to include things like moving the sources about, generating
+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>`__.
+
+!!! tip “BuildStream Workspaces and Configure Commands”
+
+::
+
+ When a
[workspace](https://docs.buildstream.build/2.4/developing/workspaces.html)
+ is opened, it stages all the sources for the indicated element locally, then
+ when doing a build of that element it uses these local sources instead of
+ 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
Review Comment:
> repeatable
It seems this word is using in a different meaning here than usual.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@buildstream.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
