Skip to content

Commit fdde57d

Browse files
[Github] Fix revisions in code format action reproducers (#155193)
This patch makes it so the revisions that the code format action returns in its reproducers actually work when applying them locally given the differences in how revisions are setup in CI. Fixes #154294
1 parent 8d5dead commit fdde57d

File tree

1 file changed

+52
-30
lines changed

1 file changed

+52
-30
lines changed

llvm/utils/git/code-format-helper.py

Lines changed: 52 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,12 @@ def pr_comment_text_for_diff(self, diff: str) -> str:
9292
{self.instructions}
9393
``````````
9494
95+
:warning:
96+
The reproduction instructions above might return results for more than one PR
97+
in a stack if you are using a stacked PR workflow. You can limit the results by
98+
changing `origin/main` to the base branch/commit you want to compare against.
99+
:warning:
100+
95101
</details>
96102
97103
<details>
@@ -175,9 +181,32 @@ class ClangFormatHelper(FormatHelper):
175181
name = "clang-format"
176182
friendly_name = "C/C++ code formatter"
177183

184+
def _construct_command(self, diff_expression: list[str] | None):
185+
cf_cmd = [self.clang_fmt_path, "--diff"]
186+
187+
if diff_expression:
188+
cf_cmd.extend(diff_expression)
189+
190+
# Gather the extension of all modified files and pass them explicitly to git-clang-format.
191+
# This prevents git-clang-format from applying its own filtering rules on top of ours.
192+
extensions = set()
193+
for file in self._cpp_files:
194+
_, ext = os.path.splitext(file)
195+
extensions.add(
196+
ext.strip(".")
197+
) # Exclude periods since git-clang-format takes extensions without them
198+
cf_cmd.append("--extensions")
199+
cf_cmd.append(",".join(extensions))
200+
201+
cf_cmd.append("--")
202+
cf_cmd += self._cpp_files
203+
return cf_cmd
204+
178205
@property
179206
def instructions(self) -> str:
180-
return " ".join(self.cf_cmd)
207+
# TODO(boomanaiden154): Add --diff_from_common_commit option when it has
208+
# landed as in available in a released version.
209+
return " ".join(self._construct_command(["origin/main", "HEAD"]))
181210

182211
def should_include_extensionless_file(self, path: str) -> bool:
183212
return path.startswith("libcxx/include")
@@ -218,33 +247,19 @@ def has_tool(self) -> bool:
218247
return proc.returncode == 0
219248

220249
def format_run(self, changed_files: List[str], args: FormatArgs) -> Optional[str]:
221-
cpp_files = self.filter_changed_files(changed_files)
222-
if not cpp_files:
250+
self._cpp_files = self.filter_changed_files(changed_files)
251+
if not self._cpp_files:
223252
return None
224253

225-
cf_cmd = [self.clang_fmt_path, "--diff"]
226-
254+
diff_expression = []
227255
if args.start_rev and args.end_rev:
228-
cf_cmd.append(args.start_rev)
229-
cf_cmd.append(args.end_rev)
230-
231-
# Gather the extension of all modified files and pass them explicitly to git-clang-format.
232-
# This prevents git-clang-format from applying its own filtering rules on top of ours.
233-
extensions = set()
234-
for file in cpp_files:
235-
_, ext = os.path.splitext(file)
236-
extensions.add(
237-
ext.strip(".")
238-
) # Exclude periods since git-clang-format takes extensions without them
239-
cf_cmd.append("--extensions")
240-
cf_cmd.append(",".join(extensions))
256+
diff_expression.append(args.start_rev)
257+
diff_expression.append(args.end_rev)
241258

242-
cf_cmd.append("--")
243-
cf_cmd += cpp_files
259+
cf_cmd = self._construct_command(diff_expression)
244260

245261
if args.verbose:
246262
print(f"Running: {' '.join(cf_cmd)}")
247-
self.cf_cmd = cf_cmd
248263
proc = subprocess.run(cf_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
249264
sys.stdout.write(proc.stderr.decode("utf-8"))
250265

@@ -263,9 +278,20 @@ class DarkerFormatHelper(FormatHelper):
263278
name = "darker"
264279
friendly_name = "Python code formatter"
265280

281+
def _construct_command(self, diff_expression: str | None) -> str:
282+
darker_cmd = [
283+
self.darker_fmt_path,
284+
"--check",
285+
"--diff",
286+
]
287+
if diff_expression:
288+
darker_cmd += ["-r", diff_expression]
289+
darker_cmd += self._py_files
290+
return darker_cmd
291+
266292
@property
267293
def instructions(self) -> str:
268-
return " ".join(self.darker_cmd)
294+
return " ".join(self._construct_command("origin/main...HEAD"))
269295

270296
def filter_changed_files(self, changed_files: List[str]) -> List[str]:
271297
filtered_files = []
@@ -295,17 +321,13 @@ def format_run(self, changed_files: List[str], args: FormatArgs) -> Optional[str
295321
py_files = self.filter_changed_files(changed_files)
296322
if not py_files:
297323
return None
298-
darker_cmd = [
299-
self.darker_fmt_path,
300-
"--check",
301-
"--diff",
302-
]
324+
self._py_files = py_files
325+
diff_expression = None
303326
if args.start_rev and args.end_rev:
304-
darker_cmd += ["-r", f"{args.start_rev}...{args.end_rev}"]
305-
darker_cmd += py_files
327+
diff_expression = f"{args.start_rev}...{args.end_rev}"
328+
darker_cmd = self._construct_command(diff_expression)
306329
if args.verbose:
307330
print(f"Running: {' '.join(darker_cmd)}")
308-
self.darker_cmd = darker_cmd
309331
proc = subprocess.run(
310332
darker_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE
311333
)

0 commit comments

Comments
 (0)