When a backport request is made, the resulting pull request will have a
title like this:
<release branch>: <First line of HEAD commit for the branch>
And a body that says:
Backport <commit0> <commit1> ..
Requested By: <user>
This will allow us to track the state of the backport request in the PR,
rather than in the issue. The state updates for PRs can be automated, so
this will save us some triage work.
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
This change adds a comment to the first PR from a new contributor that
is merged, which tells them what to expect post merge from the build
bots.
How they will be notified, where to ask questions, that you're more
likely to be reverted than in other projects, etc. The information
overlaps with, and links to
https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr.
So that users who simply read the email are still aware, and know where
to follow up if they do get reports.
To do this, I have added a hidden HTML comment to the new contributor
greeting comment. This workflow will look for that to tell if the author
of the PR was a new contributor at the time they opened the merge. It
has to be done this way because as soon as the PR is merged, they are by
GitHub's definition no longer a new contributor and I suspect that their
author association will be "contributor" instead.
I cannot 100% confirm that without a whole lot of effort and probably
breaking GitHub's terms of service, but it's fairly cheap to work around
anyway. It seems rare / almost impossible to reopen a PR in llvm at
least, but in case it does happen the buildbot info comment has its own
hidden HTML comment. If we find this we will not post another copy of
the same information.
We used to support a /branch comment to specify a branch with commits to
backport to the release branch. However, now that we can use pull
requests this is not needed.
This also simplifies the process, because now the cherry-pick job can
create the pull request directly instead of having it split across two
separate jobs.
This is currently broken, because the check is performed on the wrong
repository. repo here is llvm/llvm-project, which is not a fork (so this
will always trigger), then we'll push a new branch to
llvmbot/llvm-project, and then again set the wrong owner, so we'll look
for the branch in llvm/llvm-project rather than llvmbot/llvm-project.
Rather than fixing this, I'm removing the code entirely, as it shouldn't
be needed anymore (llvmbot/llvm-project is a fork of llvm/llvm-project).
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.
This includes some commonly needed information like how to add
reviewers.
This is implemented as a job before the labeler, so that on a new PR the
comment is added before there are any subscribers and only the author
gets a nofitication.
The labeler job depends on the greeter having run or having been
skipped. So if the PR wasn't just opened, or it's from a regular
contributor, the labeling still happens.
But we can be sure that when a greeting comment is left, it's the very
first thing we do.
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.
Before this patch, the github automation script would fail when trying
to escape the text of a PR/issue description that was empty as the
Github library returns None instead of an empty string in those
scenarios. This patch special cases this and makes the escape function
return an empty string when given a value of None.
* 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`
Currently the email that gets sent out to people subscribing to a label
that the bot tags on the PR doesn't include any authorship information
which some people are interested in having. This patch adds an author
field to the message with the relevant information.
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.
* This avoid pinging folks on all issue when they got pinged on bugzilla
eons ago
* Avoid formatting bugs when there is html in the issue description
* Truncate the list of files and the diff independently of each other.
This avoids truncating cutting a file line in 2 and to cut in the middle
of html markup. This is a fringe case but it does happen when people
accidentally push weird branches conflicting on all the files.
The builtin concurrency functionality for the workflows will cancel a
pending job if there is another job from the same workflow running. For
the pr-subscriber job, this means that if multiple labels are added at
the same time, then some of the pr-subscriber jobs will be cancelled and
the PR will not have all the necessary mentions.
This will reduce the number of notifications created when a pull request
label is added. Each team will only get a notification when their team's
label is added and not when other teams' labels are added.
This new workflow will make it possible for people to subscribe to pull
requests based on the labels that are added. Labels will be added
automatically to the pull requests based on the modified files and each
label will be associated with a GitHub team that will be notified when
the label is added.
See
https://discourse.llvm.org/t/changes-to-pull-request-subscription-system/73296
