Skip to contentSkip to conversation

b4: ty: Fix incorrect patch/commit association

PATCH Applied Will Deacon 2 participants 7 messages Mar 4, 2024 – May 2, 2024

lore
Share & tools
lore
#0 lore raw

v2 cover b4: ty: Fix incorrect patch/commit association

Hi folks,

It's me again. This time I fixed my git configuration so that the authorship is correct and matches the S-o-B line. That's the only change since the v1 I sent on Friday.

Enjoy!

Will

Cc Konstantin Ryabitsev <[email protected]>
Cc Jason Gunthorpe <[email protected]>
Cc Nicolin Chen <[email protected]>

--->8

Will Deacon (2):
ty: Try subject matching for every commit before using trackers
ty: Remove unused 'matches' variable

 src/b4/ty.py | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
Signature · 1 line
2.44.0.rc1.240.g4c46232300-goog
#1 lore raw

PATCH v2 1/2 ty: Try subject matching for every commit before using trackers

Nicolin reports that 'b4 ty' produced a "thank you" letter with incorrect commit IDs:

https://lore.kernel.org/r/ZeDtfKMUXnOnJOic@Asurada-Nvidia

This appears to be because of the creative use of Message-Ids in the series at:

https://lore.kernel.org/r/[email protected]

For example, comparing the Message-Id of patches 1 and 11 of that series, we can see that one is a substring of the other:

Message-Id: <[email protected]>
Message-Id: <[email protected]>

This confuses auto_locate_series(), as it will end up matching the Message-Id of patch 1 with the Link: tag in patch 11 and therefore get the wrong patch/commit association.

Fix this problem by trying to match each commit based on the Subject before falling back to the fuzzier tracker-based matching.

---
 src/b4/ty.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
src/b4/ty.py +7 -1
index a8bdabf5bcbc..950c357a89c5 100644
@@ -217,7 +217,13 @@ def auto_locate_series(gitdir: Optional[str], jsondata: dict, branch: str,
                     success = True
                     matches += 1
                     break
-                elif len(patch) > 2 and len(patch[2]) and len(commit[2]):
+
+            if success:
+                continue
+
+            # try to locate by tracker
+            for pwhash, commit in commits.items():
+                if len(patch) > 2 and len(patch[2]) and len(commit[2]):
                     for tracker in commit[2]:
                         if tracker.find(patch[2]) >= 0:
                             logger.debug('Matched using recorded message-id')
Signature · 1 line
2.44.0.rc1.240.g4c46232300-goog
#2 lore raw

PATCH v2 2/2 ty: Remove unused 'matches' variable

In reply to Will Deacon

The 'matches' variable in auto_locate_series() is updated but not used for anything, so remove it.

---
 src/b4/ty.py | 4 ----
 1 file changed, 4 deletions(-)
src/b4/ty.py +0 -4
index 950c357a89c5..97f4c44a0832 100644
@@ -198,7 +198,6 @@ def auto_locate_series(gitdir: Optional[str], jsondata: dict, branch: str,
     patchids = set(commits.keys())
     # We need to find all of them in the commits
     found = list()
-    matches = 0
     at = 0
     for patch in jsondata['patches']:
         at += 1
@@ -206,7 +205,6 @@ def auto_locate_series(gitdir: Optional[str], jsondata: dict, branch: str,
         if patch[1] in patchids:
             logger.debug('Found: %s', patch[0])
             found.append((at, commits[patch[1]][0]))
-            matches += 1
         else:
             # try to locate by subject
             success = False
@@ -215,7 +213,6 @@ def auto_locate_series(gitdir: Optional[str], jsondata: dict, branch: str,
                     logger.debug('Matched using subject')
                     found.append((at, commit[0]))
                     success = True
-                    matches += 1
                     break
 
             if success:
@@ -229,7 +226,6 @@ def auto_locate_series(gitdir: Optional[str], jsondata: dict, branch: str,
                             logger.debug('Matched using recorded message-id')
                             found.append((at, commit[0]))
                             success = True
-                            matches += 1
                             break
                 if success:
                     break
Signature · 1 line
2.44.0.rc1.240.g4c46232300-goog
#3 lore raw
In reply to Will Deacon
Quoted 9 lines · Will Deacon

It's me again. This time I fixed my git configuration so that the authorship is correct and matches the S-o-B line. That's the only change since the v1 I sent on Friday.

Enjoy!

Will

[...]

Applied, thanks!

[1/2] ty: Try subject matching for every commit before using trackers

      commit: 8a3d8bfae18ad1f4c72b91e6a8d37e12f9b5dcb4
[2/2] ty: Remove unused 'matches' variable
      commit: bb6d9075ae89df46ad91e85466a7b01f6aa534d8

Best regards,

Signature · 1 line
Konstantin Ryabitsev <[email protected]>
#4 lore raw
In reply to Will Deacon
Quoted 9 lines · Will Deacon

It's me again. This time I fixed my git configuration so that the authorship is correct and matches the S-o-B line. That's the only change since the v1 I sent on Friday.

Enjoy!

Will

[...]

Applied, thanks!

[1/2] ty: Try subject matching for every commit before using trackers

      commit: 47bd55d3182d875802700670270f5ee0877a8d5d
[2/2] ty: Remove unused 'matches' variable
      commit: dced92071d2135ccbec521cb4face63cafcdc407

Best regards,

Signature · 1 line
Konstantin Ryabitsev <[email protected]>
#5 lore raw
Quoted 10 lines · Konstantin Ryabitsev
Quoted 8 lines · Will Deacon

It's me again. This time I fixed my git configuration so that the authorship is correct and matches the S-o-B line. That's the only change since the v1 I sent on Friday.

Enjoy!

Will

Quoted 8 lines
Quoted 1 line

[...]

Applied, thanks!

[1/2] ty: Try subject matching for every commit before using trackers

      commit: 47bd55d3182d875802700670270f5ee0877a8d5d
[2/2] ty: Remove unused 'matches' variable
      commit: dced92071d2135ccbec521cb4face63cafcdc407

You applied these already (with different SHAs), I think:

https://lore.kernel.org/r/[email protected]

So I'm not sure what's happened here :/

Will

#6 lore raw
Quoted 7 lines · Will Deacon
Quoted 6 lines

Applied, thanks!

[1/2] ty: Try subject matching for every commit before using trackers

      commit: 47bd55d3182d875802700670270f5ee0877a8d5d
[2/2] ty: Remove unused 'matches' variable
      commit: dced92071d2135ccbec521cb4face63cafcdc407
Quoted 5 lines

You applied these already (with different SHAs), I think:

https://lore.kernel.org/r/[email protected]

So I'm not sure what's happened here :/

Bah, you're right. I blame the split brain between b4 and patchwork here -- for some reason in patchwork these were still showing as unprocessed.

Effectively the 2nd time was a no-op:

  $ git log --oneline --reverse 590044aff85f3e11cbd5efb9e192847bb3b51b3f..
  47bd55d ty: Try subject matching for every commit before using trackers
  dced920 ty: Remove unused 'matches' variable
  775893e (HEAD -> master, origin/master, github/master) Merge patch series "b4: ty: Fix incorrect patch/commit association"
  $ git diff 590044aff85f3e11cbd5efb9e192847bb3b51b3f..
  (no output)

I am actually annoyed that this operation doesn't throw an error and doubly annoyed that the only way to fix this is to rebase -- but rebase I will.

Sorry about the mess.

-K