diff mbox series

[2/4] bitbake-layers: layerindex-fetch: Fix branch detection method

Message ID 20251212190806.32476-3-osama.abdelkader@gmail.com
State Accepted, archived
Commit af9dd012e7f4d16dccd1d6118be5da94ede68f85
Headers show
Series bitbake-layers: layerindex-fetch: respect --branch for already-configured layers | expand

Commit Message

Osama Abdelkader Dec. 12, 2025, 7:08 p.m. UTC
Replace fragile parsing of 'git branch' output with the more reliable
'git rev-parse --abbrev-ref HEAD' command to get the current branch name.
This avoids parsing issues and is the recommended way to get the current branch name.

Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
---
 lib/bblayers/layerindex.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Paul Barker Dec. 15, 2025, 11:54 a.m. UTC | #1
On Fri, 2025-12-12 at 20:08 +0100, Osama Abdelkader wrote:
> Replace fragile parsing of 'git branch' output with the more reliable
> 'git rev-parse --abbrev-ref HEAD' command to get the current branch name.
> This avoids parsing issues and is the recommended way to get the current branch name.

Do you have a source for this recommendation? I thought the preferred
command was `git branch --show-current` as that just prints the branch
name with no extra formatting.

Best regards,
Osama Abdelkader Dec. 15, 2025, 10:34 p.m. UTC | #2
On Mon, Dec 15, 2025 at 11:54:20AM +0000, Paul Barker wrote:
> On Fri, 2025-12-12 at 20:08 +0100, Osama Abdelkader wrote:
> > Replace fragile parsing of 'git branch' output with the more reliable
> > 'git rev-parse --abbrev-ref HEAD' command to get the current branch name.
> > This avoids parsing issues and is the recommended way to get the current branch name.
> 
> Do you have a source for this recommendation? I thought the preferred
> command was `git branch --show-current` as that just prints the branch
> name with no extra formatting.

Git commands are divided into two groups:

Porcelain - High level commands intended to be used at the command line
Plumbing - Low level commands intended for use in scripts

git rev-parse is a "plumbing" command, meaning its output format is guaranteed to be stable across different versions of Git.
It reliably outputs only the raw branch name to standard output.

git branch is a "porcelain" command, intended for human users. While git branch --show-current currently behaves helpfully,
the formatting of porcelain commands is theoretically subject to change in future Git releases, which could break your build
scripts unexpectedly.

https://git-scm.com/book/en/v2/Git-Internals-Plumbing-and-Porcelain
https://git-scm.com/docs/git-rev-parse

> 
> Best regards,
> 
> -- 
> Paul Barker
> 

Best regards,
Osama
Paul Barker Dec. 18, 2025, 7:39 p.m. UTC | #3
On Mon, 2025-12-15 at 23:34 +0100, Osama Abdelkader wrote:
> On Mon, Dec 15, 2025 at 11:54:20AM +0000, Paul Barker wrote:
> > On Fri, 2025-12-12 at 20:08 +0100, Osama Abdelkader wrote:
> > > Replace fragile parsing of 'git branch' output with the more reliable
> > > 'git rev-parse --abbrev-ref HEAD' command to get the current branch name.
> > > This avoids parsing issues and is the recommended way to get the current branch name.
> > 
> > Do you have a source for this recommendation? I thought the preferred
> > command was `git branch --show-current` as that just prints the branch
> > name with no extra formatting.
> 
> Git commands are divided into two groups:
> 
> Porcelain - High level commands intended to be used at the command line
> Plumbing - Low level commands intended for use in scripts
> 
> git rev-parse is a "plumbing" command, meaning its output format is guaranteed to be stable across different versions of Git.
> It reliably outputs only the raw branch name to standard output.
> 
> git branch is a "porcelain" command, intended for human users. While git branch --show-current currently behaves helpfully,
> the formatting of porcelain commands is theoretically subject to change in future Git releases, which could break your build
> scripts unexpectedly.
> 
> https://git-scm.com/book/en/v2/Git-Internals-Plumbing-and-Porcelain
> https://git-scm.com/docs/git-rev-parse

