Split commands.yml into running and saving #18688
Conversation
TODO - add channel for variables about outcome between jobs
✅ No release notes required |
|
@T-Gro would be possible to include |
|
Will have to figure out how to do it without a dummy commit (my current naive idea) and without risks with exposing the access token. Will check how this is done elsewhere. |
|
AFAIK it never worked for OSS contributors, who also do not have the "rerun" buttons when seeing the |
|
/azp run |
|
Commenter does not have sufficient privileges for PR 18688 in repo dotnet/fsharp |
|
@T-Gro i get |
Correct, but what if it's person's own PR? I thought arcade folks fixed it - that you can rerun own PR. |
Tried in my own PR and did not work. See #18670 (comment) |
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| outputs: | ||
| command: ${{ steps.parse.outputs.command }} |
There was a problem hiding this comment.
Review: Crucial to verify that this command is always from the known set.
And gets not set if it is something else ( = the comment-pipeline must fail on unknown input0
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| outputs: | ||
| command: ${{ steps.parse.outputs.command }} | ||
| arg: ${{ steps.parse.outputs.arguments }} |
There was a problem hiding this comment.
Crucial to verify:
This must be used in a safe way, especially in the second job.
| "/run fantomas") dotnet fantomas . ;; | ||
| "/run xlf") dotnet build src/Compiler /t:UpdateXlf ;; | ||
| "/run ilverify") pwsh tests/ILVerify/ilverify.ps1 ;; | ||
| "/run test-baseline") dotnet test ./FSharp.Compiler.Service.sln --filter "${{ steps.parse.outputs.arguments }}" -c Release || true ;; |
There was a problem hiding this comment.
Review:
This runs user-provided snippet and can pass other arguments to dotnet test, even run other script. This is still in the context of read-only permissions.
| git config user.name "GH Actions" | ||
| git config user.email "actions@github.com" | ||
| git add -u | ||
| git commit -m "Apply patch from ${{ needs.detect-and-run.outputs.command }}" |
There was a problem hiding this comment.
Important: The command must be from a known set, otherwise this is shell injection.
The command is coming from the comment pipeline, only valid command will go trough.
| _CurrentWorkflowID="$(gh api -X GET "$_UrlPath" | jq '.workflows[] | select(.name == '\""$GITHUB_WORKFLOW"\"') | .id')" | ||
| - name: Parse comment | ||
| id: parse | ||
| uses: dotnet/comment-pipeline@1 |
There was a problem hiding this comment.
Using a dotnet fork of the pipeline instead of the original.
This PR restructures the GH actions into two jobs.
First one runs and executes arbitrary code, and is by nature unsafe. It ends with creating a diff patch, and does not have any write permissions to anything.
The second job is not user extensible, and applies the diff to the source branch of the PR.
The following comments in a PR can be used as commands to execute scripts which automate repository maintenance and make it part of the visible diff.
/run fantomasrunsdotnet fantomas ./run ilverifyupdates IL verification baseline/run xlfrefreshes localisation files for translatable strings/run test-baseline ...runs tests with theTEST_UPDATE_BSL: 1environment variable and an argument supplied filter (passed todotnet test --filter ..). Its goal is to refresh baselines.@edgarfgp , @auduchinok : Are there other types of content changes that would also made use of a command, and would not be covered by the above?