From 42636a3e2c0ddb0fcbcfe887f56a5a070baeea0d Mon Sep 17 00:00:00 2001 From: jack1142 <6032823+jack1142@users.noreply.github.com> Date: Sun, 8 Aug 2021 20:32:55 +0200 Subject: [PATCH 1/4] Add some tests --- cherry_picker/test_cherry_picker.py | 90 +++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/cherry_picker/test_cherry_picker.py b/cherry_picker/test_cherry_picker.py index 6f733e3..b05d607 100644 --- a/cherry_picker/test_cherry_picker.py +++ b/cherry_picker/test_cherry_picker.py @@ -468,6 +468,96 @@ def test_normalize_short_commit_message(): ) +@pytest.mark.parametrize( + "commit_message,expected_commit_message", + ( + # ensure existing co-author is retained + ( + """Fix broken `Show Source` links on documentation pages (GH-3113) + +Co-authored-by: Elmar Ritsch <35851+elritsch@users.noreply.github.com>""", + """[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) +(cherry picked from commit b9ff498793611d1c6a9b99df464812931a1e2d69) + +Co-authored-by: Author Name +Co-authored-by: Elmar Ritsch <35851+elritsch@users.noreply.github.com>""", + ), + # ensure co-author trailer is not duplicated + ( + """Fix broken `Show Source` links on documentation pages (GH-3113) + +Co-authored-by: Author Name """, + """[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) +(cherry picked from commit b9ff498793611d1c6a9b99df464812931a1e2d69) + +Co-authored-by: Author Name """, + ), + # ensure message is formatted properly when original commit is short + ( + "Fix broken `Show Source` links on documentation pages (GH-3113)", + """[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) +(cherry picked from commit b9ff498793611d1c6a9b99df464812931a1e2d69) + +Co-authored-by: Author Name """, + ), + # ensure message is formatted properly when original commit is long + ( + """Fix broken `Show Source` links on documentation pages (GH-3113) + +The `Show Source` was broken because of a change made in sphinx 1.5.1 +In Sphinx 1.4.9, the sourcename was "index.txt". +In Sphinx 1.5.1+, it is now "index.rst.txt".""", + """[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) + +The `Show Source` was broken because of a change made in sphinx 1.5.1 +In Sphinx 1.4.9, the sourcename was "index.txt". +In Sphinx 1.5.1+, it is now "index.rst.txt". +(cherry picked from commit b9ff498793611d1c6a9b99df464812931a1e2d69) + +Co-authored-by: Author Name """, + ), + # ensure message is formatted properly when original commit is long + # and it has a co-author + ( + """Fix broken `Show Source` links on documentation pages (GH-3113) + +The `Show Source` was broken because of a change made in sphinx 1.5.1 +In Sphinx 1.4.9, the sourcename was "index.txt". +In Sphinx 1.5.1+, it is now "index.rst.txt". + +Co-authored-by: Elmar Ritsch <35851+elritsch@users.noreply.github.com>""", + """[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) + +The `Show Source` was broken because of a change made in sphinx 1.5.1 +In Sphinx 1.4.9, the sourcename was "index.txt". +In Sphinx 1.5.1+, it is now "index.rst.txt". +(cherry picked from commit b9ff498793611d1c6a9b99df464812931a1e2d69) + +Co-authored-by: Author Name +Co-authored-by: Elmar Ritsch <35851+elritsch@users.noreply.github.com>""", + ), + ), +) +def test_get_updated_commit_message_with_trailers(commit_message, expected_commit_message): + cherry_pick_branch = "backport-22a594a-3.6" + commit = "b9ff498793611d1c6a9b99df464812931a1e2d69" + + with mock.patch("cherry_picker.cherry_picker.validate_sha", return_value=True): + cherry_picker = CherryPicker("origin", commit, []) + + with mock.patch( + "cherry_picker.cherry_picker.validate_sha", return_value=True + ), mock.patch.object( + cherry_picker, "get_commit_message", return_value=commit_message + ), mock.patch( + "cherry_picker.cherry_picker.get_author_info_from_short_sha", + return_value="Author Name ", + ): + updated_commit_message = cherry_picker.get_updated_commit_message(cherry_pick_branch) + + assert updated_commit_message == expected_commit_message + + @pytest.mark.parametrize( "input_path", ("/some/path/without/revision", "HEAD:some/non-existent/path") ) From dd779b49d56888e06639cb253ca3d97c06ffb4ef Mon Sep 17 00:00:00 2001 From: jack1142 <6032823+jack1142@users.noreply.github.com> Date: Sun, 8 Aug 2021 20:33:26 +0200 Subject: [PATCH 2/4] Update commit message handling --- cherry_picker/cherry_picker.py | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index bb4b848..d132940 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -296,11 +296,31 @@ def get_updated_commit_message(self, cherry_pick_branch): commit_prefix = "" if self.prefix_commit: commit_prefix = f"[{get_base_branch(cherry_pick_branch)}] " - return f"""{commit_prefix}{self.get_commit_message(self.commit_sha1)} -(cherry picked from commit {self.commit_sha1}) + updated_commit_message = f"{commit_prefix}{self.get_commit_message(self.commit_sha1)}" + cherry_pick_information = f"(cherry picked from commit {self.commit_sha1})\n:" + cmd = [ + "git", + "interpret-trailers", + "--where", + "start", + "--trailer", + f"Co-authored-by: {get_author_info_from_short_sha(self.commit_sha1)}", + "--trailer", + cherry_pick_information, + ] + output = subprocess.check_output(cmd, input=updated_commit_message.encode()) + # Replace the right most-occurence of the "cherry picked from commit" string. + # + # This needs to be done because `git interpret-trailers` adds `:` + # to the end of the trailer. + before, after = output.strip().decode().rsplit(f"\n{cherry_pick_information}", 1) + if not before.endswith("\n"): + # ensure that we still have a newline between cherry pick information + # and commit headline + cherry_pick_information = f"\n{cherry_pick_information}" + updated_commit_message = cherry_pick_information[:-1].join((before, after)) - -Co-authored-by: {get_author_info_from_short_sha(self.commit_sha1)}""" + return updated_commit_message def amend_commit_message(self, cherry_pick_branch): """ prefix the commit message with (X.Y) """ From f8d118e59ca7a4af7848839ed9ec33f1b952b200 Mon Sep 17 00:00:00 2001 From: Jakub Kuczys <6032823+jack1142@users.noreply.github.com> Date: Mon, 15 Aug 2022 14:35:37 +0200 Subject: [PATCH 3/4] Use more informative co-author names in tests --- cherry_picker/test_cherry_picker.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/cherry_picker/test_cherry_picker.py b/cherry_picker/test_cherry_picker.py index b05d607..a63c9d4 100644 --- a/cherry_picker/test_cherry_picker.py +++ b/cherry_picker/test_cherry_picker.py @@ -475,22 +475,22 @@ def test_normalize_short_commit_message(): ( """Fix broken `Show Source` links on documentation pages (GH-3113) -Co-authored-by: Elmar Ritsch <35851+elritsch@users.noreply.github.com>""", +Co-authored-by: PR Co-Author """, """[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) (cherry picked from commit b9ff498793611d1c6a9b99df464812931a1e2d69) -Co-authored-by: Author Name -Co-authored-by: Elmar Ritsch <35851+elritsch@users.noreply.github.com>""", +Co-authored-by: PR Author +Co-authored-by: PR Co-Author """, ), # ensure co-author trailer is not duplicated ( """Fix broken `Show Source` links on documentation pages (GH-3113) -Co-authored-by: Author Name """, +Co-authored-by: PR Author """, """[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) (cherry picked from commit b9ff498793611d1c6a9b99df464812931a1e2d69) -Co-authored-by: Author Name """, +Co-authored-by: PR Author """, ), # ensure message is formatted properly when original commit is short ( @@ -498,7 +498,7 @@ def test_normalize_short_commit_message(): """[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) (cherry picked from commit b9ff498793611d1c6a9b99df464812931a1e2d69) -Co-authored-by: Author Name """, +Co-authored-by: PR Author """, ), # ensure message is formatted properly when original commit is long ( @@ -514,7 +514,7 @@ def test_normalize_short_commit_message(): In Sphinx 1.5.1+, it is now "index.rst.txt". (cherry picked from commit b9ff498793611d1c6a9b99df464812931a1e2d69) -Co-authored-by: Author Name """, +Co-authored-by: PR Author """, ), # ensure message is formatted properly when original commit is long # and it has a co-author @@ -525,7 +525,7 @@ def test_normalize_short_commit_message(): In Sphinx 1.4.9, the sourcename was "index.txt". In Sphinx 1.5.1+, it is now "index.rst.txt". -Co-authored-by: Elmar Ritsch <35851+elritsch@users.noreply.github.com>""", +Co-authored-by: PR Co-Author """, """[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) The `Show Source` was broken because of a change made in sphinx 1.5.1 @@ -533,8 +533,8 @@ def test_normalize_short_commit_message(): In Sphinx 1.5.1+, it is now "index.rst.txt". (cherry picked from commit b9ff498793611d1c6a9b99df464812931a1e2d69) -Co-authored-by: Author Name -Co-authored-by: Elmar Ritsch <35851+elritsch@users.noreply.github.com>""", +Co-authored-by: PR Author +Co-authored-by: PR Co-Author """, ), ), ) @@ -551,7 +551,7 @@ def test_get_updated_commit_message_with_trailers(commit_message, expected_commi cherry_picker, "get_commit_message", return_value=commit_message ), mock.patch( "cherry_picker.cherry_picker.get_author_info_from_short_sha", - return_value="Author Name ", + return_value="PR Author ", ): updated_commit_message = cherry_picker.get_updated_commit_message(cherry_pick_branch) From 2284cc438ac44f9b5c3d6978a6df04ad59c90f40 Mon Sep 17 00:00:00 2001 From: Jakub Kuczys <6032823+jack1142@users.noreply.github.com> Date: Mon, 15 Aug 2022 16:33:12 +0200 Subject: [PATCH 4/4] Add some more comments --- cherry_picker/cherry_picker.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index d132940..b4e18dd 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -293,11 +293,25 @@ def get_exit_message(self, branch): """ def get_updated_commit_message(self, cherry_pick_branch): + """ + Get updated commit message for the cherry-picked commit. + """ + # Get the original commit message and prefix it with the branch name + # if that's enabled. commit_prefix = "" if self.prefix_commit: commit_prefix = f"[{get_base_branch(cherry_pick_branch)}] " updated_commit_message = f"{commit_prefix}{self.get_commit_message(self.commit_sha1)}" + + # Add '(cherry picked from commit ...)' to the message + # and add new Co-authored-by trailer if necessary. cherry_pick_information = f"(cherry picked from commit {self.commit_sha1})\n:" + # Here, we're inserting new Co-authored-by trailer and we *somewhat* + # abuse interpret-trailers by also adding cherry_pick_information which + # is not an actual trailer. + # `--where start` makes it so we insert new trailers *before* the existing + # trailers so cherry-pick information gets added before any of the trailers + # which prevents us from breaking the trailers. cmd = [ "git", "interpret-trailers", @@ -311,8 +325,8 @@ def get_updated_commit_message(self, cherry_pick_branch): output = subprocess.check_output(cmd, input=updated_commit_message.encode()) # Replace the right most-occurence of the "cherry picked from commit" string. # - # This needs to be done because `git interpret-trailers` adds `:` - # to the end of the trailer. + # This needs to be done because `git interpret-trailers` required us to add `:` + # to `cherry_pick_information` when we don't actually want it. before, after = output.strip().decode().rsplit(f"\n{cherry_pick_information}", 1) if not before.endswith("\n"): # ensure that we still have a newline between cherry pick information