Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contrib: match commit subject exactly when searching for upstream commit #13630

Merged
merged 1 commit into from
Oct 19, 2020

Conversation

tklauser
Copy link
Member

In generate_commit_list_for_pr, the commit subject is used to determine
the upstream commit ID from $REMOTE/master. However, if in the meantime
another commit with e.g. a Fixes tag that mentions this commit subject,
it appears first and leads to the original commit not being found. This
can be demonstrated using #13383:

 * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- https://github.com/cilium/cilium/pull/13383
   Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200!
     Branch:     master (!)                          refs/pull/13383/head
                 ----------                          -------------------
     v (start)
     |  Warning: No commit correlation found!    via dbac86cffc6d57e8c093d2821e0d794f4c13d284 ("daemon: Enable configuration of iptables --random-fully")
     |  350f0b36fd9b4cf23ebc11f4365c5c89591d0ff4 via 22d4554e963e2d8029ff95087ac03e55e90a7377 ("test: Test iptables masquerading with --random-fully")
     v (end)

$ # this is the git log command (with the subject added) from
$ # contrib/backporting/check-stable that should extract a single
$ # upstream commit
$ git log -F --since="1year" --pretty="%H %s" --no-merges --grep "daemon: Enable configuration of iptables --random-fully" origin/master
078ec543d36a8f5d6caed5c4649c74c72090ae20 install/kubernetes: consistent case spelling of iptables related values
4e39def13bca568a21087238877fbc60f8751567 daemon: Enable configuration of iptables --random-fully
$ git show 078ec543d36a8f5d6caed5c4649c74c72090ae20
commit 078ec543d36a8f5d6caed5c4649c74c72090ae20
Author: Tobias Klauser <tklauser@distanz.ch>
Date:   Wed Oct 14 11:58:29 2020 +0200

    install/kubernetes: consistent case spelling of iptables related values

    Make the case spelling of the newly introduced "ipTablesRandomFully"
    value consistent with other iptables option values which use the
    "iptables" spelling.

    Fixes: 4e39def13bca ("daemon: Enable configuration of iptables --random-fully")
    Signed-off-by: Tobias Klauser <tklauser@distanz.ch>

Note the Fixes: ... line in commit 078ec54 above.

Fix this behavior by grepping for the subject line from start of line:

$ git log -F --since="1year" --pretty="%H %s" --no-merges --extended-regexp --grep "^daemon: Enable configuration of iptables --random-fully" origin/master
4e39def13bca568a21087238877fbc60f8751567 daemon: Enable configuration of iptables --random-fully

 * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- https://github.com/cilium/cilium/pull/13383
   Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200!
     Branch:     master (!)                          refs/pull/13383/head
                 ----------                          -------------------
     v (start)
     |  4e39def13bca568a21087238877fbc60f8751567 via dbac86cffc6d57e8c093d2821e0d794f4c13d284 ("daemon: Enable configuration of iptables --random-fully")
     |  350f0b36fd9b4cf23ebc11f4365c5c89591d0ff4 via 22d4554e963e2d8029ff95087ac03e55e90a7377 ("test: Test iptables masquerading with --random-fully")
     v (end)

Reported-by: Robin Hahling robin.hahling@gw-computing.net

@tklauser tklauser added release-note/misc This PR makes changes that have no direct user impact. needs-backport/1.7 labels Oct 19, 2020
@tklauser tklauser requested a review from rolinh October 19, 2020 15:55
@tklauser tklauser requested a review from a team as a code owner October 19, 2020 15:55
In generate_commit_list_for_pr, the commit subject is used to determine
the upstream commit ID from $REMOTE/master. However, if in the meantime
another commit with e.g. a Fixes tag that mentions this commit subject,
it appears first and leads to the original commit not being found. This
can be demonstrated using #13383:

```
 * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383
   Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200!
     Branch:     master (!)                          refs/pull/13383/head
                 ----------                          -------------------
     v (start)
     |  Warning: No commit correlation found!    via dbac86c ("daemon: Enable configuration of iptables --random-fully")
     |  350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully")
     v (end)

$ # this is the git log command (with the subject added) from
$ # contrib/backporting/check-stable that should extract a single
$ # upstream commit
$ git log -F --since="1year" --pretty="%H %s" --no-merges --grep "daemon: Enable configuration of iptables --random-fully" origin/master
078ec54 install/kubernetes: consistent case spelling of iptables related values
4e39def daemon: Enable configuration of iptables --random-fully
$ git show 078ec54
commit 078ec54
Author: Tobias Klauser <tklauser@distanz.ch>
Date:   Wed Oct 14 11:58:29 2020 +0200

    install/kubernetes: consistent case spelling of iptables related values

    Make the case spelling of the newly introduced "ipTablesRandomFully"
    value consistent with other iptables option values which use the
    "iptables" spelling.

    Fixes: 4e39def ("daemon: Enable configuration of iptables --random-fully")
    Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
```

Note the `Fixes: ...` line in commit 078ec54 above.

Fix this behavior by grepping for the subject line from start of line:

```
$ git log -F --since="1year" --pretty="%H %s" --no-merges --extended-regexp --grep "^daemon: Enable configuration of iptables --random-fully" origin/master
4e39def daemon: Enable configuration of iptables --random-fully

 * PR: 13383 -- daemon: Enable configuration of iptables --random-fully (@kh34) -- #13383
   Merge with 2 commit(s) merged at: Wed, 14 Oct 2020 11:41:51 +0200!
     Branch:     master (!)                          refs/pull/13383/head
                 ----------                          -------------------
     v (start)
     |  4e39def via dbac86c ("daemon: Enable configuration of iptables --random-fully")
     |  350f0b3 via 22d4554 ("test: Test iptables masquerading with --random-fully")
     v (end)
```

Reported-by: Robin Hahling <robin.hahling@gw-computing.net>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.0-rc3 Oct 19, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.5 Oct 19, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.7.11 Oct 19, 2020
@tklauser tklauser force-pushed the pr/tklauser/fix-backport-subject-grep branch from b9a351b to c8495d3 Compare October 19, 2020 15:55
Copy link
Member

@rolinh rolinh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks!

@aanm aanm merged commit 6557f75 into master Oct 19, 2020
@aanm aanm deleted the pr/tklauser/fix-backport-subject-grep branch October 19, 2020 17:02
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport done to v1.7 in 1.7.11 Oct 20, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport done to v1.8 in 1.8.5 Oct 20, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport done to v1.8 in 1.8.5 Oct 20, 2020
@nathanjsweet nathanjsweet mentioned this pull request Oct 22, 2020
25 tasks
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.0-rc3 Oct 22, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.0-rc3 Oct 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.7.11
Backport done to v1.7
1.8.5
Backport done to v1.8
1.9.0-rc3
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

None yet

6 participants