Handle deletion of uncommitted news fragments

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]>

Author: abompard

src/towncrier/_git.py

@@ -1,12 +1,24 @@
 # Copyright (c) Amber Brown, 2015
 # See LICENSE for details.
 
-from subprocess import call
-
 import os
+import sys
+from subprocess import call, check_output, CalledProcessError
+
 import click
 
 
+def run(args, **kwargs):
+    # Use UTF-8 both when sys.stdout does not have .encoding (Python 2.7) and
+    # when the attribute is present but set to None (explicitly piped output
+    # and also some CI such as GitHub Actions).
+    encoding = getattr(sys.stdout, "encoding", None)
+    if encoding is None:
+        encoding = "utf8"
+
+    return check_output(args, **kwargs).decode(encoding).strip()
+
+
 def remove_files(fragment_filenames, answer_yes):
     if not fragment_filenames:
         return
@@ -16,11 +28,23 @@ def remove_files(fragment_filenames, answer_yes):
     else:
         click.echo("I want to remove the following files:")
 
-    for filename in fragment_filenames:
+    for filename in sorted(fragment_filenames):
         click.echo(filename)
 
+    # Filter out files that are unknown to git
+    try:
+        known_fragments = run(
+            ["git", "ls-files"] + fragment_filenames
+        ).split("\n")
+    except CalledProcessError:
+        known_fragments = []
+
     if answer_yes or click.confirm("Is it okay if I remove those files?", default=True):
-        call(["git", "rm", "--quiet"] + fragment_filenames)
+        call(["git", "rm", "--quiet", "--force"] + known_fragments)
+        known_fragments_full = [os.path.abspath(f) for f in known_fragments]
+        unknown_fragments = set(fragment_filenames) - set(known_fragments_full)
+        for unknown_fragment in unknown_fragments:
+            os.remove(unknown_fragment)
 
 
 def stage_newsfile(directory, filename):

src/towncrier/check.py

@@ -8,15 +8,11 @@
 
 import click
 
-from subprocess import CalledProcessError, check_output, STDOUT
+from subprocess import CalledProcessError, STDOUT
 
 from ._settings import load_config_from_options
 from ._builder import find_fragments
-
-
-def _run(args, **kwargs):
-    kwargs["stderr"] = STDOUT
-    return check_output(args, **kwargs)
+from ._git import run
 
 
 @click.command(name="check")
@@ -31,20 +27,10 @@ def __main(comparewith, directory, config):
 
     base_directory, config = load_config_from_options(directory, config)
 
-    # Use UTF-8 both when sys.stdout does not have .encoding (Python 2.7) and
-    # when the attribute is present but set to None (explicitly piped output
-    # and also some CI such as GitHub Actions).
-    encoding = getattr(sys.stdout, "encoding", None)
-    if encoding is None:
-        encoding = "utf8"
-
     try:
-        files_changed = (
-            _run(
-                ["git", "diff", "--name-only", comparewith + "..."], cwd=base_directory
-            )
-            .decode(encoding)
-            .strip()
+        files_changed = run(
+            ["git", "diff", "--name-only", comparewith + "..."],
+            cwd=base_directory, stderr=STDOUT
         )
     except CalledProcessError as e:
         click.echo("git produced output while failing:")

src/towncrier/newsfragments/357.feature.rst

@@ -0,0 +1 @@
+Handle deletion of uncommitted news fragments

src/towncrier/test/test_build.py

@@ -836,3 +836,81 @@ def test_start_string(self):
         """)
 
         self.assertEqual(expected_output, output)
+
+    def test_uncommitted_files(self):
+        runner = CliRunner()
+
+        with runner.isolated_filesystem():
+            setup_simple_project()
+            with open("foo/newsfragments/123.feature", "w") as f:
+                f.write("Adds levitation")
+            with open("foo/newsfragments/124.feature", "w") as f:
+                f.write("Extends levitation")
+            with open("foo/newsfragments/125.feature", "w") as f:
+                f.write("Baz levitation")
+            with open("foo/newsfragments/126.feature", "w") as f:
+                f.write("Fix (literal) crash")
+
+            call(["git", "init"])
+            call(["git", "config", "user.name", "user"])
+            call(["git", "config", "user.email", "[email protected]"])
+            # 123 is committed, 124 is modified, 125 is just added, 126 is unknown
+            call([
+                "git", "add",
+                "foo/newsfragments/123.feature",
+                "foo/newsfragments/124.feature"
+            ])
+            call(["git", "commit", "-m", "Initial Commit"])
+            with open("foo/newsfragments/124.feature", "a") as f:
+                f.write(" 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(
+                        "foo/newsfragments/{}.feature".format(fragment)
+                    )
+                )
+
+            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 (#123)
+                    - Extends levitation for an hour (#124)
+                    - Baz levitation (#125)
+                    - Fix (literal) crash (#126)
+                    """
+                ),
+            )
+            self.assertEqual(
+                result.output,
+                dedent(
+                    """\
+                    Loading template...
+                    Finding news fragments...
+                    Rendering news fragments...
+                    Writing to newsfile...
+                    Staging newsfile...
+                    Removing news fragments...
+                    Removing the following files:
+                    {cwd}/foo/newsfragments/123.feature
+                    {cwd}/foo/newsfragments/124.feature
+                    {cwd}/foo/newsfragments/125.feature
+                    {cwd}/foo/newsfragments/126.feature
+                    Done!
+                    """.format(cwd=os.getcwd())
+                ),
+            )