| Message ID | 20210618182452.2577871-2-robh@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] check_applies_clean: Don't append '.git' repeatedly | expand |
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
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
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
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
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(-)