diff mbox series

doc/bitbake-user-manual-fetching: update the Git fetcher tag description

Message ID 20250717-new-tag-git-fetcher-v1-1-77e46929103b@bootlin.com
State Accepted, archived
Commit 85b31a55d114a1430868233d56573b470fef8908
Headers show
Series doc/bitbake-user-manual-fetching: update the Git fetcher tag description | expand

Commit Message

Antonin Godard July 17, 2025, 12:23 p.m. UTC
After commit d591d7633fe8 ("fetch/git: Rework tag parameter handling"),
update the description of the tag= parameter for the Git fetcher.

Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
---
 doc/bitbake-user-manual/bitbake-user-manual-fetching.rst | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)


---
base-commit: 09f29a4968841ee5070f70277ba8c253bb14f017
change-id: 20250717-new-tag-git-fetcher-6287232fc0b1

Best regards,
--  
Antonin Godard <antonin.godard@bootlin.com>

Comments

Quentin Schulz July 17, 2025, 12:42 p.m. UTC | #1
Hi Antonin,

On 7/17/25 2:23 PM, Antonin Godard via lists.openembedded.org wrote:
> After commit d591d7633fe8 ("fetch/git: Rework tag parameter handling"),
> update the description of the tag= parameter for the Git fetcher.
> 
> Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
> ---
>   doc/bitbake-user-manual/bitbake-user-manual-fetching.rst | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/doc/bitbake-user-manual/bitbake-user-manual-fetching.rst b/doc/bitbake-user-manual/bitbake-user-manual-fetching.rst
> index a2c2432db1..b3fc3ce72c 100644
> --- a/doc/bitbake-user-manual/bitbake-user-manual-fetching.rst
> +++ b/doc/bitbake-user-manual/bitbake-user-manual-fetching.rst
> @@ -439,10 +439,11 @@ This fetcher supports the following parameters:
>   -  *"rev":* The revision to use for the checkout. The default is
>      "master".
>   

This seems... odd? I thought rev was supposed to be a full hash (though 
that probably is me remembering SRCREV instead?). There seems to be 
tests using it in bitbake/lib/bb/tests/fetch.py so I guess it is valid? 
Not a clue what this is supposed to do though or if master really is the 
default value?

> --  *"tag":* Specifies a tag to use for the checkout. To correctly
> -   resolve tags, BitBake must access the network. For that reason, tags
> -   are often not used. As far as Git is concerned, the "tag" parameter
> -   behaves effectively the same as the "rev" parameter.
> +-  *"tag":* Specifies a tag to match against the supplied revision (``rev=``).
> +   This does not replace the ``rev`` parameter: the ``tag`` parameter should
> +   only be specified if you want to verify that the tag commit SHA matches the
> +   supplied ``rev`` SHA. The ``tag`` parameter is not mandatory to use, but

s/not mandatory to use/optional/ ?

> +   strongly recommended.
>   

ok so I was clearly not aware of that change and got surprised by a few 
of the latest patches on the ML adding that parameter.

1) I've read on the ML that it is possible there may be a difference in 
behavior (or even an issue?) with annotated vs lightweight tags? Would 
be worth checking and specifying this limitation if it does exist.
2) Since this is a change in behavior, we should probably document that 
in the appropriate migration manual?

>   -  *"subpath":* Limits the checkout to a specific subpath of the tree.
>      By default, the whole tree is checked out.
> 

Finally, this is not alphabetically ordered so we could take this as an 
opportunity to fix that oversight :) mmmm but reading more than the git 
context shows that most aren't ordered in that fetcher so... maybe for 
another patch :)

Cheers,
Quentin
Antonin Godard July 18, 2025, 9:07 a.m. UTC | #2
On Thu Jul 17, 2025 at 2:42 PM CEST, Quentin Schulz wrote:
> Hi Antonin,
>
> On 7/17/25 2:23 PM, Antonin Godard via lists.openembedded.org wrote:
>> After commit d591d7633fe8 ("fetch/git: Rework tag parameter handling"),
>> update the description of the tag= parameter for the Git fetcher.
>> 
>> Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
>> ---
>>   doc/bitbake-user-manual/bitbake-user-manual-fetching.rst | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>> 
>> diff --git a/doc/bitbake-user-manual/bitbake-user-manual-fetching.rst b/doc/bitbake-user-manual/bitbake-user-manual-fetching.rst
>> index a2c2432db1..b3fc3ce72c 100644
>> --- a/doc/bitbake-user-manual/bitbake-user-manual-fetching.rst
>> +++ b/doc/bitbake-user-manual/bitbake-user-manual-fetching.rst
>> @@ -439,10 +439,11 @@ This fetcher supports the following parameters:
>>   -  *"rev":* The revision to use for the checkout. The default is
>>      "master".
>>   
>
> This seems... odd? I thought rev was supposed to be a full hash (though 
> that probably is me remembering SRCREV instead?). There seems to be 
> tests using it in bitbake/lib/bb/tests/fetch.py so I guess it is valid? 
> Not a clue what this is supposed to do though or if master really is the 
> default value?

I think it acts just like SRCREV as I understand it.
"master" seems odd indeed, maybe it was mistaken with 'branch'?

https://git.openembedded.org/bitbake/tree/lib/bb/fetch2/repo.py#n39

>> --  *"tag":* Specifies a tag to use for the checkout. To correctly
>> -   resolve tags, BitBake must access the network. For that reason, tags
>> -   are often not used. As far as Git is concerned, the "tag" parameter
>> -   behaves effectively the same as the "rev" parameter.
>> +-  *"tag":* Specifies a tag to match against the supplied revision (``rev=``).
>> +   This does not replace the ``rev`` parameter: the ``tag`` parameter should
>> +   only be specified if you want to verify that the tag commit SHA matches the
>> +   supplied ``rev`` SHA. The ``tag`` parameter is not mandatory to use, but
>
> s/not mandatory to use/optional/ ?

Yes, thanks

>> +   strongly recommended.
>>   
>
> ok so I was clearly not aware of that change and got surprised by a few 
> of the latest patches on the ML adding that parameter.
>
> 1) I've read on the ML that it is possible there may be a difference in 
> behavior (or even an issue?) with annotated vs lightweight tags? Would 
> be worth checking and specifying this limitation if it does exist.

From 136c06e251de ("fetch2/git: Handle srcrevs for annotated tags in tag
check"), this doesn't seem to be an issue: if rev is set to the revision of the
annotated tag, it is resolved to the underlying commit sha and compared again.
If rev is set to the revision of the tag's underlying commit sha, it passes the
first check.

NB: In any case, the tag in 'tag=' is always resolved to the underlying commit
sha.

> 2) Since this is a change in behavior, we should probably document that 
> in the appropriate migration manual?

Yep, I'm on it. I came with this patch in the process of writing the release /
migration notes

>>   -  *"subpath":* Limits the checkout to a specific subpath of the tree.
>>      By default, the whole tree is checked out.
>> 
>
> Finally, this is not alphabetically ordered so we could take this as an 
> opportunity to fix that oversight :) mmmm but reading more than the git 
> context shows that most aren't ordered in that fetcher so... maybe for 
> another patch :)

True, I'd say send a patch if it bothers you too much hehe :)

Antonin
diff mbox series

Patch

diff --git a/doc/bitbake-user-manual/bitbake-user-manual-fetching.rst b/doc/bitbake-user-manual/bitbake-user-manual-fetching.rst
index a2c2432db1..b3fc3ce72c 100644
--- a/doc/bitbake-user-manual/bitbake-user-manual-fetching.rst
+++ b/doc/bitbake-user-manual/bitbake-user-manual-fetching.rst
@@ -439,10 +439,11 @@  This fetcher supports the following parameters:
 -  *"rev":* The revision to use for the checkout. The default is
    "master".
 
--  *"tag":* Specifies a tag to use for the checkout. To correctly
-   resolve tags, BitBake must access the network. For that reason, tags
-   are often not used. As far as Git is concerned, the "tag" parameter
-   behaves effectively the same as the "rev" parameter.
+-  *"tag":* Specifies a tag to match against the supplied revision (``rev=``).
+   This does not replace the ``rev`` parameter: the ``tag`` parameter should
+   only be specified if you want to verify that the tag commit SHA matches the
+   supplied ``rev`` SHA. The ``tag`` parameter is not mandatory to use, but
+   strongly recommended.
 
 -  *"subpath":* Limits the checkout to a specific subpath of the tree.
    By default, the whole tree is checked out.