[PATCH] ez: allow remote-tracking branches as 'base-branch'

[PATCH] ez: allow remote-tracking branches as 'base-branch'

[Philippe Blain]

Since 83b185a (ez: support enrolling branches using tags, 2022-08-16),
we use 'git show-ref --heads' to check if the argument given to
'--enroll-base' is a branch, so that the alternate code path can be
taken for tags. This means that since that commit, using a
remote-tracking branch as argument to '--enroll-base' does not work
well, because such branches are not shown by 'git show-ref --heads'. The
code path for tags is taken instead, and this usually leads to a
"Multiple branches contain object" error (since several local branches
are often based on the same remote-tracking branch).

Pass a fully-qualified ref 'refs/heads/<enroll_base>' as pattern to 'git
show-ref', and also pass it a second second pattern
'refs/remotes/<enroll_base>' to allow both local and remote-tracking
branches.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
 b4/ez.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/b4/ez.py b/b4/ez.py
index 74afddc..9cafe0c 100644
--- a/b4/ez.py
+++ b/b4/ez.py
@@ -370,7 +370,7 @@ def start_new_series(cmdargs: argparse.Namespace) -> None:
         slug = re.sub(r'\W+', '-', branchname).strip('-').lower()
         enroll_base = cmdargs.enroll_base
         # Is it a branch?
-        gitargs = ['show-ref', '--heads', enroll_base]
+        gitargs = ['show-ref', f'refs/heads/{enroll_base}', f'refs/remotes/{enroll_base}']
         lines = b4.git_get_command_lines(None, gitargs)
         if lines:
             try:

---
base-commit: a3281834d6d5dec44f58071fca2d22e04a97fe18
change-id: 20230219-allow-remote-branches-as-base-1ccdeb741f0c
--
b4 

Re: [PATCH] ez: allow remote-tracking branches as 'base-branch'

[Philippe Blain]

Hi Konstantin

Le 2023-02-19 à 19:45, Philippe Blain a écrit :
> Since 83b185a (ez: support enrolling branches using tags, 2022-08-16),
> we use 'git show-ref --heads' to check if the argument given to
> '--enroll-base' is a branch, so that the alternate code path can be
> taken for tags. This means that since that commit, using a
> remote-tracking branch as argument to '--enroll-base' does not work
> well, because such branches are not shown by 'git show-ref --heads'. The
> code path for tags is taken instead, and this usually leads to a
> "Multiple branches contain object" error (since several local branches
> are often based on the same remote-tracking branch).
> 
> Pass a fully-qualified ref 'refs/heads/<enroll_base>' as pattern to 'git
> show-ref', and also pass it a second second pattern
> 'refs/remotes/<enroll_base>' to allow both local and remote-tracking
> branches.

I'll send a v2 of this to also allow '@{u}' as enroll-base, so you might want
to wait before applying it so that the whole series can be applied at the same time.

Cheers,

Philippe.

[PATCH v2 0/4] ez: allow remote-tracking branches as ENROLL_BASE

[Philippe Blain]

This is a more complete take at allowing remote-tracking branches
as ENROLL_BASE in 'b4 prep --enroll'. The first commit is unchanged.

Changes in v2:
- New patch to also check remote-tracking branches when ENROLL_BASE is
  not a branch (2/4)
- New patches (3/4-4/4) to support '@{u}'
- Rebased on master
- Link to v1: https://msgid.link/20230219-allow-remote-branches-as-base-v1-1-a1cf7948b2cc@gmail.com

---
Philippe Blain (4):
      ez: allow remote-tracking branches as ENROLL_BASE
      ez: also check remote-tracking branches when ENROLL_BASE is not a branch
      ez: allow '@{upstream}' as ENROLL_BASE
      ez: let ENROLL_BASE default to '@{upstream}'

 b4/__init__.py |  4 +++-
 b4/command.py  |  4 ++--
 b4/ez.py       | 15 ++++++++++++---
 3 files changed, 17 insertions(+), 6 deletions(-)
