[PATCH 0/2] ref-manual: variables: document IMGDEPLOYDIR

[PATCH 0/2] ref-manual: variables: document IMGDEPLOYDIR

[Daniel Wagenknecht]

Hello!

I recently stumbled upon direct references to DEPLOY_DIR_IMAGE when
upgrading an old Yocto Krogoth project. Those caused the symlinking of
the most recent image to fail. I needed quite some time to find the root
cause, since IMGDEPLOYDIR was not documented yet.

I'm unsure if a note to `documentation/ref-manual/migration-2.2.rst`
should also be added, since that is when the changes where introduced.

Best Regards
Daniel Wagenknecht


Daniel Wagenknecht (2):
  ref-manual: variables: update examples refering to DEPLOY_DIR_IMAGE
  ref-manual: variables: document IMGDEPLOYDIR

 documentation/ref-manual/variables.rst | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

-- 
2.25.1

[PATCH 1/2] ref-manual: variables: update examples refering to DEPLOY_DIR_IMAGE

[Daniel Wagenknecht]

DEPLOY_DIR_IMAGE has been replaced by IMGDEPLOYDIR for most use cases
since OE-Core rev 6d969bacc718e21a5246d4da9bf9639dcae29b02. Update
examples given in the documentation to use IMGDEPLOYDIR and match their
implementation.

Signed-off-by: Daniel Wagenknecht <dwagenknecht@emlix.com>
---
 documentation/ref-manual/variables.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
index 8ebc17f24..2e859c718 100644
--- a/documentation/ref-manual/variables.rst
+++ b/documentation/ref-manual/variables.rst
@@ -2946,7 +2946,7 @@ system and gives an overview of their function and contents.
       an override for the associated type. Here is an example::
 
          IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} \
-             --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 \
+             --faketime --output=${IMGDEPLOYDIR}/${IMAGE_NAME}.rootfs.jffs2 \
              ${EXTRA_IMAGECMD}"
 
       You typically do not need to set this variable unless you are adding
@@ -3095,10 +3095,10 @@ system and gives an overview of their function and contents.
       The :ref:`image <ref-classes-image>` class defines the manifest
       file as follows::
 
-         IMAGE_MANIFEST ="${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.manifest"
+         IMAGE_MANIFEST ="${IMGDEPLOYDIR}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.manifest"
 
       The location is
-      derived using the :term:`DEPLOY_DIR_IMAGE`
+      derived using the :term:`IMGDEPLOYDIR`
       and :term:`IMAGE_NAME` variables. You can find
       information on how the image is created in the ":ref:`overview-manual/concepts:image generation`"
       section in the Yocto Project Overview and Concepts Manual.
-- 
2.25.1

[PATCH 2/2] ref-manual: variables: document IMGDEPLOYDIR

[Daniel Wagenknecht]

DEPLOY_DIR_IMAGE has been replaced by IMGDEPLOYDIR for most use cases
since OE-Core rev 6d969bacc718e21a5246d4da9bf9639dcae29b02. Document
IMGDEPLOYDIR and add a note to DEPLOY_DIR_IMAGE about it.

Signed-off-by: Daniel Wagenknecht <dwagenknecht@emlix.com>
---
 documentation/ref-manual/variables.rst | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
index 2e859c718..8b41a1fe1 100644
--- a/documentation/ref-manual/variables.rst
+++ b/documentation/ref-manual/variables.rst
@@ -1673,6 +1673,11 @@ system and gives an overview of their function and contents.
       resides within the :term:`Build Directory` as
       ``${DEPLOY_DIR}/images/${MACHINE}/``.
 
+      It is usually not used directly in recipes, but filled with the contents
+      of :term:`DEPLOYDIR` by the :ref:`deploy <ref-classes-deploy>` class or
+      with the contents of :term:`IMGDEPLOYDIR` by the :ref:`image
+      <ref-classes-image>` class.
+
       For more information on the structure of the Build Directory, see
       ":ref:`ref-manual/structure:the build directory - \`\`build/\`\``" section.
       For more detail on the contents of the ``deploy`` directory, see the
@@ -3324,6 +3329,18 @@ system and gives an overview of their function and contents.
       desired, and this suffix would then be used consistently across
       the build artifacts.
 
