Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proof of concept: gh-64595 fix #37

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions Lib/test/test_clinic.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ def test_eol(self):
# and since you really already had one,
# the last line of the block got corrupted.
raw = "/*[clinic]\nfoo\n[clinic]*/"
cooked = self.clinic.parse(raw).splitlines()
_, cooked = self.clinic.parse(raw)
cooked = cooked.splitlines()
end_line = cooked[2].rstrip()
# this test is redundant, it's just here explicitly to catch
# the regression test so we don't forget what it looked like
Expand Down Expand Up @@ -131,7 +132,7 @@ def test_parse_with_body_prefix(self):
//module test
//[clinic stop]
""").strip()
out = cl.parse(raw)
_, out = cl.parse(raw)
expected = dedent("""
//[clinic start]
//module test
Expand Down Expand Up @@ -198,7 +199,7 @@ def test_directive_output_print(self):
output print 'I told you once.'
[clinic start generated code]*/
""")
out = self.clinic.parse(raw)
_, out = self.clinic.parse(raw)
# The generated output will differ for every run, but we can check that
# it starts with the clinic block, we check that it contains all the
# expected fields, and we check that it contains the checksum line.
Expand Down Expand Up @@ -397,7 +398,7 @@ def test_dest_buffer_not_empty_at_eof(self):
[clinic start generated code]*/
""")
with support.captured_stdout() as stdout:
generated = self.clinic.parse(block)
_, generated = self.clinic.parse(block)
self.assertIn(expected_warning, stdout.getvalue())
self.assertEqual(generated, expected_generated)

Expand Down Expand Up @@ -435,7 +436,7 @@ def test_directive_set_prefix(self):
dump buffer
[clinic start generated code]*/
""")
generated = self.clinic.parse(block)
_, generated = self.clinic.parse(block)
expected_docstring_prototype = "// PyDoc_VAR(fn__doc__);"
self.assertIn(expected_docstring_prototype, generated)

Expand All @@ -455,7 +456,7 @@ def test_directive_set_suffix(self):
dump buffer
[clinic start generated code]*/
""")
generated = self.clinic.parse(block)
_, generated = self.clinic.parse(block)
expected_docstring_prototype = "PyDoc_VAR(fn__doc__); // test"
self.assertIn(expected_docstring_prototype, generated)

Expand All @@ -476,7 +477,7 @@ def test_directive_set_prefix_and_suffix(self):
dump buffer
[clinic start generated code]*/
""")
generated = self.clinic.parse(block)
_, generated = self.clinic.parse(block)
expected_docstring_prototype = "/* PyDoc_VAR(fn__doc__); */"
self.assertIn(expected_docstring_prototype, generated)

Expand All @@ -495,7 +496,7 @@ def test_directive_printout(self):
test
/*[clinic end generated code: output=4e1243bd22c66e76 input=898f1a32965d44ca]*/
""")
generated = self.clinic.parse(block)
_, generated = self.clinic.parse(block)
self.assertEqual(generated, expected)

def test_directive_preserve_twice(self):
Expand Down Expand Up @@ -533,7 +534,7 @@ def test_directive_preserve_output(self):
[clinic start generated code]*/
/*[clinic end generated code: output=da39a3ee5e6b4b0d input=524ce2e021e4eba6]*/
""")
generated = self.clinic.parse(block)
_, generated = self.clinic.parse(block)
self.assertEqual(generated, block)

def test_directive_output_invalid_command(self):
Expand Down Expand Up @@ -813,7 +814,7 @@ def _test_clinic(self, input, output):
c = clinic.Clinic(language, filename="file", limited_capi=False)
c.parsers['inert'] = InertParser(c)
c.parsers['copy'] = CopyParser(c)
computed = c.parse(input)
_, computed = c.parse(input)
self.assertEqual(output, computed)

def test_clinic_1(self):
Expand Down
24 changes: 17 additions & 7 deletions Tools/clinic/clinic.py
Original file line number Diff line number Diff line change
Expand Up @@ -2407,6 +2407,7 @@ def parse(self, input: str) -> str:
header_includes=self.includes)

# these are destinations not buffers
generated = []
for name, destination in self.destinations.items():
if destination.type == 'suppress':
continue
Expand Down Expand Up @@ -2450,11 +2451,12 @@ def parse(self, input: str) -> str:
core_includes=True,
limited_capi=self.limited_capi,
header_includes=self.includes)
libclinic.write_file(destination.filename,
printer_2.f.getvalue())
generated.append((destination.filename,
printer_2.f.getvalue()))
continue

return printer.f.getvalue()
cooked = printer.f.getvalue()
return generated, cooked

def _module_and_class(
self, fields: Sequence[str]
Expand Down Expand Up @@ -2493,6 +2495,7 @@ def parse_file(
limited_capi: bool,
output: str | None = None,
verify: bool = True,
force: bool = False,
) -> None:
if not output:
output = filename
Expand Down Expand Up @@ -2522,9 +2525,14 @@ def parse_file(
verify=verify,
filename=filename,
limited_capi=limited_capi)
cooked = clinic.parse(raw)
generated, cooked = clinic.parse(raw)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need better naming here. cooked must go.


libclinic.write_file(output, cooked)
changes = [libclinic.file_changed(f, data) for f, data in generated]
if any(changes) or libclinic.file_changed(output, cooked) or force:
libclinic.write_file(output, cooked)
for (output, cooked), changed in zip(generated, changes):
if changed or force:
libclinic.write_file(output, cooked)


class PythonParser:
Expand Down Expand Up @@ -6295,7 +6303,8 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None:
if ns.verbose:
print(path)
parse_file(path,
verify=not ns.force, limited_capi=ns.limited_capi)
verify=not ns.force, limited_capi=ns.limited_capi,
force=ns.force)
Comment on lines +6306 to +6307
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels silly that verify is always the opposite of force.

return

if not ns.filename:
Expand All @@ -6308,7 +6317,8 @@ def run_clinic(parser: argparse.ArgumentParser, ns: argparse.Namespace) -> None:
if ns.verbose:
print(filename)
parse_file(filename, output=ns.output,
verify=not ns.force, limited_capi=ns.limited_capi)
verify=not ns.force, limited_capi=ns.limited_capi,
force=ns.force)


def main(argv: list[str] | None = None) -> NoReturn:
Expand Down
2 changes: 2 additions & 0 deletions Tools/clinic/libclinic/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
FormatCounterFormatter,
compute_checksum,
create_regex,
file_changed,
write_file,
)

Expand All @@ -43,6 +44,7 @@
"FormatCounterFormatter",
"compute_checksum",
"create_regex",
"file_changed",
"write_file",
]

Expand Down
12 changes: 6 additions & 6 deletions Tools/clinic/libclinic/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@
from typing import Literal


def write_file(filename: str, new_contents: str) -> None:
"""Write new content to file, iff the content changed."""
def file_changed(filename: str, new_contents: str) -> bool:
try:
with open(filename, encoding="utf-8") as fp:
old_contents = fp.read()

if old_contents == new_contents:
# no change: avoid modifying the file modification time
return
return old_contents != new_contents
except FileNotFoundError:
pass
return False


def write_file(filename: str, new_contents: str) -> None:
# Atomic write using a temporary file and os.replace()
filename_new = f"{filename}.new"
with open(filename_new, "w", encoding="utf-8") as fp:
Expand Down
Loading