---
base-commit: bd1bf7905f2f9c2340b75b5074285105a6fa35f3
change-id: 20230219-allow-remote-branches-as-base-1ccdeb741f0c
--
b4 

[PATCH v2 1/4] ez: allow remote-tracking branches as ENROLL_BASE

[Philippe Blain]

Since 83b185a (ez: support enrolling branches using tags, 2022-08-16),
we use 'git show-ref --heads' to check if the argument given to
'--enroll-base' is a branch, so that the alternate code path can be
taken for tags. This means that since that commit, using a
remote-tracking branch as argument to '--enroll-base' does not work
well, because such branches are not shown by 'git show-ref --heads'. The
code path for tags is taken instead, and this usually leads to a
"Multiple branches contain object" error (since several local branches
are often based on the same remote-tracking branch).

Pass a fully-qualified ref 'refs/heads/<enroll_base>' as pattern to 'git
show-ref', and also pass it a second second pattern
'refs/remotes/<enroll_base>' to allow both local and remote-tracking
branches.

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
 b4/ez.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/b4/ez.py b/b4/ez.py
index daa1b4f..c3b0237 100644
--- a/b4/ez.py
+++ b/b4/ez.py
@@ -371,7 +371,7 @@ def start_new_series(cmdargs: argparse.Namespace) -> None:
         slug = re.sub(r'\W+', '-', branchname).strip('-').lower()
         enroll_base = cmdargs.enroll_base
         # Is it a branch?
-        gitargs = ['show-ref', '--heads', enroll_base]
+        gitargs = ['show-ref', f'refs/heads/{enroll_base}', f'refs/remotes/{enroll_base}']
         lines = b4.git_get_command_lines(None, gitargs)
         if lines:
             try:

-- 
2.34.1

[PATCH v2 2/4] ez: also check remote-tracking branches when ENROLL_BASE is not a branch

[Philippe Blain]

When invoking 'b4 prep --enroll ENROLL_BASE' with a revision ENROLL_BASE
that does not correspond to a branch, ez.py::start_new_series tries to
find branches where the commit corresponding to the given revision is
found. If there is only one such branch, it is used as the
'base-branch'.

The function 'git_branch_contains' used to find where the commit lives
invokes 'git branch --contains', so only local branches are checked.
This means that if the given revision corresponds to a remote-tracking
branch, git_branch_contains won't recognize it and we will hit:

    CRITICAL: No other branch contains %s: cannot use as fork base

Ensure that remote-tracking branches are also checked by adding an
optional 'checkall' argument to 'git_branch_contains', that in turns
adds a '--all' argument to the 'git branch' invocation, such that both
local and remote-tracking branches are checked. Add that argument to the
call in 'start_new_series'.

An alternative would be to unconditionnally invoke 'git branch' with
'--all' in 'git_branch_contains', but this function is also called in
pr.py::main and ty.py::auto_locate_pr to check if the tip commit of the
given pull requests already exists in a local branch, and it seems safer
to not change behaviour for these two functions.
---
 b4/__init__.py | 4 +++-
 b4/ez.py       | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/b4/__init__.py b/b4/__init__.py
index 3138f3c..78418c1 100644
--- a/b4/__init__.py
+++ b/b4/__init__.py
@@ -2986,8 +2986,10 @@ def git_revparse_tag(gitdir: Optional[str], tagname: str) -> Optional[str]:
     return out.strip()
 
 
-def git_branch_contains(gitdir: Optional[str], commit_id: str) -> List[str]:
+def git_branch_contains(gitdir: Optional[str], commit_id: str, checkall: bool = False) -> List[str]:
     gitargs = ['branch', '--format=%(refname:short)', '--contains', commit_id]
+    if checkall:
+        gitargs.append('--all')
     lines = git_get_command_lines(gitdir, gitargs)
     return lines
 
diff --git a/b4/ez.py b/b4/ez.py
index c3b0237..ce080f1 100644
--- a/b4/ez.py
+++ b/b4/ez.py
@@ -390,7 +390,7 @@ def start_new_series(cmdargs: argparse.Namespace) -> None:
                 raise RuntimeError('Object %s not found' % enroll_base)
             forkpoint = out.strip()
             # check branches where this object lives
-            heads = b4.git_branch_contains(None, forkpoint)
+            heads = b4.git_branch_contains(None, forkpoint, checkall=True)
             if mybranch not in heads:
                 logger.critical('CRITICAL: object %s does not exist on current branch', enroll_base)
                 sys.exit(1)

-- 
2.34.1

[PATCH v2 3/4] ez: allow '@{upstream}' as ENROLL_BASE

[Philippe Blain]

Invoking 'b4 prep --enroll @{u}' (or @{upstream}) does not work reliably
with the current code. In 'start_new_series', we take the code path that
checks on which branch the object given as ENROLL_BASE lives using 'git
branch --all --contains', so if multiple local branches build on top of
the current branch's upstream branch, they are all listed by this
invocation of 'git branch' and so we error with 'CRITICAL: Multiple
branches contain object @{u}, please pass a branch name as base'.

Fix that by invoking 'git rev-parse --abbrev-ref --verify ENROLL_BASE'
before the 'git show-branch' invocation. That command will convert @{u},
@{upstream} and @{push} [1-2] to an abbreviated ref like
'upstream/master'. If the given revision exist but can't be directly
converted to an abbreviated ref, the command will succeed and not output
anything, so only change ENROLL_BASE when we actually get an output. We
avoid checking the error code as we already invoke 'git rev-parse
--verify' later on to check if the object exists. We could use
'git_get_command_lines' instead, but we will check the error code in a
subsequent commit, so it makes more sense to use 'git_run_command'.

While at it, fix a "CRITICAL" error message that was missing a value for
the '%s' string formatting operator.

[1] https://git-scm.com/docs/gitrevisions#Documentation/gitrevisions.txt-emltbranchnamegtupstreamemegemmasterupstreamememuem
[2] https://git-scm.com/docs/gitrevisions#Documentation/gitrevisions.txt-emltbranchnamegtpushemegemmasterpushemempushem
---
 b4/ez.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/b4/ez.py b/b4/ez.py
index ce080f1..53a4b2f 100644
--- a/b4/ez.py
+++ b/b4/ez.py
@@ -370,6 +370,11 @@ def start_new_series(cmdargs: argparse.Namespace) -> None:
         seriesname = branchname
         slug = re.sub(r'\W+', '-', branchname).strip('-').lower()
         enroll_base = cmdargs.enroll_base
+        # Convert @{upstream}, @{push} to an abbreviated ref
+        gitargs = ['rev-parse', '--abbrev-ref', '--verify', enroll_base]
+        ecode, out = b4.git_run_command(None, gitargs)
+        if out:
+            enroll_base = out.strip()
         # Is it a branch?
         gitargs = ['show-ref', f'refs/heads/{enroll_base}', f'refs/remotes/{enroll_base}']
         lines = b4.git_get_command_lines(None, gitargs)
@@ -377,7 +382,7 @@ def start_new_series(cmdargs: argparse.Namespace) -> None:
             try:
                 forkpoint = get_base_forkpoint(enroll_base, mybranch)
             except RuntimeError as ex:
-                logger.critical('CRITICAL: could not use %s as enrollment base:')
+                logger.critical('CRITICAL: could not use %s as enrollment base:', enroll_base)
                 logger.critical('          %s', ex)
                 sys.exit(1)
             basebranch = enroll_base

-- 
2.34.1

[PATCH v2 4/4] ez: let ENROLL_BASE default to '@{upstream}'

[Philippe Blain]

It is a common worflow to set 'branch.<name>.merge' and
'branch.<name>.remote' to the remote-tracking branch that corresponds to
the default branch of the canonical repository of the project (say
'upstream/master'), such that 'git rebase -i' automatically "does the
right thing" and rebases only the commits in your feature branch that
are not reachable from 'upstream/master'. The revision syntax
'@{upstream}' (short: '@{u}') can be used to denote the configured
upstream branch of the current branch.

Support that workflow better by allowing the 'ENROLL_BASE' argument to
'b4 prep --enroll' to be ommitted and default to @{upstream}.

To avoid a RuntimeError when enroll_base is @{upstream} (either from the
default or because it was given litterally on the command line) and the
current branch has no configured upstream, add some error-checking to
the 'git rev-parse --verify --abbrev-ref' invocation along with a clear
message. Let other error situations be handled by the second 'git
rev-parse --verify' invocation later on.
---
 b4/command.py | 4 ++--
 b4/ez.py      | 6 +++++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/b4/command.py b/b4/command.py
index e7656a5..1ae73d2 100644
--- a/b4/command.py
+++ b/b4/command.py
@@ -290,8 +290,8 @@ def setup_parser() -> argparse.ArgumentParser:
     ag_prepn.add_argument('-F', '--from-thread', metavar='MSGID', dest='msgid',
                           help='When creating a new branch, use this thread')
     ag_prepe = sp_prep.add_argument_group('Enroll existing branch', 'Enroll existing branch for prep work')
-    ag_prepe.add_argument('-e', '--enroll', dest='enroll_base',
-                          help='Enroll current branch, using the passed tag, branch, or commit as fork base')
+    ag_prepe.add_argument('-e', '--enroll', dest='enroll_base', nargs='?', const='@{upstream}',
+                          help='Enroll current branch, using its configured upstream branch as fork base, or the passed tag, branch, or commit')
     sp_prep.set_defaults(func=cmd_prep)
 
     # b4 trailers
diff --git a/b4/ez.py b/b4/ez.py
index 53a4b2f..0a2936a 100644
--- a/b4/ez.py
+++ b/b4/ez.py
@@ -373,7 +373,11 @@ def start_new_series(cmdargs: argparse.Namespace) -> None:
         # Convert @{upstream}, @{push} to an abbreviated ref
         gitargs = ['rev-parse', '--abbrev-ref', '--verify', enroll_base]
         ecode, out = b4.git_run_command(None, gitargs)
-        if out:
+        if ecode > 0:
+            if enroll_base == '@{upstream}' or enroll_base == '@{u}':
+                logger.critical('CRITICAL: current branch has no configured upstream')
+                sys.exit(1)
+        elif out:
             enroll_base = out.strip()
         # Is it a branch?
         gitargs = ['show-ref', f'refs/heads/{enroll_base}', f'refs/remotes/{enroll_base}']

-- 
2.34.1

Re: [PATCH v2 0/4] ez: allow remote-tracking branches as ENROLL_BASE

[Konstantin Ryabitsev]

On Mon, 06 Mar 2023 13:02:08 -0500, Philippe Blain wrote:
> This is a more complete take at allowing remote-tracking branches
> as ENROLL_BASE in 'b4 prep --enroll'. The first commit is unchanged.
> 
> Changes in v2:
> - New patch to also check remote-tracking branches when ENROLL_BASE is
>   not a branch (2/4)
> - New patches (3/4-4/4) to support '@{u}'
> - Rebased on master
> - Link to v1: https://msgid.link/20230219-allow-remote-branches-as-base-v1-1-a1cf7948b2cc@gmail.com
> 
> [...]

Applied, thanks!

[1/4] ez: allow remote-tracking branches as ENROLL_BASE
      commit: 800dae4b48fbbfb2bd52e0bc2da351b0627659b8
[2/4] ez: also check remote-tracking branches when ENROLL_BASE is not a branch
      commit: 6d27eef275c2b88663b446f156293a5c3c7ad7c6
[3/4] ez: allow '@{upstream}' as ENROLL_BASE
      commit: 1f34f802252976549d82338807c75729766653c6
[4/4] ez: let ENROLL_BASE default to '@{upstream}'
      commit: 800dae4b48fbbfb2bd52e0bc2da351b0627659b8

Best regards,
-- 
Konstantin Ryabitsev <konstantin@linuxfoundation.org>