+   :term:`IMGDEPLOYDIR`
+      When inheriting the :ref:`image <ref-classes-image>` directly or
+      through the :ref:`core-image <ref-classes-core-image>` class, the
+      ``IMGDEPLOYDIR`` points to a temporary work area for deployed files
+      that is set in the ``image`` class as follows::
+
+         IMGDEPLOYDIR = "${WORKDIR}/deploy-${PN}-image-complete"
+
+      Recipes inheriting the ``image`` class should copy files to be
+      deployed into ``IMGDEPLOYDIR``, and the class will take care of
+      copying them into :term:`DEPLOY_DIR_IMAGE` afterwards.
+
    :term:`INC_PR`
       Helps define the recipe revision for recipes that share a common
       ``include`` file. You can think of this variable as part of the
-- 
2.25.1

Re: [docs] [PATCH 2/2] ref-manual: variables: document IMGDEPLOYDIR

[Nicolas Dechesne]

[-- Attachment #1: Type: text/plain, Size: 2832 bytes --]

On Fri, Jun 11, 2021 at 8:26 AM Daniel Wagenknecht <dwagenknecht@emlix.com>
wrote:

> DEPLOY_DIR_IMAGE has been replaced by IMGDEPLOYDIR for most use cases
> since OE-Core rev 6d969bacc718e21a5246d4da9bf9639dcae29b02. Document
> IMGDEPLOYDIR and add a note to DEPLOY_DIR_IMAGE about it.
>

It's a really good idea to document that better, I've had to explain that a
few times, and it's not very intuitive for someone not familiar with how
sstate works.
Thanks!


>
> Signed-off-by: Daniel Wagenknecht <dwagenknecht@emlix.com>
> ---
>  documentation/ref-manual/variables.rst | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/documentation/ref-manual/variables.rst
> b/documentation/ref-manual/variables.rst
> index 2e859c718..8b41a1fe1 100644
> --- a/documentation/ref-manual/variables.rst
> +++ b/documentation/ref-manual/variables.rst
> @@ -1673,6 +1673,11 @@ system and gives an overview of their function and
> contents.
>        resides within the :term:`Build Directory` as
>        ``${DEPLOY_DIR}/images/${MACHINE}/``.
>
> +      It is usually not used directly in recipes, but filled with the
> contents
> +      of :term:`DEPLOYDIR` by the :ref:`deploy <ref-classes-deploy>`
> class or
> +      with the contents of :term:`IMGDEPLOYDIR` by the :ref:`image
> +      <ref-classes-image>` class.
>

The message should be even stronger. It *must* not be used in recipes when
deploying files. It can be used only when a recipe need to 'read' a file
already deployed by a dependency of course.



> +
>        For more information on the structure of the Build Directory, see
>        ":ref:`ref-manual/structure:the build directory - \`\`build/\`\``"
> section.
>        For more detail on the contents of the ``deploy`` directory, see the
> @@ -3324,6 +3329,18 @@ system and gives an overview of their function and
> contents.
>        desired, and this suffix would then be used consistently across
>        the build artifacts.
>
> +   :term:`IMGDEPLOYDIR`
> +      When inheriting the :ref:`image <ref-classes-image>` directly or
> +      through the :ref:`core-image <ref-classes-core-image>` class, the
> +      ``IMGDEPLOYDIR`` points to a temporary work area for deployed files
> +      that is set in the ``image`` class as follows::
> +
> +         IMGDEPLOYDIR = "${WORKDIR}/deploy-${PN}-image-complete"
> +
> +      Recipes inheriting the ``image`` class should copy files to be
> +      deployed into ``IMGDEPLOYDIR``, and the class will take care of
> +      copying them into :term:`DEPLOY_DIR_IMAGE` afterwards.
>
+
>     :term:`INC_PR`
>        Helps define the recipe revision for recipes that share a common
>        ``include`` file. You can think of this variable as part of the
> --
> 2.25.1
>
>
> 
>
>

[-- Attachment #2: Type: text/html, Size: 3910 bytes --]

Re: [docs] [PATCH 0/2] ref-manual: variables: document IMGDEPLOYDIR

[Nicolas Dechesne]

[-- Attachment #1: Type: text/plain, Size: 984 bytes --]

On Fri, Jun 11, 2021 at 8:26 AM Daniel Wagenknecht <dwagenknecht@emlix.com>
wrote:

> Hello!
>
> I recently stumbled upon direct references to DEPLOY_DIR_IMAGE when
> upgrading an old Yocto Krogoth project. Those caused the symlinking of
> the most recent image to fail. I needed quite some time to find the root
> cause, since IMGDEPLOYDIR was not documented yet.
>
> I'm unsure if a note to `documentation/ref-manual/migration-2.2.rst`
> should also be added, since that is when the changes where introduced.
>

I would suggest adding it. It's an important change very visible and
impacting for users.


>
> Best Regards
> Daniel Wagenknecht
>
>
> Daniel Wagenknecht (2):
>   ref-manual: variables: update examples refering to DEPLOY_DIR_IMAGE
>   ref-manual: variables: document IMGDEPLOYDIR
>
>  documentation/ref-manual/variables.rst | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> --
> 2.25.1
>
>
> 
>
>

[-- Attachment #2: Type: text/html, Size: 1527 bytes --]

[PATCH 3/3] ref-manual: migration-2.2: add note about IMGDEPLOYDIR

[Daniel Wagenknecht]

Signed-off-by: Daniel Wagenknecht <dwagenknecht@emlix.com>
---
 documentation/ref-manual/migration-2.2.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/documentation/ref-manual/migration-2.2.rst b/documentation/ref-manual/migration-2.2.rst
index a60ce8d52..5fab00faf 100644
--- a/documentation/ref-manual/migration-2.2.rst
+++ b/documentation/ref-manual/migration-2.2.rst
@@ -263,6 +263,17 @@ If you have recipes or
 classes that use ``KERNEL_IMAGE_BASE_NAME`` directly, you might need to
 update the references to ensure they continue to work.
 
+.. _migration-2.2-imgdeploydir-replaces-deploy-dir-image-for-most-use-cases:
+
+``IMGDEPLOYDIR`` Replaces ``DEPLOY_DIR_IMAGE`` for Most Use Cases
+-----------------------------------------------------------------
+
+The :term:`IMGDEPLOYDIR` variable was introduced to allow sstate caching of
+image creation results. Image recipes defining custom :term:`IMAGE_CMD` or
+doing postprocessing on the generated images need to be adapted to use
+``IMGDEPLOYDIR`` instead of :term:`DEPLOY_DIR_IMAGE`. ``IMAGE_MANIFEST``
+creation and symlinking of the most recent image file will fail otherwise.
+
 .. _migration-2.2-bitbake-changes:
 
 BitBake Changes
-- 
2.25.1

Re: [docs] [PATCH 2/2] ref-manual: variables: document IMGDEPLOYDIR

[Michael Opdenacker]

Daniel, Nicolas,

Thank you for the patch and review.

On 6/11/21 8:46 AM, Nicolas Dechesne wrote:
>
>
>     +      It is usually not used directly in recipes, but filled with
>     the contents
>     +      of :term:`DEPLOYDIR` by the :ref:`deploy
>     <ref-classes-deploy>` class or
>     +      with the contents of :term:`IMGDEPLOYDIR` by the :ref:`image
>     +      <ref-classes-image>` class.
>
>
> The message should be even stronger. It *must* not be used in recipes
> when deploying files. It can be used only when a recipe need to 'read'
> a file already deployed by a dependency of course.


I can try to take care of this to save you time, Daniel... Here's what
I've included in my local branch meant to merge your changes into
"master-next" of yocto-docs.

      It must not be used directly in recipes when deploying files. Instead,
      it's only useful when a recipe needs to "read" a file already deployed
      by a dependency. So, it should be filled with the contents of
      :term:`DEPLOYDIR` by the :ref:`deploy <ref-classes-deploy>` class or
      with the contents of :term:`IMGDEPLOYDIR` by the :ref:`image
      <ref-classes-image>` class.

I'll be happy to rephrase if you have anything better.

Cheers,

Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH 2/2] ref-manual: variables: document IMGDEPLOYDIR

[Michael Opdenacker]

On 6/11/21 8:26 AM, Daniel Wagenknecht wrote:
>  
> +   :term:`IMGDEPLOYDIR`
> +      When inheriting the :ref:`image <ref-classes-image>` directly or

I guess you meant "the  :ref:`image <ref-classes-image>` *class* directly".

I fixed that in my branch too.

Thanks again,

Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH 2/2] ref-manual: variables: document IMGDEPLOYDIR

[Daniel Wagenknecht]

[-- Attachment #1: Type: text/plain, Size: 729 bytes --]

Hello Michael,

On Fri, 2021-06-11 at 11:13 +0200, Michael Opdenacker wrote:
> I can try to take care of this to save you time, Daniel... Here's what
> I've included in my local branch meant to merge your changes into
> "master-next" of yocto-docs.
> 
>       It must not be used directly in recipes when deploying files. Instead,
>       it's only useful when a recipe needs to "read" a file already deployed
>       by a dependency. So, it should be filled with the contents of
>       :term:`DEPLOYDIR` by the :ref:`deploy <ref-classes-deploy>` class or
>       with the contents of :term:`IMGDEPLOYDIR` by the :ref:`image
>       <ref-classes-image>` class.

Yes, that looks good, thank you!

Cheers
Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 849 bytes --]

Re: [docs] [PATCH 2/2] ref-manual: variables: document IMGDEPLOYDIR

[Michael Opdenacker]

Hi Daniel,

On 6/11/21 12:14 PM, Daniel Wagenknecht wrote:
> Hello Michael,
>
> On Fri, 2021-06-11 at 11:13 +0200, Michael Opdenacker wrote:
>> I can try to take care of this to save you time, Daniel... Here's what
>> I've included in my local branch meant to merge your changes into
>> "master-next" of yocto-docs.
>>
>>       It must not be used directly in recipes when deploying files. Instead,
>>       it's only useful when a recipe needs to "read" a file already deployed
>>       by a dependency. So, it should be filled with the contents of
>>       :term:`DEPLOYDIR` by the :ref:`deploy <ref-classes-deploy>` class or
>>       with the contents of :term:`IMGDEPLOYDIR` by the :ref:`image
>>       <ref-classes-image>` class.
> Yes, that looks good, thank you!


Thanks for confirming. I merged your changes into "master-next" in
yocto-docs.

Cheers,

Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [docs] [PATCH 1/2] ref-manual: variables: update examples refering to DEPLOY_DIR_IMAGE

[Quentin Schulz]

Hi Daniel,

On Fri, Jun 11, 2021 at 08:26:12AM +0200, Daniel Wagenknecht wrote:
> DEPLOY_DIR_IMAGE has been replaced by IMGDEPLOYDIR for most use cases
> since OE-Core rev 6d969bacc718e21a5246d4da9bf9639dcae29b02. Update
> examples given in the documentation to use IMGDEPLOYDIR and match their
> implementation.
> 
> Signed-off-by: Daniel Wagenknecht <dwagenknecht@emlix.com>
> ---
>  documentation/ref-manual/variables.rst | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
> index 8ebc17f24..2e859c718 100644
> --- a/documentation/ref-manual/variables.rst
> +++ b/documentation/ref-manual/variables.rst
> @@ -2946,7 +2946,7 @@ system and gives an overview of their function and contents.
>        an override for the associated type. Here is an example::
>  
>           IMAGE_CMD_jffs2 = "mkfs.jffs2 --root=${IMAGE_ROOTFS} \
> -             --faketime --output=${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.rootfs.jffs2 \
> +             --faketime --output=${IMGDEPLOYDIR}/${IMAGE_NAME}.rootfs.jffs2 \
>               ${EXTRA_IMAGECMD}"
>  

It'd be awesome if you could do a follow-up patch here since this isn't the actual
content of this variable anymore.
C.f.: https://cgit.openembedded.org/openembedded-core/tree/meta/classes/image_types.bbclass#n59

>        You typically do not need to set this variable unless you are adding
> @@ -3095,10 +3095,10 @@ system and gives an overview of their function and contents.
>        The :ref:`image <ref-classes-image>` class defines the manifest

IMAGE_MANIFEST isn't in this class anymore, it'd be great again if you
could send a follow-up patch for this.
c.f. http://cgit.openembedded.org/openembedded-core/tree/meta/classes/rootfs-postcommands.bbclass#n31

Thanks for the patch!

Reviewed-by: Quentin Schulz <foss@0leil.net>

[...]
>        The location is
> -      derived using the :term:`DEPLOY_DIR_IMAGE`
> +      derived using the :term:`IMGDEPLOYDIR`

On a side note: you should have added the term in the glossary (from patch 2)
first, because technically this commit does not build.

Cheers,
Quentin