Merge ~racb/git-ubuntu:commit-message-spec into git-ubuntu:master

[Robie Basak]

Make Jenkins happy

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:49d62ab97b7107abfb3dc2a993b50a74cf79215b
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/485/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Build
    SUCCESS: Unit Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/485//rebuild

review: Approve (continuous-integration)

[Bryce Harrington (bryce)]

Looks good, I just ask for a couple comments for the refs fetch and push, and a couple minor inline notes. Otherwise looks solid, some really good refactoring, and I'm looking forward to seeing the new notes feature in practice. I'm still a bit nervous that this will uncover warts in git, but maybe not, and in any case it feels like the right way to go technically.

If you agree with the comment suggestions, please go ahead and land after adding, no need for second round of review.

* Review
  √ python3 setup.py build
  √ python3 setup.py check
  √ ./snap-wrappers/wrappers/git-ubuntu-self-test
    No broken requirements found.
    pip3 found all required dependencies
    pylint passed!
    PASS (syntax) source-package-walker.py
    PASS (compilation) source-package-walker.py
    PASS (syntax) update-repository-alias.py
    PASS (compilation) update-repository-alias.py
    PASS (syntax) import-source-packages.py
    PASS (compilation) import-source-packages.py
    332 passed, 3 xfailed in 187.90 seconds
  ! Per-patch code review
    √ ef26b8a077b6ff3a8c455d38fc838dad4625df00
      + Die wrappers!
      + LGTM
    √ 4bdd58db60bad113b1eb6a87621554f8f8bedef6
      + Pythonic objects FTW
      + LGTM
    √ 69bc94052dbd1793fce7f4f9f32b76cb2c8f3b98
      + Verified the promised skip of test_get_import_commit_msg() is
        indeed dropped in 061371a7 (next patch).
      + LGTM
    √ 061371a77089930b27a261ff0447ba60d517fa82
      + This is the key patch in the set. Moves change info from the
        commit message into the git note.
      + add_changelog_note_to_commit() is added, and has corresponding
        test cases test_add_changelog_note_to_commit() and
        test_double_changelog_note_add_does_not_fail()
      + Uses the new gitubuntu.spec module
      + LGTM
    √ 224684300a41a929492f981c453a3f71d77175d6
      + Largely cleanup/refactor from prior commit. Uses patch_state.
      + LGTM, nice cleanup
    √ 9323899deab7a1d57e3c0ac9f92a47f552602502
      + Verified all call points are updated with correct argument list.
      + LGTM
    √ 37ea80888ea65deaeeb509e3624e2a0f6b92d61c
      + Verified no consumers for _commit_import() other than importer.py, and all
        call points are updated with correct argument list.
      + LGTM
    ! a133d0da5316cedf72c220649ffa8691057a3c2c
      + If I understand the refspecs properly, then the fetch with this
        refspec:
        'refs/notes/commits:refs/notes/usd-import-team/changelog'
        will import the notes from 'refs/notes/commits' in the remote
        repository, into 'refs/notes/usd-import-team/changelog' in the
        local checkout. Then later, there is a repo.git_run() call that
        pushes them back in the opposite direction.
      + The commit message explains this sufficiently, but the
        _main_with_repo() itself is lengthy (250+ lines!) and only
        lightly commented, so this notes pushing/pulling feature may not
        be clear to future readers. This would be a good opportunity to
        add a comment each for the repo.fetch_remote_refspecs() and
        git_run() calls, to define the refspec's being fetched and
        pulled. Hopefully over time this ro...

review: Needs Fixing

[Robie Basak (racb)]

Thank you for the review! Comments addressed and pushed. Just waiting now for CI before landing.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:1c54cd378f08b9d6e28ba5deb3c9612a4114e504
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/486/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Build
    SUCCESS: Unit Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/486//rebuild

review: Approve (continuous-integration)

diff --git a/gitubuntu/clone.py b/gitubuntu/clone.py
index ea8dee9..0eca56e 100644
--- a/gitubuntu/clone.py
+++ b/gitubuntu/clone.py
@@ -114,6 +114,8 @@ def main(
             'pkg/ubuntu/devel branch exist?'
         )
 
