diff mbox series

[v2,1/6] patchtest: fix failure when oe-core repo is in detached HEAD

Message ID 20251201163100.143476-1-naftaly.ralamboarivony@smile.fr
State Accepted, archived
Commit 98ecba96fb3ade50a686d390ec6139de421be9a5
Headers show
Series [v2,1/6] patchtest: fix failure when oe-core repo is in detached HEAD | expand

Commit Message

naftaly.ralamboarivony@smile.fr Dec. 1, 2025, 4:30 p.m. UTC
From: Naftaly RALAMBOARIVONY <naftaly.ralamboarivony@smile.fr>

Patchtest fails when oe-core git repo is in a "detached HEAD" state:

Error log:

> File "/usr/lib/python3/dist-packages/git/repo/base.py", line 881, in
active_branch return self.head.reference ^^^^^^^^^^^^^^^^^^^

> File "/usr/lib/python3/dist-packages/git/refs/symbolic.py", line 311, in
_get_reference raise TypeError("%s is a detached symbolic reference as it
points to %r" % (self, sha)) TypeError: HEAD is a detached symbolic reference
as it points to '3dd31d3b29730fa1130645d76bb71914ac036335' None

In this case, no current branch is available for the clean operation.

To fix this, updates the checkout logic:
- if a current branch is available, use it,
- otherwise, fall back to the commit pointed to by HEAD.

This ensures that the script works correctly even when HEAD is detached.

Signed-off-by: Naftaly RALAMBOARIVONY <naftaly.ralamboarivony@smile.fr>
---
 meta/lib/patchtest/repo.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Trevor Gamblin Dec. 1, 2025, 6:33 p.m. UTC | #1
On 2025-12-01 11:30, naftaly.ralamboarivony via lists.openembedded.org 
wrote:
> From: Naftaly RALAMBOARIVONY <naftaly.ralamboarivony@smile.fr>
>
> Patchtest fails when oe-core git repo is in a "detached HEAD" state:
>
> Error log:
>
>> File "/usr/lib/python3/dist-packages/git/repo/base.py", line 881, in
> active_branch return self.head.reference ^^^^^^^^^^^^^^^^^^^
>
>> File "/usr/lib/python3/dist-packages/git/refs/symbolic.py", line 311, in
> _get_reference raise TypeError("%s is a detached symbolic reference as it
> points to %r" % (self, sha)) TypeError: HEAD is a detached symbolic reference
> as it points to '3dd31d3b29730fa1130645d76bb71914ac036335' None
>
> In this case, no current branch is available for the clean operation.
>
> To fix this, updates the checkout logic:
> - if a current branch is available, use it,
> - otherwise, fall back to the commit pointed to by HEAD.
>
> This ensures that the script works correctly even when HEAD is detached.

Hi,

This is a really nice improvement to the patchtest code, thank you for 
submitting it. I have two general comments:

1. There are a few places where the new printouts are confusing, 
particularly in test_head_detached, e.g.:

|Test '%s' failed with git in detach HEAD repository state changed after 
detached HEAD test

I think this could be simpler, like

|Test '%s' failed with git in detached HEAD mode: state changed after test

2. The bigger issue, IMO is the way the test output has changed. Here's 
a result from my local selftest with your patches applied:

|(venv) tgamblin@megalith ~/workspace/yocto/openembedded-core (master)$ 
./meta/lib/patchtest/selftest/selftest
|XFAIL: TestMbox.test_author_valid (file: TestMbox.test_author_valid.1.fail)
|XPASS: TestMbox.test_author_valid (file: TestMbox.test_author_valid.1.pass)
|...
|
|XFAIL: TestMbox.test_author_valid (file: TestMbox.test_author_valid.1.fail)
|Test 'TestMbox.test_author_valid' passed, with git in detach HEAD 
repository state remained identical after detached HEAD test.
|============================================================================
|Testsuite summary for patchtest
|============================================================================
|# TOTAL: 37
|# XPASS: 17
|# XFAIL: 18
|# XSKIP: 2
|# PASS: 0
|# FAIL: 0
|# SKIP: 0
|# ERROR: 0
|============================================================================

My thought is that the selftests shouldn't have the else: part of 
test_head_detached(), since it's not tracking that pass in the totals.

If we're going to include it, then maybe the code could be adjusted so 
that the output looks like:

|(venv) tgamblin@megalith ~/workspace/yocto/openembedded-core (master)$ 
./meta/lib/patchtest/selftest/selftest
|XFAIL: TestMbox.test_author_valid (file: TestMbox.test_author_valid.1.fail)
|XPASS: TestMbox.test_author_valid (file: TestMbox.test_author_valid.1.pass)
|...
|
|XFAIL: TestMbox.test_author_valid (file: TestMbox.test_author_valid.1.fail)
|XPASS: selftest.test_head_detached

And the xpass part of counts could be incremented. What do you think?

>
> Signed-off-by: Naftaly RALAMBOARIVONY <naftaly.ralamboarivony@smile.fr>
> ---
>   meta/lib/patchtest/repo.py | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/meta/lib/patchtest/repo.py b/meta/lib/patchtest/repo.py
> index 2cdd6736e4..6a7d7d2d3b 100644
> --- a/meta/lib/patchtest/repo.py
> +++ b/meta/lib/patchtest/repo.py
> @@ -21,7 +21,12 @@ class PatchTestRepo(object):
>           self.repodir = repodir
>           self.repo = git.Repo.init(repodir)
>           self.patch = mbox.PatchSeries(patch)
> -        self.current_branch = self.repo.active_branch.name
> +
> +        if self.repo.head.is_detached:
> +            self.current_commit = self.repo.head.commit.hexsha
> +            self.current_branch = None
> +        else:
> +            self.current_branch = self.repo.active_branch.name
>   
>           # targeted branch defined on the patch may be invalid, so make sure there
>           # is a corresponding remote branch
> @@ -80,6 +85,6 @@ class PatchTestRepo(object):
>               self._patchmerged = True
>   
>       def clean(self):
> -        self.repo.git.execute(['git', 'checkout', self.current_branch])
> +        self.repo.git.execute(['git', 'checkout', self.current_branch if self.current_branch else self.current_commit])
>           self.repo.git.execute(['git', 'branch', '-D', self._workingbranch])
>           self._patchmerged = False
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#227129): https://lists.openembedded.org/g/openembedded-core/message/227129
> Mute This Topic: https://lists.openembedded.org/mt/116559758/7611679
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [tgamblin@baylibre.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
diff mbox series

Patch

diff --git a/meta/lib/patchtest/repo.py b/meta/lib/patchtest/repo.py
index 2cdd6736e4..6a7d7d2d3b 100644
--- a/meta/lib/patchtest/repo.py
+++ b/meta/lib/patchtest/repo.py
@@ -21,7 +21,12 @@  class PatchTestRepo(object):
         self.repodir = repodir
         self.repo = git.Repo.init(repodir)
         self.patch = mbox.PatchSeries(patch)
-        self.current_branch = self.repo.active_branch.name
+
+        if self.repo.head.is_detached:
+            self.current_commit = self.repo.head.commit.hexsha
+            self.current_branch = None
+        else:
+            self.current_branch = self.repo.active_branch.name
 
         # targeted branch defined on the patch may be invalid, so make sure there
         # is a corresponding remote branch
@@ -80,6 +85,6 @@  class PatchTestRepo(object):
             self._patchmerged = True
 
     def clean(self):
-        self.repo.git.execute(['git', 'checkout', self.current_branch])
+        self.repo.git.execute(['git', 'checkout', self.current_branch if self.current_branch else self.current_commit])
         self.repo.git.execute(['git', 'branch', '-D', self._workingbranch])
         self._patchmerged = False