Re: [docs] [PATCH 6/6] ref-manual/variables: improve the UNPACKDIR documentation

["Antonin Godard" <antonin.godard@bootlin.com>]

Hi Quentin,

On Thu Feb 20, 2025 at 5:41 PM CET, Quentin Schulz wrote:
> Hi Antonin,
>
> On 2/18/25 10:12 AM, Antonin Godard via lists.yoctoproject.org wrote:
>> It was clear why UNPACKDIR was introduced at first, and what is the
>> recommended way of setting S and UNPACKDIR for a clean separation of the
>> source code and WORKDIR. Add documentation for this in the reference
>> manual.
>> 
>> Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
>> ---
>>   documentation/ref-manual/variables.rst | 39 +++++++++++++++++++++++++++++++++-
>>   1 file changed, 38 insertions(+), 1 deletion(-)
>> 
>> diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
>> index adbef69d8f39d33be87c5db6688a807156540410..d970fc21f69fe35830a9d6b5a28da7cb257c0709 100644
>> --- a/documentation/ref-manual/variables.rst
>> +++ b/documentation/ref-manual/variables.rst
>> @@ -10035,9 +10035,46 @@ system and gives an overview of their function and contents.
>>   
>>      :term:`UNPACKDIR`
>>         This variable, used by the :ref:`ref-classes-base` class,
>> -      specifies where fetches sources should be unpacked by the
>> +      specifies where fetched sources should be unpacked by the
>>         :ref:`ref-tasks-unpack` task.
>>   
>> +      Although the default value of this directory is
>> +      ``${WORKDIR}/sources-unpack``, the recommended way of setting
>> +      :term:`UNPACKDIR` and :term:`S` is::
>> +
>> +         S = "${WORKDIR}/sources"
>> +         UNPACKDIR = "${S}"
>> +
>
> Mmmm I think this is an issue, because we then show a non-recommended 
> configuration in the SVG in the previous patches where UNPACKDIR != S.

I'd rather show the default definitions in the figures, because there's no
guarantee that a recipe will define UNPACKDIR and S that way.

It also feels weird to me that a recommended approach is not the default, but it
is probably related to backward compatiblity, that would be my guess.

>> +      This allows the source code to be safely unpacked and patched in a
>> +      separate directory.
>> +
>
> I think I know where you want to go with this, but the first time I read 
> this I didn't understand it. UNPACKDIR = S in the example above, why 
> does this say the source code is unpacked and patched (implying two 
> steps/stages) in a separate directory then?

I wanted to say that the code is unpacked and patched in the same directory, but
separate from the WORKDIR. I should probably rephrase or remove this sentence to
avoid adding confusion.

> I think this is because we used to put everything from the fetcher into
> WORKDIR and there were unexpected leftovers between builds and UNPACKDIR 
> now is a separate directory for the fetcher to put everything it does 
> in, instead of WORKDIR?

Yes, I think that's it.

> I'm not sure we need to document how messed up it was in the past rather 
> just what this does now?

So I guess we can go with the sentence removal. However I think we'd probably
want to keep the recommended assignment.

>> +      .. note::
>> +
>> +         In some cases, the :term:`UNPACKDIR` directory only holds the sources
>> +         temporarily. When the first directory after :term:`WORKDIR` in the
>> +         :term:`S` variable is matched in :term:`UNPACKDIR`, the directory
>> +         in :term:`UNPACKDIR` is moved to the :term:`WORKDIR` directory.
>> +
>> +         For example, let's represent the :term:`UNPACKDIR` directory
>> +         for the ``foobar`` recipe after extraction as follows::
>> +
>> +            ${WORKDIR}/sources-unpack/foobar-1.0.0/...
>> +
>> +         And the variable :term:`S` for this recipe defined as::
>> +
>> +            S = "${WORKDIR}/${BP}"
>> +
>> +         Where :term:`BP` expands to ``foobar-1.0.0``.
>> +
>> +         Since the first directory of :term:`S` after :term:`WORKDIR`
>> +         (``foobar-1.0.0``) matches the directory ``foobar-1.0.0`` under
>> +         ``sources-unpack``, the latter is moved to the :term:`WORKDIR`
>> +         directory. The final location of the source code will then be
>> +         ``${WORKDIR}/foobar-1.0.0``.
>> +
>> +         This behavior was added for compatibility for recipes that do not set
>> +         the :term:`UNPACKDIR` and :term:`S` variable in the recommended way.
>> +
>
> I've read this 5 times now, it may be too late for my brain today but I 
> am not sure I got it :)
>
> Let me try to reword it just to make sure I understood/got it right?
>
> If UNPACKDIR differs from S, but the name of the directory (dirA) where 
> sources are unpacked in UNPACKDIR matches the basename of the directory 
> used for S, then the content of UNPACKDIR/dirA is moved into S?

That's it. And it is a pain to describe textually. I'd take any recommendations
for improving it.

> If that's right, what really happens? Is UNPACKDIR variable modified to 
> point to S once the move is done? When is this happening?

No, UNPACKDIR isn't modified. This move is repeated at the end of the do_unpack
task, each time. See:
https://git.openembedded.org/openembedded-core/tree/meta/classes-global/base.bbclass#n158

I may consider removing that note entirely depending on how useful it is to
document a backwards-compatibility behavior.

Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com