+    local_repo.git_run(['config', 'notes.displayRef', 'refs/notes/changelog'])
+
     if os.path.isfile(os.path.join(directory, '.gitignore')):
         logging.warning('A .gitignore file exists in the source '
             'package. This will affect the behavior of git. Consider '
diff --git a/gitubuntu/git_repository.py b/gitubuntu/git_repository.py
index 49bd9b8..ce7ebe8 100644
--- a/gitubuntu/git_repository.py
+++ b/gitubuntu/git_repository.py
@@ -1305,6 +1305,13 @@ class GitUbuntuRepository:
                 remote_name,
                 '+refs/tags/*:refs/tags/%s/*' % remote_name,
             )
+            # The changelog notes are kept at refs/notes/commits on
+            # Launchpad due to LP: #1871838 even though our standard place for
+            # them is refs/notes/changelog.
+            self.raw_repo.remotes.add_fetch(
+                remote_name,
+                '+refs/notes/commits:refs/notes/changelog',
+            )
             if push_url:
                 self.raw_repo.remotes.set_push_url(
                     remote_name,
diff --git a/gitubuntu/importer.py b/gitubuntu/importer.py
index 6d546bc..604b478 100644
--- a/gitubuntu/importer.py
+++ b/gitubuntu/importer.py
@@ -66,6 +66,7 @@ from gitubuntu.source_information import (
     SourceExtractionException,
     launchpad_login_auth,
 )
+import gitubuntu.spec as spec
 from gitubuntu.version import VERSION
 from gitubuntu.versioning import version_compare
 try:
@@ -181,8 +182,16 @@ def _main_with_repo(
 
     if not no_fetch and not reimport:
         try:
+            # This is a normal import run. Fetch in to the "namespaced" local
+            # refs the tags and changelog notes (the branch fetches are handled
+            # elsewhere). The changelog notes are kept at refs/notes/commits on
+            # Launchpad due to LP: #1871838 even though our standard place for
+            # them is refs/notes/changelog.
             repo.fetch_remote_refspecs('pkg',
-                refspecs=['refs/tags/*:refs/tags/%s/*' % namespace],
+                refspecs=[
+                    'refs/tags/*:refs/tags/%s/*' % namespace,
+                    'refs/notes/commits:refs/notes/%s/changelog' % namespace,
+                ],
             )
         except GitUbuntuRepositoryFetchError:
             pass
@@ -365,6 +374,8 @@ def _main_with_repo(
             'push', '--atomic', 'pkg',
             '+refs/heads/%s/*:refs/heads/*' % namespace,
             'refs/tags/%s/*:refs/tags/*' % namespace,
+            # Changelog notes go to refs/notes/commits due to LP: #1871838
+            'refs/notes/%s/changelog:refs/notes/commits' % namespace,
         ])
         # update our reference
         lp_git_repo = lp.git_repositories.getByPath(path=repo_path)
@@ -530,12 +541,7 @@ def get_changelog_for_commit(
 def get_import_commit_msg(
     repo,
     version,
-    target_head_name,
-    namespace,
-    tree_hash,
-    changelog_parent_commits,
-    upload_parent_commit,
-    unapplied_parent_commit,
+    patch_state,
 ):
     """Compose an appropriate commit message
 
@@ -546,62 +552,30 @@ def get_import_commit_msg(
     :param GitUbuntuRepository repo: the repository in which the tree being
         committed can be found
     :param str version: the package version string being committed
-    :param str target_head_name: the nominal name of the branch to which the
-        commit is being made
-    :param str namespace: the namespace name to prefix to the nominal branch
-        name to determine the final branch name
-    :param str tree_hash: the hash of the tree object being committed
-    :param list(str) changelog_parent_commits: any changelog parents to use in
-        the commit
-    :param str upload_parent_commit: the upload parent to use in the commit, or
-        None if there isn't one
-    :param str unapplied_parent_commit: the unapplied parent to use in the
-        commit, or None if this is an applied branch commit
-
+    :param PatchState patch_state: whether the commit is for an unapplied or
+        applied import
+    :rtype: bytes
+    :returns: the commit message to use
     """
-    if unapplied_parent_commit:
-        import_type = 'patches-applied'
-    else:
-        import_type = 'patches-unapplied'
+    import_type = {
+        PatchState.UNAPPLIED: 'patches unapplied',
+        PatchState.APPLIED: 'patches applied',
+    }[patch_state]
 
-    # Do not show importer/ namespace to user
-    _, _, pretty_head_name = target_head_name.partition('%s/' % namespace)
-    changelog_entry = get_changelog_for_commit(
-        repo,
-        tree_hash,
-        changelog_parent_commits
-    )
-
-    msg = (
-        b'Import %s version %b to %b\n\nImported using git-ubuntu import.' % (
-            import_type.encode(),
+    return (
+        b'%b (%b)\n\nImported using git-ubuntu import.' % (
             version.encode(),
-            pretty_head_name.encode(),
+            import_type.encode(),
         )
     )
 
-    if any([changelog_parent_commits, upload_parent_commit, unapplied_parent_commit]):
-        msg += b'\n'
-
-    for changelog_parent_commit in changelog_parent_commits:
-        msg += b'\nChangelog parent: %b' % changelog_parent_commit.encode()
-    if upload_parent_commit is not None:
-        msg += b'\nUpload parent: %b' % upload_parent_commit.encode()
-    if unapplied_parent_commit is not None:
-        msg += b'\nUnapplied parent: %b' % unapplied_parent_commit.encode()
-
-    msg += b'%b' % changelog_entry
-
-    return msg
 
 def _commit_import(
     repo,
     version,
-    target_head_name,
     tree_hash,
     namespace,
     changelog_parent_commits,
-    upload_parent_commit,
     unapplied_parent_commit,
     commit_date,
 ):
@@ -617,19 +591,16 @@ def _commit_import(
          which the source package contents corresponding to @spi should
          be imported.
     :param version str Changelog version
-    :param target_head_name str Git branch name to import to
     :param namespace str Namespace under which relevant refs can be
          found.
     :param list(str) changelog_parent_commits Git commit hashes of changelog
          parents.
-    :param upload_parent_commit str Git commit hash of
-         upload parent. Should be None if patches-applied import.
     :param unapplied_parent_commit str Git commit hash of unapplied
          parent. Should be None if patches-unapplied import.
     :param str commit_date: committer date to use for the git commit metadata
 
-    :rtype str
-    :returns Hash of created commit
+    :rtype: pygit2.Commit
+    :returns: created commit object
     """
     parents = []
 
@@ -637,75 +608,27 @@ def _commit_import(
         pygit2.Oid(hex=changelog_parent_commit)
         for changelog_parent_commit in changelog_parent_commits
     )
-    if upload_parent_commit is not None:
-        parents.append(pygit2.Oid(hex=upload_parent_commit))
     if unapplied_parent_commit is not None:
         parents.append(pygit2.Oid(hex=unapplied_parent_commit))
 
+    if unapplied_parent_commit:
+        patch_state = PatchState.APPLIED
+    else:
+        patch_state = PatchState.UNAPPLIED
+
     commit_hash = repo.commit_source_tree(
         tree=pygit2.Oid(hex=tree_hash),
         parents=parents,
         log_message=get_import_commit_msg(
             repo=repo,
             version=version,
-            target_head_name=target_head_name,
-            namespace=namespace,
-            tree_hash=tree_hash,
-            changelog_parent_commits=changelog_parent_commits,
-            upload_parent_commit=upload_parent_commit,
-            unapplied_parent_commit=unapplied_parent_commit,
+            patch_state=patch_state,
         ),
         commit_date=commit_date,
     )
 
-    return str(commit_hash)
+    return repo.raw_repo.get(str(commit_hash))
 
-def commit_unapplied_patches_import(
-    repo,
-    version,
-    head_name,
-    import_tree_hash,
-    namespace,
-    changelog_parent_commits,
-    upload_parent_commit,
-    commit_date,
-):
-    return _commit_import(
-        repo,
-        version,
-        head_name,
-        import_tree_hash,
-        namespace,
-        changelog_parent_commits,
-        upload_parent_commit,
-        # unapplied trees do not have a distinct unapplied parent
-        None,
-        commit_date,
-    )
-
-def commit_applied_patches_import(
-    repo,
-    version,
-    head_name,
-    import_tree_hash,
-    namespace,
-    changelog_parent_commits,
-    unapplied_parent_commit,
-    commit_date,
-):
-    return _commit_import(
-        repo,
-        version,
-        head_name,
-        import_tree_hash,
-        namespace,
-        changelog_parent_commits,
-        # uploads will be unapplied trees currently, so applied trees
-        # can not have them as direct parents
-        None,
-        unapplied_parent_commit,
-        commit_date,
-    )
 
 def import_dsc(repo, dsc, namespace, version, dist):
     """Imports the dsc file to importer/{debian,ubuntu}/dsc
@@ -1526,6 +1449,61 @@ def validate_upload_tag(
     return True
 
 
+def add_changelog_note_to_commit(
+    repo,
+    namespace,
+    commit,
+    changelog_parents,
+):
+    """Compute and add a changelog note to the given commit.
+
+    For user convenience, changelog notes allow for "git log" and similar tools
+    to display the debian/changelog entry associated with a
+    importer-synthesized commit. See LP #1633114 for details.
+
+    If a changelog note already exists for the given commit, then return
+    without doing anything.
+
+    :param gitubuntu.GitUbuntuRepository repo: the repository that is the
+        target of this import.
+    :param str namespace: the namespace under which relevant refs can be found.
+    :param pygit2.Commit commit: the commit for which the changelog note should
+        be calculated and added.
+    :param list(str) changelog_parents: the list of commit hashes that are the
+        parents of the provided commit, from which the changelog note may be
+        calculated.
+    :returns: None
+    """
+    # If the note already exists, return without doing anything.
+    try:
+        repo.raw_repo.lookup_note(
+            str(commit.id),
+            'refs/notes/%s/changelog' % namespace,
+        )
+    except KeyError:
+        pass
+    else:
+        return
+
+    # The note doesn't exist, so figure out what it should be and add it.
+    changelog_summary = get_changelog_for_commit(
+        repo=repo,
+        tree_hash=str(commit.peel(pygit2.Tree).id),
+        changelog_parent_commits=changelog_parents,
+    )
+    changelog_signature = pygit2.Signature(
+        spec.SYNTHESIZED_COMMITTER_NAME,
+        spec.SYNTHESIZED_COMMITTER_EMAIL,
+    )
+    repo.raw_repo.create_note(
+        changelog_summary.decode(),
+        changelog_signature,
+        changelog_signature,
+        str(commit.id),
+        'refs/notes/%s/changelog' % namespace,
+    )
+
+
 def find_or_create_unapplied_commit(
     repo,
     version,
@@ -1533,7 +1511,6 @@ def find_or_create_unapplied_commit(
     import_tree,
     parent_overrides,
     commit_date,
-    head_name,
 ):
     """Return the commit to be used for an unapplied import
 
@@ -1559,9 +1536,6 @@ def find_or_create_unapplied_commit(
     :param dict parent_overrides: see parse_parentfile()
     :param str commit_date: committer date to use for the git commit metadata.
         If None, use the current date.
-    :param str head_name: git branch name that is the target of this import.
-        This is currently used in the generated commit message, but will go
-        away when the commit messages are overhauled to the new spec.
     :rtype: tuple(pygit2.Commit, pygit2.Reference or None)
     :returns: the commit that has been found or created, and the import tag
         corresponding to an existing import if it had previously existed.
@@ -1596,21 +1570,26 @@ def find_or_create_unapplied_commit(
             return upload_tag.peel(), None
 
     # Create a commit
-    commit = repo.raw_repo.get(commit_unapplied_patches_import(
+    commit = _commit_import(
         repo=repo,
         version=version,
-        head_name=head_name,
-        import_tree_hash=str(import_tree.id),
+        tree_hash=str(import_tree.id),
         namespace=namespace,
         changelog_parent_commits=changelog_parents,
-        upload_parent_commit=None,
+        # unapplied trees do not have a distinct unapplied parent
+        unapplied_parent_commit=None,
         commit_date=commit_date,
-    ))
+    )
     logging.debug(
-        "Committed patches-unapplied import of %s as %s in %s",
+        "Committed patches-unapplied import of %s as %s",
         version,
         str(commit.id),
-        head_name.partition('%s/' % namespace)[2],
+    )
+    add_changelog_note_to_commit(
+        repo=repo,
+        namespace=namespace,
+        commit=commit,
+        changelog_parents=changelog_parents,
     )
     return commit, None
 
@@ -1686,7 +1665,6 @@ def import_unapplied_dsc(
             import_tree=repo.raw_repo.get(unapplied_import_tree_hash),
             parent_overrides=parent_overrides,
             commit_date=commit_date,
-            head_name=head_name,
         )
     except ParentOverrideError as e:
         logging.error("%s" % e)
@@ -1917,16 +1895,15 @@ def import_applied_dsc(
             return
         raise
 
-    commit_hash = commit_applied_patches_import(
+    commit_hash = str(_commit_import(
         repo=repo,
         version=version,
-        head_name=head_name,
-        import_tree_hash=applied_import_tree_hash,
+        tree_hash=applied_import_tree_hash,
         namespace=namespace,
         changelog_parent_commits=applied_changelog_parent_commits,
         unapplied_parent_commit=str(unapplied_parent_commit),
         commit_date=commit_date,
-    )
+    ).id)
     logging.debug(
         "Committed patches-applied import of %s as %s in %s",
         version,
diff --git a/gitubuntu/importer_tag_test.py b/gitubuntu/importer_tag_test.py
index a216e73..3573d9b 100644
--- a/gitubuntu/importer_tag_test.py
+++ b/gitubuntu/importer_tag_test.py
@@ -205,6 +205,7 @@ from gitubuntu.importer_test import MockSPI, patch_get_commit_environment
                 'refs/tags/importer/upstream/ubuntu/1.gz',
                 'refs/heads/importer/importer/ubuntu/dsc',
                 'refs/heads/importer/importer/ubuntu/pristine-tar',
+                'refs/notes/importer/changelog',
             ],
             # validation_repo_delta:
             {
@@ -255,6 +256,7 @@ from gitubuntu.importer_test import MockSPI, patch_get_commit_environment
                 'refs/tags/importer/upstream/ubuntu/1.gz',
                 'refs/heads/importer/importer/ubuntu/dsc',
                 'refs/heads/importer/importer/ubuntu/pristine-tar',
+                'refs/notes/importer/changelog',
             ],
             # validation_repo_delta:
             {
@@ -335,6 +337,7 @@ from gitubuntu.importer_test import MockSPI, patch_get_commit_environment
                 'refs/tags/importer/upstream/ubuntu/1.gz',
                 'refs/heads/importer/importer/ubuntu/dsc',
                 'refs/heads/importer/importer/ubuntu/pristine-tar',
+                'refs/notes/importer/changelog',
             ],
             # validation_repo_delta:
             {
@@ -372,6 +375,7 @@ from gitubuntu.importer_test import MockSPI, patch_get_commit_environment
                 'refs/tags/importer/upstream/ubuntu/1.gz',
                 'refs/heads/importer/importer/ubuntu/dsc',
                 'refs/heads/importer/importer/ubuntu/pristine-tar',
+                'refs/notes/importer/changelog',
             ],
             # validation_repo_delta:
             {
diff --git a/gitubuntu/importer_test.py b/gitubuntu/importer_test.py
index 54f1f2b..f94150d 100644
--- a/gitubuntu/importer_test.py
+++ b/gitubuntu/importer_test.py
@@ -73,58 +73,21 @@ def test_get_import_tag_msg():
 
 
 @pytest.mark.parametrize(
-    'head_name_value, changelog_parent_commits, upload_parent_commit, unapplied_parent_commit, expected',
+    'patch_state, expected',
     [
         (
-            'importer/ubuntu/trusty',
-            [],
-            None,
-            None,
-            b'Import patches-unapplied version 1-1 to ubuntu/trusty\n\nImported using git-ubuntu import.',
-        ),
-        (
-            'importer/ubuntu/trusty',
-            ['123456'],
-            None,
-            None,
-            b'Import patches-unapplied version 1-1 to ubuntu/trusty\n\nImported using git-ubuntu import.\n\nChangelog parent: 123456',
-        ),
-        (
-            'importer/ubuntu/trusty',
-            [],
-            '123456',
-            None,
-            b'Import patches-unapplied version 1-1 to ubuntu/trusty\n\nImported using git-ubuntu import.\n\nUpload parent: 123456',
-        ),
-        (
-            'importer/ubuntu/trusty',
-            ['123456', '234567'],
-            None,
-            None,
-            b'Import patches-unapplied version 1-1 to ubuntu/trusty\n\nImported using git-ubuntu import.\n\nChangelog parent: 123456\nChangelog parent: 234567',
-        ),
-        (
-            'importer/ubuntu/trusty',
-            [],
-            None,
-            '123456',
-            b'Import patches-applied version 1-1 to ubuntu/trusty\n\nImported using git-ubuntu import.\n\nUnapplied parent: 123456',
+            PatchState.UNAPPLIED,
+            b'1-1 (patches unapplied)\n\nImported using git-ubuntu import.',
         ),
         (
-            'importer/ubuntu/trusty',
-            ['123456'],
-            '789123',
-            '456789',
-            b'Import patches-applied version 1-1 to ubuntu/trusty\n\nImported using git-ubuntu import.\n\nChangelog parent: 123456\nUpload parent: 789123\nUnapplied parent: 456789'
+            PatchState.APPLIED,
+            b'1-1 (patches applied)\n\nImported using git-ubuntu import.',
         ),
     ],
 )
 def test_get_import_commit_msg(
     repo,
-    head_name_value,
-    changelog_parent_commits,
-    upload_parent_commit,
-    unapplied_parent_commit,
+    patch_state,
     expected,
 ):
     """Test that get_import_commit_msg is generally correct
@@ -138,17 +101,10 @@ def test_get_import_commit_msg(
 
     :param GitUbuntuRepository repo: fixture providing a temporary
         GitUbuntuRepository instance to use
-    :param str head_name_value: passed through to get_import_commit_msg as
-        target_head_name
-    :param list(str) changelog_parent_commits: passed through to
-        get_import_commit_msg
-    :param str upload_parent_commit: passed through to get_import_commit_msg
-    :param str unapplied_parent_commit: passed through to get_import_commit_msg
+    :param PatchState patch_state: whether the commit is for an unapplied or
+        applied import, as passed through to the function under test
     :param bytes expected: the expected return value from get_import_commit_msg
     """
-    target.get_changelog_for_commit = Mock()
-    target.get_changelog_for_commit.return_value = b''
-
     publish_spec = source_builder.SourceSpec()
     publish_source = source_builder.Source(publish_spec)
 
@@ -160,12 +116,7 @@ def test_get_import_commit_msg(
     assert target.get_import_commit_msg(
         repo,
         publish_spec.version,
-        head_name_value,
-        'importer',
-        publish_tree_hash,
-        changelog_parent_commits,
-        upload_parent_commit,
-        unapplied_parent_commit,
+        patch_state,
     ) == expected
 
 
@@ -862,6 +813,81 @@ def test_validate_upload_tag(
     assert result == expected_result
 
 
+def test_add_changelog_note_to_commit(repo):
+    """add_changelog_note_to_commit should add the expected note"""
+    repo_builder.Repo(
+        commits=[
+            repo_builder.Commit.from_spec(name='1-1'),
+            repo_builder.Commit.from_spec(
+                name='1-2',
+                changelog_versions=['1-1', '1-2'],
+                parents=[Placeholder('1-1')],
+            ),
+        ],
+        tags={'1-1': Placeholder('1-1'), '1-2': Placeholder('1-2')},
+    ).write(repo.raw_repo)
+
+    parent_commit_ref = repo.raw_repo.lookup_reference('refs/tags/1-1')
+    parent_commit = parent_commit_ref.peel(pygit2.Commit)
+
+    child_commit_ref = repo.raw_repo.lookup_reference('refs/tags/1-2')
+    child_commit = child_commit_ref.peel(pygit2.Commit)
+
+    target.add_changelog_note_to_commit(
+        repo=repo,
+        namespace='importer',
+        commit=child_commit,
+        changelog_parents=[str(parent_commit.id)],
+    )
+
+    note = repo.raw_repo.lookup_note(
+        str(child_commit.id),
+        'refs/notes/importer/changelog',
+    )
+    assert note.message == (
+        "\n\nNew changelog entries:\n"
+        "    * Test build from source_builder.\n"
+    )
+
+
+def test_double_changelog_note_add_does_not_fail(repo):
+    """add_changelog_note_to_commit shouldn't fail if a note already exists"""
+    repo_builder.Repo(
+        commits=[
+            repo_builder.Commit.from_spec(name='1-1'),
+            repo_builder.Commit.from_spec(
+                name='1-2',
+                changelog_versions=['1-1', '1-2'],
+                parents=[Placeholder('1-1')],
+            ),
+        ],
+        tags={'1-1': Placeholder('1-1'), '1-2': Placeholder('1-2')},
+    ).write(repo.raw_repo)
+
+    parent_commit_ref = repo.raw_repo.lookup_reference('refs/tags/1-1')
+    parent_commit = parent_commit_ref.peel(pygit2.Commit)
+
+    child_commit_ref = repo.raw_repo.lookup_reference('refs/tags/1-2')
+    child_commit = child_commit_ref.peel(pygit2.Commit)
+
+    # The first call should create the note (tested elsewhere)
+    target.add_changelog_note_to_commit(
+        repo=repo,
+        namespace='importer',
+        commit=child_commit,
+        changelog_parents=[str(parent_commit.id)],
+    )
+
+    # The second call should not fail even though the note already exists (as
+    # created by the previous call)
+    target.add_changelog_note_to_commit(
+        repo=repo,
+        namespace='importer',
+        commit=child_commit,
+        changelog_parents=[str(parent_commit.id)],
+    )
+
+
 def patch_get_commit_environment(repo):
     """Patch a repository to use constant metadata