Linux maintainer tooling and workflows
 help / color / Atom feed
* [PATCH 1/2] check_applies_clean: Don't append '.git' repeatedly
@ 2021-06-18 18:24 Rob Herring
  2021-06-18 18:24 ` [PATCH 2/2] am: Allow specifying a base to check applying series Rob Herring
  2021-06-21 20:02 ` [PATCH 1/2] check_applies_clean: Don't append '.git' repeatedly Konstantin Ryabitsev
  0 siblings, 2 replies; 6+ messages in thread
From: Rob Herring @ 2021-06-18 18:24 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: tools

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] am: Allow specifying a base to check applying series
  2021-06-18 18:24 [PATCH 1/2] check_applies_clean: Don't append '.git' repeatedly Rob Herring
@ 2021-06-18 18:24 ` Rob Herring
  2021-06-21 20:03   ` Konstantin Ryabitsev
  2021-06-21 20:02 ` [PATCH 1/2] check_applies_clean: Don't append '.git' repeatedly Konstantin Ryabitsev
  1 sibling, 1 reply; 6+ messages in thread
From: Rob Herring @ 2021-06-18 18:24 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: tools

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] check_applies_clean: Don't append '.git' repeatedly
  2021-06-18 18:24 [PATCH 1/2] check_applies_clean: Don't append '.git' repeatedly Rob Herring
  2021-06-18 18:24 ` [PATCH 2/2] am: Allow specifying a base to check applying series Rob Herring
@ 2021-06-21 20:02 ` Konstantin Ryabitsev
  1 sibling, 0 replies; 6+ messages in thread
From: Konstantin Ryabitsev @ 2021-06-21 20:02 UTC (permalink / raw)
  To: Rob Herring; +Cc: tools

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] am: Allow specifying a base to check applying series
  2021-06-18 18:24 ` [PATCH 2/2] am: Allow specifying a base to check applying series Rob Herring
@ 2021-06-21 20:03   ` Konstantin Ryabitsev
  2021-06-21 23:07     ` Rob Herring
  0 siblings, 1 reply; 6+ messages in thread
From: Konstantin Ryabitsev @ 2021-06-21 20:03 UTC (permalink / raw)
  To: Rob Herring; +Cc: tools

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] am: Allow specifying a base to check applying series
  2021-06-21 20:03   ` Konstantin Ryabitsev
@ 2021-06-21 23:07     ` Rob Herring
  2021-06-22 14:23       ` Konstantin Ryabitsev
  0 siblings, 1 reply; 6+ messages in thread
From: Rob Herring @ 2021-06-21 23:07 UTC (permalink / raw)
  To: Konstantin Ryabitsev; +Cc: tools

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] am: Allow specifying a base to check applying series
  2021-06-21 23:07     ` Rob Herring
@ 2021-06-22 14:23       ` Konstantin Ryabitsev
  0 siblings, 0 replies; 6+ messages in thread
From: Konstantin Ryabitsev @ 2021-06-22 14:23 UTC (permalink / raw)
  To: Rob Herring; +Cc: tools

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-18 18:24 [PATCH 1/2] check_applies_clean: Don't append '.git' repeatedly Rob Herring
2021-06-18 18:24 ` [PATCH 2/2] am: Allow specifying a base to check applying series Rob Herring
2021-06-21 20:03   ` Konstantin Ryabitsev
2021-06-21 23:07     ` Rob Herring
2021-06-22 14:23       ` Konstantin Ryabitsev
2021-06-21 20:02 ` [PATCH 1/2] check_applies_clean: Don't append '.git' repeatedly Konstantin Ryabitsev

Linux maintainer tooling and workflows

Archives are clonable:
	git clone --mirror https://lore.kernel.org/tools/0 tools/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tools tools/ https://lore.kernel.org/tools \
		tools@linux.kernel.org
	public-inbox-index tools

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.linux.tools


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git