Refuse to overwrite output C/C++ files that probably were not created by Cython (GH-4178)

This is a common gotcha for new users who name their .pyx file after the C file that they want to wrap.
Closes https://github.com/cython/cython/issues/4177

Cython/Compiler/ModuleNode.py

@@ -27,9 +27,9 @@ from . import TypeSlots
 from . import PyrexTypes
 from . import Pythran
 
-from .Errors import error, warning
+from .Errors import error, warning, CompileError
 from .PyrexTypes import py_object_type
-from ..Utils import open_new_file, replace_suffix, decode_filename, build_hex_version
+from ..Utils import open_new_file, replace_suffix, decode_filename, build_hex_version, is_cython_generated_file
 from .Code import UtilityCode, IncludeCode, TempitaUtilityCode
 from .StringEncoding import EncodedString, encoded_string_or_bytes_literal
 from .Pythran import has_np_pythran
@@ -401,6 +401,13 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
         i_code.dedent()
 
     def generate_c_code(self, env, options, result):
+        # Check for a common gotcha for new users: naming your .pyx file after the .c file you want to wrap
+        if not is_cython_generated_file(result.c_file, allow_failed=True, if_not_found=True):
+            # Raising a fatal CompileError instead of calling error() to prevent castrating an existing file.
+            raise CompileError(
+                self.pos, 'The output file already exists and does not look like it was generated by Cython: "%s"' %
+                os.path.basename(result.c_file))
+
         modules = self.referenced_modules
 
         if Options.annotate or options.annotate:

Cython/Coverage.py

@@ -14,7 +14,7 @@ from collections import defaultdict
 from coverage.plugin import CoveragePlugin, FileTracer, FileReporter  # requires coverage.py 4.0+
 from coverage.files import canonical_filename
 
-from .Utils import find_root_package_dir, is_package_dir, open_source_file
+from .Utils import find_root_package_dir, is_package_dir, is_cython_generated_file, open_source_file
 
 
 from . import __version__
@@ -165,12 +165,11 @@ class Plugin(CoveragePlugin):
             py_source_file = os.path.splitext(c_file)[0] + '.py'
             if not os.path.exists(py_source_file):
                 py_source_file = None
-
-            try:
-                with open(c_file, 'rb') as f:
-                    if b'/* Generated by Cython ' not in f.read(30):
-                        return None, None  # not a Cython file
-            except (IOError, OSError):
+            if not is_cython_generated_file(c_file, if_not_found=False):
+                if py_source_file and os.path.exists(c_file):
+                    # if we did not generate the C file,
+                    # then we probably also shouldn't care about the .py file.
+                    py_source_file = None
                 c_file = None
 
         return c_file, py_source_file

Cython/Utils.py

@@ -95,6 +95,9 @@ def castrate_file(path, st):
     #  failed compilation.
     #  Also sets access and modification times back to
     #  those specified by st (a stat struct).
+    if not is_cython_generated_file(path, allow_failed=True, if_not_found=False):
+        return
+
     try:
         f = open_new_file(path)
     except EnvironmentError:
@@ -107,6 +110,30 @@ def castrate_file(path, st):
             os.utime(path, (st.st_atime, st.st_mtime-1))
 
 
+def is_cython_generated_file(path, allow_failed=False, if_not_found=True):
+    failure_marker = b"#error Do not use this file, it is the result of a failed Cython compilation."
+    file_content = None
+    if os.path.exists(path):
+        try:
+            with open(path, "rb") as f:
+                file_content = f.read(len(failure_marker))
+        except (OSError, IOError):
+            pass  # Probably just doesn't exist any more
+
+    if file_content is None:
+        # file does not exist (yet)
+        return if_not_found
+
+    return (
+        # Cython C file?
+        file_content.startswith(b"/* Generated by Cython ") or
+        # Cython output file after previous failures?
+        (allow_failed and file_content == failure_marker) or
+        # Let's allow overwriting empty files as well. They might have resulted from previous failures.
+        not file_content
+    )
+
+
 def file_newer_than(path, time):
     ftime = modification_time(path)
     return ftime > time

tests/run/existing_output_files.srctree

+PYTHON test.py
+
+######## test.py ########
+
+from __future__ import print_function
+
+import os.path
+from Cython.Utils import is_cython_generated_file
+from Cython.Compiler.Errors import CompileError
+from Cython.Build.Dependencies import cythonize
+
+# Make sure the source files are newer than the .c files, so that cythonize() regenerates them.
+files = {}
+for source_file in sorted(os.listdir(os.getcwd())):
+    if 'module' in source_file and not source_file.endswith(".c"):
+        c_file = files[source_file] = os.path.splitext(source_file)[0] + ".c"
+        os.utime(source_file, None)
+        assert not os.path.exists(c_file) or os.path.getmtime(source_file) >= os.path.getmtime(c_file)
+
+for source_file, c_file in files.items():
+    print("Testing:", source_file, c_file)
+    assert is_cython_generated_file(c_file, allow_failed=True, if_not_found=True)
+    cythonize(source_file, language_level=3)
+    assert is_cython_generated_file(c_file, if_not_found=False)
+    assert os.path.getmtime(source_file) <= os.path.getmtime(c_file)
+
+# But overwriting an unknown file should fail, even when requested multiple times.
+for source_file in [
+        "refuse_to_overwrite.pyx",
+        "refuse_to_overwrite.pyx",
+        "refuse_to_overwrite.pyx",
+        #
+        "refuse_to_overwrite.py",
+        "refuse_to_overwrite.py",
+        "refuse_to_overwrite.py",
+        #
+        "compile_failure.pyx",
+        "compile_failure.pyx",
+        "compile_failure.pyx",
+]:
+    os.utime(source_file, None)
+    c_file = os.path.splitext(source_file)[0] + ".c"
+    assert not is_cython_generated_file(c_file)
+    try:
+        print("Testing:", source_file)
+        cythonize(source_file, language_level=3)
+    except CompileError:
+        print("REFUSED to overwrite %s, OK" % c_file)
+        assert not is_cython_generated_file(c_file)
+    else:
+        assert False, "FAILURE: Existing output file was overwritten for source file %s" % source_file
+
+
+######## pymodule.c ########
+#error Do not use this file, it is the result of a failed Cython compilation.
+
+######## pymodule.py ########
+"""
+Overwriting a failed .py file result works
+"""
+
+######## cymodule.c ########
+#error Do not use this file, it is the result of a failed Cython compilation.
+
+######## cymodule.pyx ########
+"""
+Overwriting a failed .pyx file result works
+"""
+
+######## overwritten_cymodule.c ########
+/* Generated by Cython 0.8.15 */
+
+######## overwritten_cymodule.pyx ########
+"""
+Overwriting an outdated .c file works
+"""
+
+
+######## new_cymodule.pyx ########
+"""
+Creating a new .c file works
+"""
+
+######## new_pymodule.py ########
+"""
+Creating a new .c file works
+"""
+
+
+######## refuse_to_overwrite.c ########
+static int external_function(int x) {
+    return x + 1;
+}
+
+######## refuse_to_overwrite.py ########
+"""
+Do not overwrite an unknown output file
+"""
+
+######## refuse_to_overwrite.pyx ########
+"""
+Do not overwrite an unknown output file
+"""
+
+
+######## compile_failure.c ########
+static int external_function(int x) {
+    return x + 1;
+}
+
+######## compile_failure.pyx ########
+"""
+Do not overwrite an unknown output file even on compile failures.
+"""
+
+Not Python syntax!