Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Meta-copy-symlinks-new-actions-cache #2337

Merged
merged 7 commits into from
May 29, 2024

Conversation

ChristopherHX
Copy link
Contributor

@ChristopherHX ChristopherHX commented May 22, 2024

Experimental

Fixes #2334

@ChristopherHX ChristopherHX requested a review from a team as a code owner May 22, 2024 21:56
Copy link
Contributor

github-actions bot commented May 22, 2024

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ EDITORCONFIG editorconfig-checker 2 0 0.01s
✅ REPOSITORY gitleaks yes no 2.32s
✅ REPOSITORY git_diff yes no 0.01s
✅ REPOSITORY grype yes no 8.54s
✅ REPOSITORY secretlint yes no 1.2s
✅ REPOSITORY trivy-sbom yes no 0.62s
✅ REPOSITORY trufflehog yes no 4.1s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@ChristopherHX
Copy link
Contributor Author

@jsoref Please play around with this change to the "--use-new-action-cache`.

  • your sample now has a copy of the file (maybe needs an unit test)
  • copies of files in symlinked folders might work or not (maybe needs an unit test)
  • copies of multilevel symlinks might work or not (maybe needs an unit test)

act is my golang playground, there are bugs hiding behind every corner

@ChristopherHX
Copy link
Contributor Author

If this is in, I might propose to retire the old action cache and update all tests to use the new cache.

Copy link

codecov bot commented May 22, 2024

Codecov Report

Attention: Patch coverage is 50.87719% with 28 lines in your changes are missing coverage. Please review.

Project coverage is 76.25%. Comparing base (5a80a04) to head (ecc9afe).
Report is 74 commits behind head on master.

Files Patch % Lines
pkg/runner/action_cache.go 50.00% 21 Missing and 7 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2337       +/-   ##
===========================================
+ Coverage   61.56%   76.25%   +14.68%     
===========================================
  Files          53       61        +8     
  Lines        9002     7803     -1199     
===========================================
+ Hits         5542     5950      +408     
+ Misses       3020     1297     -1723     
- Partials      440      556      +116     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jsoref
Copy link
Contributor

jsoref commented May 23, 2024

https://github.com/check-spelling-sandbox/sturdy-tribble-2/actions/runs/9178827583/job/25332160974

Download action repository 'check-spelling-sandbox/sturdy-tribble@main' (SHA:7ea13882b70984e855964e344023114797d41e8a)
Error: Could not find file '/home/runner/work/_actions/_temp_dbefcafa-7a88-4493-9349-4e6ecae342f1/_staging/check-spelling-sandbox-sturdy-tribble-7ea1388/.github/workflows/config'.

It's relatively easy to parse the error to relating to .github/workflows/config and see that it corresponds to:
https://github.com/check-spelling-sandbox/sturdy-tribble/blob/7ea13882b70984e855964e344023114797d41e8a/.github/workflows/config

Act yields this:

INFO[0000] Using docker host 'unix:///var/run/docker.sock', and daemon socket 'unix:///var/run/docker.sock'
WARN  ⚠ You are using Apple M-series chip and you have not specified container architecture, you might encounter issues while running act. If so, try running it with '--container-architecture linux/amd64'. ⚠
[Test/test] 🚀  Start image=catthehacker/ubuntu:act-latest
[Test/test]   🐳  docker pull image=catthehacker/ubuntu:act-latest platform= username= forcePull=true
[Test/test]   🐳  docker create image=catthehacker/ubuntu:act-latest platform= entrypoint=["tail" "-f" "/dev/null"] cmd=[] network="host"
[Test/test]   🐳  docker run image=catthehacker/ubuntu:act-latest platform= entrypoint=["tail" "-f" "/dev/null"] cmd=[] network="host"
[Test/test] ⭐ Run Pre check-spelling-sandbox/sturdy-tribble@main
[Test/test]   ✅  Success - Pre check-spelling-sandbox/sturdy-tribble@main
[Test/test] ⭐ Run Main check-spelling-sandbox/sturdy-tribble@main
[Test/test]   ❌  Failure - Main check-spelling-sandbox/sturdy-tribble@main
[Test/test] failed to copy content to container: error during connect: Put "http://%2Fvar%2Frun%2Fdocker.sock/v1.43/containers/dc48f1170a64943b32f4018eb3a92533da472a08a7ea742087c317d1c4a68f9a/archive?noOverwriteDirNonDir=true&path=%2Fvar%2Frun%2Fact%2Factions%2Fcheck-spelling-sandbox-sturdy-tribble%40main%2F": file not found
[Test/test] ⭐ Run Post check-spelling-sandbox/sturdy-tribble@main
[Test/test]   ❌  Failure - Post check-spelling-sandbox/sturdy-tribble@main
[Test/test] 🏁  Job failed
Error: Error occurred running finally: failed to copy content to container: error during connect: Put "http://%2Fvar%2Frun%2Fdocker.sock/v1.43/containers/dc48f1170a64943b32f4018eb3a92533da472a08a7ea742087c317d1c4a68f9a/archive?noOverwriteDirNonDir=true&path=%2Fvar%2Frun%2Fact%2Factions%2Fcheck-spelling-sandbox-sturdy-tribble%40main%2F": file not found (original error: <nil>)

file not found and origin error: <nil> could be improved.

@ChristopherHX
Copy link
Contributor Author

actually act shouldn't defer the error so much, because the error is around this line

[Test/test]   ❌  Failure - Main check-spelling-sandbox/sturdy-tribble@main

And not at the end.

@ChristopherHX
Copy link
Contributor Author

ChristopherHX commented May 23, 2024

I now have added .git/config (.github/workflows/config): file not found

Missing filetime from git has been oberserved by me, in https://github.com/nektos/act/tree/meta-keep-filetime I tried adding it. Not shure about the performance due to calling gogit.log here... or if it is worth adding this.
I'm not adding the filetime change here, the coverage job will blame me if I do that

@jsoref
Copy link
Contributor

jsoref commented May 23, 2024

This is definitely better:

[Test/test] failed to copy content to container: error during connect: Put "http://%2Fvar%2Frun%2Fdocker.sock/v1.43/containers/471e1bc6399eeafb5a923ded9ae4e261a8cc8949990a11d43fa3e594cfa88c11/archive?noOverwriteDirNonDir=true&path=%2Fvar%2Frun%2Fact%2Factions%2Fcheck-spelling-sandbox-sturdy-tribble%40main%2F": .git/config (.github/workflows/config): file not found
[Test/test] ⭐ Run Post check-spelling-sandbox/sturdy-tribble@main
[Test/test]   ❌  Failure - Post check-spelling-sandbox/sturdy-tribble@main
[Test/test] 🏁  Job failed
Error: Error occurred running finally: failed to copy content to container: error during connect: Put "http://%2Fvar%2Frun%2Fdocker.sock/v1.43/containers/471e1bc6399eeafb5a923ded9ae4e261a8cc8949990a11d43fa3e594cfa88c11/archive?noOverwriteDirNonDir=true&path=%2Fvar%2Frun%2Fact%2Factions%2Fcheck-spelling-sandbox-sturdy-tribble%40main%2F": .git/config (.github/workflows/config): file not found (original error: <nil>)

Are you still trying to get rid of an original error: <nil>?

@jsoref
Copy link
Contributor

jsoref commented May 23, 2024

And yes, I noticed the zeroed timestamps when I was comparing / trying to determine what was going wrong. But I determined it wasn't the most pressing part.

@ChristopherHX
Copy link
Contributor Author

Are you still trying to get rid of an original error: ?

I think this is a bug of older code. In the Finally executor seem to cause this by unconditionally including both err.

The first print of the error doesn't have these text portion.

@jsoref
Copy link
Contributor

jsoref commented May 23, 2024

Right. I'm not saying you have to fix it / fix it here.

In the long term, can some of this be dropped:

[Test/test] failed to copy content to container: error during connect: Put "http://%2Fvar%2Frun%2Fdocker.sock/v1.43/containers/471e1bc6399eeafb5a923ded9ae4e261a8cc8949990a11d43fa3e594cfa88c11/archive?noOverwriteDirNonDir=true&path=%2Fvar%2Frun%2Fact%2Factions%2Fcheck-spelling-sandbox-sturdy-tribble%40main%2F":

Personally, I'd much rather much less of it -- given that the problem is purely an act / workflow thing parsing actions and has no material relation to the underlying transport (docker).

@ChristopherHX
Copy link
Contributor Author

Personally, I'd much rather much less of it -- given that the problem is purely an act / workflow thing parsing actions and has no material relation to the underlying transport (docker).

One problem you would have to take into account is streaming the virtual tar archive.

Or wrap the error into a special error type and drop the outer part of the docker library if it matches

@mergify mergify bot merged commit 4977ba9 into master May 29, 2024
11 checks passed
@mergify mergify bot deleted the meta-copy-symlinks-new-actions-cache branch May 29, 2024 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants