Allow local actions outside the workspace#2108
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2108 +/- ##
===========================================
+ Coverage 61.56% 74.67% +13.11%
===========================================
Files 53 73 +20
Lines 9002 11132 +2130
===========================================
+ Hits 5542 8313 +2771
+ Misses 3020 2183 -837
- Partials 440 636 +196 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
In my opinion is It doesn't look to be far away from |
For this change we're talking about reading an existing In the absence of being able to use contexts within |
|
Yes we both have different opinions here... the good thing for you is that my opinion alone doesn't prevent merging this. Keep in mind doing this in monorepos has an old unfixed bug when referencing local actions in actions/runner (those action which has post steps, like actions/cache, actions/checkout): actions/runner#2009, input and outputs end up with wrong values...
I'm awaiting the opinion of the other maintainers if they have any, either way you need another review |
|
Side note bug 2009 in actions/runner also technically allows endless recursion via local composite actions. I assume act has the same bug, however for both local and remote composite actions. |
|
PR is stale and will be closed in 14 days unless there is new activity |
|
I’d still like to see this reviewed and merged if possible, I have various actions that use this pattern, and I can’t currently run them under act. I’ll update the PR to resolve the conflicts |
3db5ca9 to
3fc20b5
Compare
|
@jenseng this pull request has failed checks 🛠 |
3fc20b5 to
94fbf74
Compare
|
@jenseng this pull request has failed checks 🛠 |
|
🤔 lint failure seems like a bug in i.e. using golangci-lint@1.53, i get the same failure locally if i do: or: but not if i do: |
|
Yes only me again, one maintainer left (at least made the information available) after my last message here My concern option B
has not been resolved, so not my turn ( The stale bot seems at this state a bit rough I did downgrade the go syntax in one of my PR's once due to this linter, but your case is different Updating the linter using the version field would pull new lint errors of newer stricter rules I don't really like deal with. |
jsoref
left a comment
There was a problem hiding this comment.
I'm not a member, but this fixes my test case: https://github.com/check-spelling-sandbox/nektos-act-issue-up-dir, and I see no reason it wouldn't fix my problems with the github/codeql-action repository.
It seems like a reasonable and correct change (to match a behavior upon which GitHub's devs rely).
|
PR is stale and will be closed in 14 days unless there is new activity |
|
Fwiw, I've switched check-spelling to rely on this approach for actions... If there's a way I can help, I'm somewhat available. |
|
Sorry, this slipped through the cracks, I'll resolve the conflicts and freshen this up. @ChristopherHX restricting it to What if we restricted to |
|
imo, we should remain bug-compatible with github if they allow something that shouldn't be done, but library downstreams should be aware of security implications this causes |
|
if ci passes and panekj approves I let this merge like this is |
|
Fwiw, my future release which I'm hoping to make shortly will need this. |
Also simplify actionName logic and ensure it returns sensible values for actions outside the workspace (it's only used for logging and docker image name)
94fbf74 to
c83a17e
Compare
|
@ChristopherHX @panekj let me know if there's anything else here that needs tweaking, thanks! |
ChristopherHX
left a comment
There was a problem hiding this comment.
No known regression in new action backend (where full test suite is not running here), by cherry picking this for testing only.
This is a perfect example that is not going to work in act even if implemented without aligning the folder layout
uses: actions/github-script/../../../actions/github-script/v7@v7
If you expand this further, you can get the the working directory, other actions and everywhere else as well.
IMO act is the wrong tool for being 100% bug/feature compatible, this is not documented and can be removed at any point of time from GitHub side via a bug fix.
This "approval" does not allow merging this PR without additional approval from one maintainer / owner
|
If everything goes right 🙈 this works in GitHub Actions using actions/runner locally - name: Checkout code
uses: actions/checkout@v4
if: false
- name: Create Step Summary
uses: actions/github-script/../../../actions/checkout/v4@v7Skipping the checkout action, by just disable running it. Only clone the action and call actions/checkout via actions/github-script. |
Merge Queue Status
This pull request spent 5 seconds in the queue, with no time running CI. ReasonThe pull request can't be updated
HintYou should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again. |
|
@panekj can you fix the workflow?
|
Fixes #2107
Also simplify
actionNamelogic and ensure it returns sensible values for actions outside the workspace (it's only used for logging and docker image name)