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 |
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
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 --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.
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>