[04/28] devtool: auto-pick 'main' branch in addition to 'master' in upgrades

Message ID 20220217160939.1424397-4-alex@linutronix.de
State New
Headers show
Series [01/28] gdb: update 11.1 -> 11.2 | expand

Commit Message

Alexander Kanavin Feb. 17, 2022, 4:09 p.m. UTC
Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 scripts/lib/devtool/upgrade.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Peter Kjellerstedt Feb. 17, 2022, 5:36 p.m. UTC | #1
> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Alexander Kanavin
> Sent: den 17 februari 2022 17:09
> To: openembedded-core@lists.openembedded.org
> Cc: Alexander Kanavin <alex@linutronix.de>
> Subject: [OE-core] [PATCH 04/28] devtool: auto-pick 'main' branch in addition to 'master' in upgrades
> 
> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> ---
>  scripts/lib/devtool/upgrade.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/lib/devtool/upgrade.py b/scripts/lib/devtool/upgrade.py
> index 826a3f955f..7957d41192 100644
> --- a/scripts/lib/devtool/upgrade.py
> +++ b/scripts/lib/devtool/upgrade.py
> @@ -192,7 +192,7 @@ def _extract_new_source(newpv, srctree, no_patch, srcrev, srcbranch, branch, kee
>              get_branch = [x.strip() for x in check_branch.splitlines()]
>              # Remove HEAD reference point and drop remote prefix
>              get_branch = [x.split('/', 1)[1] for x in get_branch if not x.startswith('origin/HEAD')]
> -            if 'master' in get_branch:
> +            if 'master' in get_branch or 'main' in get_branch:
>                  # If it is master, we do not need to append 'branch=master' as this is default.
>                  # Even with the case where get_branch has multiple objects, if 'master' is one
>                  # of them, we should default take from 'master'
> --
> 2.20.1

This seems incorrect since the branch= parameter is always 
required nowadays. I would more expect the code to look something 
like this:

            if len(get_branch) == 1:
                # If srcrev is on only ONE branch, then use that branch
                srcbranch = get_branch[0]
            elif 'main' in get_branch:
                # If srcrev is on multiple branches, then choose 'main' if it is one of them
                srcbranch = 'main'
            elif 'master' in get_branch:
                # Otherwise choose 'master' if it is one of the branches
                srcbranch = 'master'
            else:
                ...

//Peter
Alexander Kanavin Feb. 17, 2022, 5:55 p.m. UTC | #2
On Thu, 17 Feb 2022 at 18:36, Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
> This seems incorrect since the branch= parameter is always
> required nowadays. I would more expect the code to look something
> like this:
>
>             if len(get_branch) == 1:
>                 # If srcrev is on only ONE branch, then use that branch
>                 srcbranch = get_branch[0]
>             elif 'main' in get_branch:
>                 # If srcrev is on multiple branches, then choose 'main' if it is one of them
>                 srcbranch = 'main'
>             elif 'master' in get_branch:
>                 # Otherwise choose 'master' if it is one of the branches
>                 srcbranch = 'master'
>             else:

It has worked fine where the branch is still master; I admit I don't
fully understand how this works, but it did help with a failed update
where devtool wasn't able to pick 'main' from several branches
containing the same SRCREV. Upgrade functionality isn't easy to test
as it is made of a million corner cases and runs against a 'live',
constantly evolving set of pending updates, so I mostly adjust it from
experience with AUH. If this still isn't good enough (as show by
actual failed updates), I'll adjust further.

Alex
Alexander Kanavin Feb. 17, 2022, 6:21 p.m. UTC | #3
On Thu, 17 Feb 2022 at 18:57, Alexander Kanavin via
lists.openembedded.org <alex.kanavin=gmail.com@lists.openembedded.org>
wrote:
> It has worked fine where the branch is still master; I admit I don't
> fully understand how this works, but it did help with a failed update
> where devtool wasn't able to pick 'main' from several branches
> containing the same SRCREV. Upgrade functionality isn't easy to test
> as it is made of a million corner cases and runs against a 'live',
> constantly evolving set of pending updates, so I mostly adjust it from
> experience with AUH. If this still isn't good enough (as show by
> actual failed updates), I'll adjust further.

That said, I'll try if the suggested change works as it should and
resend if so :)

Alex
Khem Raj Feb. 17, 2022, 8:02 p.m. UTC | #4
On 2/17/22 10:21 AM, Alexander Kanavin wrote:
> On Thu, 17 Feb 2022 at 18:57, Alexander Kanavin via
> lists.openembedded.org <alex.kanavin=gmail.com@lists.openembedded.org>
> wrote:
>> It has worked fine where the branch is still master; I admit I don't
>> fully understand how this works, but it did help with a failed update
>> where devtool wasn't able to pick 'main' from several branches
>> containing the same SRCREV. Upgrade functionality isn't easy to test
>> as it is made of a million corner cases and runs against a 'live',
>> constantly evolving set of pending updates, so I mostly adjust it from
>> experience with AUH. If this still isn't good enough (as show by
>> actual failed updates), I'll adjust further.
> 
> That said, I'll try if the suggested change works as it should and
> resend if so :)

I worry about riscv emulators will break, perhaps riscv32 for sure.

> 
> Alex
> 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#161852): https://lists.openembedded.org/g/openembedded-core/message/161852
> Mute This Topic: https://lists.openembedded.org/mt/89212438/1997914
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Patch

diff --git a/scripts/lib/devtool/upgrade.py b/scripts/lib/devtool/upgrade.py
index 826a3f955f..7957d41192 100644
--- a/scripts/lib/devtool/upgrade.py
+++ b/scripts/lib/devtool/upgrade.py
@@ -192,7 +192,7 @@  def _extract_new_source(newpv, srctree, no_patch, srcrev, srcbranch, branch, kee
             get_branch = [x.strip() for x in check_branch.splitlines()]
             # Remove HEAD reference point and drop remote prefix
             get_branch = [x.split('/', 1)[1] for x in get_branch if not x.startswith('origin/HEAD')]
-            if 'master' in get_branch:
+            if 'master' in get_branch or 'main' in get_branch:
                 # If it is master, we do not need to append 'branch=master' as this is default.
                 # Even with the case where get_branch has multiple objects, if 'master' is one
                 # of them, we should default take from 'master'