Handle deletion of uncommitted news fragments (#357)

Before this commit, all the news fragments needed to be committed into
git, or the fragments removal after building the news file would crash.

In my workflow, I add missing fragments before building the news file
because I'm extracting author names from the git log, and towncrier
crashes at the end of the build process.

Signed-off-by: Aurélien Bompard <[email protected]>
Co-authored-by: Adi Roiban <[email protected]>

docs/cli.rst

@@ -28,8 +28,9 @@ If there are no news fragments (including an empty fragments directory or a
 non-existent directory), a notice of "no significant changes" will be added to
 the news file.
 
-By default, the processed news fragments are removed using ``git``, which will
-also remove the fragments directory if now empty.
+By default, the processed news fragments are removed. For any fragments
+committed in your git repository, git rm will be used (which will also remove
+the fragments directory if now empty).
 
 .. option:: --draft
 

src/towncrier/_git.py

@@ -5,12 +5,27 @@
 
 import os
 
-from subprocess import STDOUT, call, check_output
+from subprocess import STDOUT, CalledProcessError, call, check_output
 
 
 def remove_files(fragment_filenames: list[str]) -> None:
-    if fragment_filenames:
-        call(["git", "rm", "--quiet"] + fragment_filenames)
+    if not fragment_filenames:
+        return
+
+    # Filter out files that are unknown to git
+    try:
+        git_fragments = check_output(
+            ["git", "ls-files"] + fragment_filenames, encoding="utf-8"
+        ).split("\n")
+    except CalledProcessError:
+        # we may not be in a git repository
+        git_fragments = []
+
+    git_fragments = [os.path.abspath(f) for f in git_fragments if os.path.isfile(f)]
+    call(["git", "rm", "--quiet", "--force"] + git_fragments)
+    unknown_fragments = set(fragment_filenames) - set(git_fragments)
+    for unknown_fragment in unknown_fragments:
+        os.remove(unknown_fragment)
 
 
 def stage_newsfile(directory: str, filename: str) -> None:

src/towncrier/newsfragments/357.feature.rst

@@ -0,0 +1 @@
+``towncrier build`` now handles removing news fragments which are not part of the git repository. For example, uncommitted or unstaged files.

src/towncrier/test/test_build.py

@@ -6,6 +6,7 @@
 
 from datetime import date
 from pathlib import Path
+from subprocess import call
 from textwrap import dedent
 from unittest.mock import patch
 
@@ -747,8 +748,8 @@ def do_build_once_with(version, fragment_file, fragment):
                     "--yes",
                 ],
             )
-            # not git repository, manually remove fragment file
-            Path(f"newsfragments/{fragment_file}").unlink()
+            # Fragment files unknown to git are removed even without a git repo
+            assert not Path(f"newsfragments/{fragment_file}").exists()
             return result
 
         results = []
@@ -845,8 +846,8 @@ def do_build_once_with(version, fragment_file, fragment):
                 ],
                 catch_exceptions=False,
             )
-            # not git repository, manually remove fragment file
-            Path(f"newsfragments/{fragment_file}").unlink()
+            # Fragment files unknown to git are removed even without a git repo
+            assert not Path(f"newsfragments/{fragment_file}").exists()
             return result
 
         results = []
@@ -1530,3 +1531,55 @@ def test_orphans_in_non_showcontent_markdown(self, runner):
 
         self.assertEqual(0, result.exit_code, result.output)
         self.assertEqual(expected_output, result.output)
+
+    @with_git_project()
+    def test_uncommitted_files(self, runner, commit):
+        """
+        At build time, it will delete any fragment file regardless of its stage,
+        included files that are not part of the git reporsitory,
+        or are just staged or modified.
+        """
+        # 123 is committed, 124 is modified, 125 is just added, 126 is unknown
+
+        with open("foo/newsfragments/123.feature", "w") as f:
+            f.write("Adds levitation. File committed.")
+        with open("foo/newsfragments/124.feature", "w") as f:
+            f.write("Extends levitation. File modified in Git.")
+
+        commit()
+
+        with open("foo/newsfragments/125.feature", "w") as f:
+            f.write("Baz levitation. Staged file.")
+        with open("foo/newsfragments/126.feature", "w") as f:
+            f.write("Fix (literal) crash. File unknown to Git.")
+
+        with open("foo/newsfragments/124.feature", "a") as f:
+            f.write(" Extended for an hour.")
+        call(["git", "add", "foo/newsfragments/125.feature"])
+
+        result = runner.invoke(_main, ["--date", "01-01-2001", "--yes"])
+
+        self.assertEqual(0, result.exit_code)
+        for fragment in ("123", "124", "125", "126"):
+            self.assertFalse(os.path.isfile(f"foo/newsfragments/{fragment}.feature"))
+
+        path = "NEWS.rst"
+        self.assertTrue(os.path.isfile(path))
+        news_contents = open(path).read()
+        self.assertEqual(
+            news_contents,
+            dedent(
+                """\
+                Foo 1.2.3 (01-01-2001)
+                ======================
+
+                Features
+                --------
+
+                - Adds levitation. File committed. (#123)
+                - Extends levitation. File modified in Git. Extended for an hour. (#124)
+                - Baz levitation. Staged file. (#125)
+                - Fix (literal) crash. File unknown to Git. (#126)
+                """
+            ),
+        )