Actions triggered by pull_request_target events have access to all
repository secrets, so it is unsafe to use them when executing untrusted
code. The pr-code-format workflow does not execute any untrusted code,
but it passes untrused input into clang-format. An attacker could use
this to exploit a flaw in clang-format and potentially gain access to
the repository secrets.
By splitting the workflow, we can use the pull_request target which is
more secure and isolate the issue write permissions in a separate job.
The pull_request target also makes it easier to test changes to the
code-format-helepr.py script, because the version of the script from the
pull request will be used rather than the version of the script from
main.
Fixes#77142
Follow-up from PR #83491. `Darker`'s configuration is ignored because of the way we invoke it - with an explicit list of files. We need to filter it in `code-format-helper.py`.
This reverts commit bc06cd5cbcfc22dd976f6742d10bc934e1353b8a.
This caused the job to fail for PRs which still had an older version
of code-format-helper.py in their tree.
Actions triggered by pull_request_target events have access to all
repository secrets, so it is unsafe to use them when executing untrusted
code. The pr-code-format workflow does not execute any untrusted code,
but it passes untrused input into clang-format. An attacker could use
this to exploit a flaw in clang-format and potentially gain access to
the repository secrets.
By splitting the workflow, we can use the pull_request target which is
more secure and isolate the issue write permissions in a separate job.
The pull_request target also makes it easier to test changes to the
code-format-helepr.py script, because the version of the script from the
pull request will be used rather than the version of the script from
main.
Fixes#77142
Before this patch, there was a regression in comment formatting due to
some code formatting in bd3e8eb6e325081bf7cfbe93652aa825de3170e5. This
was fixed in 428660cfb986dd0a59cd2a16972c5f7109080522. Github interprets
a tab before a string as starting code formatting. The message that
indicted the code formatting in a PR had been fixed was refactored to a
python multi-line string, but with a tab in front, causing these
messages to be rendered as code blocks in Github, instead of as
intended. This patch builds upon the original fix to reformat the
strings so that they fit within ~80 character lines and are simpler to
modify in the future, hopefully removing traps like the one that caused
the original issue.
Due to the way the f string was written, the text ended up with 4 spaces
at the start. 4 space indent in Markdown means plain text, which is not
what we intend here.
As part of #73798 there was some discussion about using the format
helper to run from a git-hook. That was not possible for a number of
reasons, but with these changes it can now be installed as a hook and
then run on the local cache in git instead of a diff between revisions.
This also checks for two environment variables DARKER_FORMAT_PATH and
CLANG_FORMAT_PATH where you can specify the path to the program you want
to use.
Using a two dot diff allows changes made in main after the merge base to
show up in the formatting diff. Using a three dot diff fixes this and
ensures that only changes made in the source branch (branch from the PR
author) will get passed along to the formatter.
Without this, issues like #73873 occur.
The ignore_format.txt file and the associated checks have been causing a
lot of confusion since we introduced them. Formatting becomes one of the
main hurdle for contributors (especially new contributors), and that is
not great.
The original goal of ignore_format.txt was to enforce clang-format only
in a subset of the files of the project. In practice, we have now
shifted to a model where we have a Github action that checks whether new
code surrounding edits is formatted. In that context, it probably
doesn't make sense to keep having a ignore list for formatting files.
After this patch, the clang-format job will enforce that all new code is
formatted properly, and that all edits to existing files are formatted
properly, regardless of which files the edits are in. This seems
reasonable and I believe will lead to much less confusion than our
current setup.
In the future, we could consider clang-formatting the whole code base
once and for all but this requires a bit of upfront technical work to
put in place a merge driver to help resolve merge conflicts across
formatting changes.
These headers were skipped by the job because they didn't have an
extension. However, such headers are extremely common in libc++.
As a drive-by change, also include `.cppm` and `.inc` extensions since
those are also common in libc++.
Currently, when the code format action fails, it leaves no log of the
diff in the output within the action itself. This has caused confusion
for some users of the action, especially when the comment becomes buried
in a 100+ comment review and someone isn't super familiar with the inner
workings of the CI. This patch prints the diff produced by the code
formatter to stdout so that it is viewable by clicking on the failed
action. This should have very little cost and make things slightly less
confusing for those that run into this situation.
* Consider the darker/clang-format command to have failed if the exit
code is non-zero, regardless of the stdout/stderr output.
* Propagate stderr from the formatter command to the script's
caller (and into the GitHub log).
* On success, dump stdout to the caller, so it ends up in GitHub's log.
I'm not sure what this would ever be, but if it exists, it should be
preserved.
* Just before the script exits, if any formatter failed, print a line
showing which formatters failed.
Fix type errors that mypy reports with code-format-helper.py.
Add a few return type annotations and change `param: [str]` to
`param: list[str]`.
Leave a few required FormatHelper members missing instead of defining a
placeholder:
- FormatHelper.name
- FormatHelper.friendly_name
- FormatHelper.format_run: NotImplementedError() instead of `pass`
I landed this format helper, but unfortunately, it didn't work because
of permissions, it could not add comments on a fork's PR. @cor3ntin
informed me there are fixes for this that you had worked on @tstellar -
but I didn't have time to read up on it too much. Can you explain what
changes are needed to get the action to be able to write comments on
fork's PR?
This helper will format python files with black/darker and
C/C++ files with clang-format.
The format helper is written so that we can expand it with new
formatters in the future like clang-tidy.
