diff --git a/cherry_picker/cherry_picker.py b/cherry_picker/cherry_picker.py index bb4b848..b4e18dd 100755 --- a/cherry_picker/cherry_picker.py +++ b/cherry_picker/cherry_picker.py @@ -293,14 +293,48 @@ 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)}] " - 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)}" + + # 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", + "--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` 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 + # 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) """ diff --git a/cherry_picker/test_cherry_picker.py b/cherry_picker/test_cherry_picker.py index 6f733e3..a63c9d4 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: PR Co-Author """, + """[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) +(cherry picked from commit b9ff498793611d1c6a9b99df464812931a1e2d69) + +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: PR Author """, + """[3.6] Fix broken `Show Source` links on documentation pages (GH-3113) +(cherry picked from commit b9ff498793611d1c6a9b99df464812931a1e2d69) + +Co-authored-by: PR Author """, + ), + # 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: PR Author """, + ), + # 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: PR Author """, + ), + # 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: 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 +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: PR Author +Co-authored-by: PR Co-Author """, + ), + ), +) +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="PR Author ", + ): + 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") )