[PATCH 1/2] check_applies_clean: Don't append '.git' repeatedly

[PATCH 1/2] check_applies_clean: Don't append '.git' repeatedly

[Rob Herring]

In check_applies_clean, each iteration of the file loop in appends '.git'
to 'gitdir' again which is obviously wrong.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 b4/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/b4/__init__.py b/b4/__init__.py
index 8d8911d392b5..ddf38293dce2 100644
--- a/b4/__init__.py
+++ b/b4/__init__.py
@@ -655,7 +655,7 @@ class LoreSeries:
                     logger.debug('Checking hash on %s:%s', when, fn)
                     # XXX: We should probably pipe the two commands instead of reading into memory,
                     #      so something to consider for the future
-                    ecode, out = git_run_command(gitdir, ['show', f'{when}:{fn}'])
+                    ecode, out = git_run_command(os.path.join(gitdir, '.git'), ['show', f'{when}:{fn}'])
                     if ecode > 0:
                         # Couldn't get this file, continue
                         logger.debug('Could not look up %s:%s', when, fn)
-- 
2.27.0

[PATCH 2/2] am: Allow specifying a base to check applying series

[Rob Herring]

Add a new option, '--check-base', to allow the user to specify a specific
base git ref to check applying a series to.

Signed-off-by: Rob Herring <robh@kernel.org>
---
I'm wondering if this should take a list (in order of preference) instead.
If so, then the 'Base: ...' message should be reworked to be easier to
parse which base was found.
---
 b4/command.py | 2 ++
 b4/mbox.py    | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/b4/command.py b/b4/command.py
index 2d6994dd5111..2d48dafa21f7 100644
--- a/b4/command.py
+++ b/b4/command.py
@@ -120,6 +120,8 @@ def cmd():
                             '"-P *globbing*" to match on commit subject)')
     sp_am.add_argument('-g', '--guess-base', dest='guessbase', action='store_true', default=False,
                        help='Try to guess the base of the series (if not specified)')
+    sp_am.add_argument('-b', '--check-base', dest='checkbase', default='HEAD',
+                       help='Check if series applies to specified ref base (HEAD if not specified)')
     sp_am.add_argument('-3', '--prep-3way', dest='threeway', action='store_true', default=False,
                        help='Prepare for a 3-way merge '
                             '(tries to ensure that all index blobs exist by making a fake commit range)')
diff --git a/b4/mbox.py b/b4/mbox.py
index e722d05a79aa..c660fb45fb36 100644
--- a/b4/mbox.py
+++ b/b4/mbox.py
@@ -234,9 +234,9 @@ def make_am(msgs, cmdargs, msgid):
     else:
         cleanmsg = ''
         if topdir is not None:
-            checked, mismatches = lser.check_applies_clean(topdir)
+            checked, mismatches = lser.check_applies_clean(topdir, cmdargs.checkbase)
             if mismatches == 0 and checked != mismatches:
-                cleanmsg = ' (applies clean to current tree)'
+                cleanmsg = ' (applies clean to %s)' % cmdargs.checkbase
             elif cmdargs.guessbase:
                 # Look at the last 10 tags and see if it applies cleanly to
                 # any of them. I'm not sure how useful this is, but I'm going
-- 
2.27.0

Re: [PATCH 2/2] am: Allow specifying a base to check applying series

[Konstantin Ryabitsev]

On Fri, Jun 18, 2021 at 12:24:52PM -0600, Rob Herring wrote:
> Add a new option, '--check-base', to allow the user to specify a specific
> base git ref to check applying a series to.

I just pushed a reimplementation of --guess-base into master -- can you see if
it does closer to something that you want?

-K

Re: [PATCH 2/2] am: Allow specifying a base to check applying series

[Rob Herring]

On Mon, Jun 21, 2021 at 2:03 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
>
> On Fri, Jun 18, 2021 at 12:24:52PM -0600, Rob Herring wrote:
> > Add a new option, '--check-base', to allow the user to specify a specific
> > base git ref to check applying a series to.
>
> I just pushed a reimplementation of --guess-base into master -- can you see if
> it does closer to something that you want?

Yeah, it works. It's a bit confusing that the result is 'Base: current
tree' when it's not HEAD.

The more I think about it, the more I think '-b' needs to be a list of
refs or should be a separate sub-command and the user can iterate thru
branches themselves. It would work as-is, but it's kind of pointless
to repeatedly go thru all the 'b4 am' steps each time. A separate
sub-command would keep with the unix way of commands that do 1 thing.

I don't know that the guessing algorithm is any better now. When I ran
it with my current tree being something a series doesn't apply to, it
gave me some pretty arbitrary ref: v5.13-rc1-69-gcfe34bb7a770 (which I
think happens to be the base I just did a rebase on)

Rob

Re: [PATCH 2/2] am: Allow specifying a base to check applying series

[Konstantin Ryabitsev]

On Mon, Jun 21, 2021 at 05:07:34PM -0600, Rob Herring wrote:
> > I just pushed a reimplementation of --guess-base into master -- can you see if
> > it does closer to something that you want?
> 
> Yeah, it works. It's a bit confusing that the result is 'Base: current
> tree' when it's not HEAD.

I've added some refinements that will hopefully improve both the matching
algorithm and the output when using -b.

> The more I think about it, the more I think '-b' needs to be a list of
> refs or should be a separate sub-command and the user can iterate thru
> branches themselves. It would work as-is, but it's kind of pointless
> to repeatedly go thru all the 'b4 am' steps each time. A separate
> sub-command would keep with the unix way of commands that do 1 thing.

By default, we'll use --all when matching blob indexes, but if -b is
specified, we'll pass it as-is to --branches, which is actually a pattern (see
"man git-log").

> I don't know that the guessing algorithm is any better now. When I ran
> it with my current tree being something a series doesn't apply to, it
> gave me some pretty arbitrary ref: v5.13-rc1-69-gcfe34bb7a770 (which I
> think happens to be the base I just did a rebase on)

I believe I've improved it in the latest commit.

NB: I think it's important to recognize that "all blob indexes matched" is not
a guarantee that the base we find is actually accurate or useful, at least not
for the purposes of CI -- a patch may be dependent on another set of changes
that modify totally different blobs than in the patch we're looking at.

E.g.: imagine that you are looking at a patch that changes files lib/foo.c and
lib/bar.c, but these changes are dependent on lib/baz.c. If you apply a patch
that modifies lib/baz.c, then the indexes for foo.c and bar.c would still
match the current tree, but the correct base-commit would actually be *before*
the baz.c modification.

We already try to handle this situation by not considering changes applied
after the patch was submitted (we force --until to be the date of the patch),
but providing an actual base-commit info in the patch/series is always going
to be more precise and reliable.

-K

Re: [PATCH 1/2] check_applies_clean: Don't append '.git' repeatedly

[Konstantin Ryabitsev]

On Fri, Jun 18, 2021 at 12:24:51PM -0600, Rob Herring wrote:
> In check_applies_clean, each iteration of the file loop in appends '.git'
> to 'gitdir' again which is obviously wrong.

There was already a fix for this in master and stable-0.7.y.

-K