Hi Osama,

Thanks for the additional context! We discussed this on the patch review
call today.

`git rev-parse --abbrev-ref` is definitely an improvement over calling
`git branch`, but we have still seen some issues with this command in
other contexts on the autobuilder. The best command to use seems to be
`git name-rev --name-only` [1].

[1]: https://git.yoctoproject.org/yocto-autobuilder-helper/commit/?id=1e80c19f62169c536e44f99fdcdf9f1e84c65d99

Best regards,
Osama Abdelkader Dec. 19, 2025, 4:05 p.m. UTC | #4
On Thu, Dec 18, 2025 at 07:39:28PM +0000, Paul Barker wrote:
> On Mon, 2025-12-15 at 23:34 +0100, Osama Abdelkader wrote:
> > On Mon, Dec 15, 2025 at 11:54:20AM +0000, Paul Barker wrote:
> > > On Fri, 2025-12-12 at 20:08 +0100, Osama Abdelkader wrote:
> > > > Replace fragile parsing of 'git branch' output with the more reliable
> > > > 'git rev-parse --abbrev-ref HEAD' command to get the current branch name.
> > > > This avoids parsing issues and is the recommended way to get the current branch name.
> > > 
> > > Do you have a source for this recommendation? I thought the preferred
> > > command was `git branch --show-current` as that just prints the branch
> > > name with no extra formatting.
> > 
> > Git commands are divided into two groups:
> > 
> > Porcelain - High level commands intended to be used at the command line
> > Plumbing - Low level commands intended for use in scripts
> > 
> > git rev-parse is a "plumbing" command, meaning its output format is guaranteed to be stable across different versions of Git.
> > It reliably outputs only the raw branch name to standard output.
> > 
> > git branch is a "porcelain" command, intended for human users. While git branch --show-current currently behaves helpfully,
> > the formatting of porcelain commands is theoretically subject to change in future Git releases, which could break your build
> > scripts unexpectedly.
> > 
> > https://git-scm.com/book/en/v2/Git-Internals-Plumbing-and-Porcelain
> > https://git-scm.com/docs/git-rev-parse
> 
> Hi Osama,
> 
> Thanks for the additional context! We discussed this on the patch review
> call today.
> 
> `git rev-parse --abbrev-ref` is definitely an improvement over calling
> `git branch`, but we have still seen some issues with this command in
> other contexts on the autobuilder. The best command to use seems to be
> `git name-rev --name-only` [1].
> 
> [1]: https://git.yoctoproject.org/yocto-autobuilder-helper/commit/?id=1e80c19f62169c536e44f99fdcdf9f1e84c65d99
> 
> Best regards,
> 
> -- 
> Paul Barker
> 

Hi Paul,

Thanks for the review, okay will change to that.

Best regards,
Osama
diff mbox series

Patch

diff --git a/lib/bblayers/layerindex.py b/lib/bblayers/layerindex.py
index 0ffc95822b..1c6511889d 100644
--- a/lib/bblayers/layerindex.py
+++ b/lib/bblayers/layerindex.py
@@ -55,13 +55,15 @@  class LayerIndexPlugin(ActionPlugin):
             switching branches if necessary and possible.
             """
             base_cmd = ['git', '--git-dir=%s/.git' % repodir, '--work-tree=%s' % repodir]
-            cmd = base_cmd + ['branch']
+            # Get current branch name
+            cmd = base_cmd + ['rev-parse', '--abbrev-ref', 'HEAD']
             completed_proc = subprocess.run(cmd, text=True, capture_output=True)
             if completed_proc.returncode:
                 logger.error("Unable to validate repo %s (%s)" % (repodir, completed_proc.stderr))
                 return None, None, None
             else:
-                if branch != completed_proc.stdout[2:-1]:
+                current_branch = completed_proc.stdout.strip()
+                if branch != current_branch:
                     cmd = base_cmd + ['status', '--short']
                     completed_proc = subprocess.run(cmd, text=True, capture_output=True)
                     if completed_proc.stdout.count('\n') != 0: