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

fix: docker cp of dangling symlink (#943) #948

Merged
merged 2 commits into from
Jan 27, 2022
Merged

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Jan 2, 2022

Fixes #943

@jsoref jsoref requested a review from a team as a code owner January 2, 2022 02:41
@codecov
Copy link

codecov bot commented Jan 2, 2022

Codecov Report

Merging #948 (653ba97) into master (0f04942) will increase coverage by 8.23%.
The diff coverage is 66.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #948      +/-   ##
==========================================
+ Coverage   49.27%   57.50%   +8.23%     
==========================================
  Files          23       32       +9     
  Lines        2401     4594    +2193     
==========================================
+ Hits         1183     2642    +1459     
- Misses       1090     1729     +639     
- Partials      128      223      +95     
Impacted Files Coverage Δ
pkg/common/job_error.go 0.00% <0.00%> (ø)
pkg/common/outbound_ip.go 0.00% <0.00%> (ø)
pkg/common/testflag.go 0.00% <0.00%> (ø)
pkg/container/docker_volume.go 0.00% <0.00%> (ø)
pkg/model/action.go 0.00% <0.00%> (ø)
pkg/model/step_result.go 0.00% <0.00%> (ø)
pkg/container/docker_run.go 5.54% <13.79%> (+3.61%) ⬆️
pkg/common/git.go 49.82% <31.81%> (-9.97%) ⬇️
pkg/runner/logger.go 65.43% <37.50%> (+1.28%) ⬆️
pkg/container/docker_auth.go 45.00% <45.00%> (ø)
... and 38 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7dbf3fc...653ba97. Read the comment docs.

pkg/container/docker_run.go Outdated Show resolved Hide resolved
@mergify mergify bot requested a review from a team January 26, 2022 21:31
@jsoref jsoref requested review from ChristopherHX and removed request for a team January 26, 2022 22:24
@mergify mergify bot requested a review from a team January 26, 2022 22:32
ChristopherHX
ChristopherHX previously approved these changes Jan 26, 2022
Copy link
Contributor

@ChristopherHX ChristopherHX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, ( dangling ) symlinks are now also working on windows.

@mergify mergify bot requested a review from a team January 26, 2022 22:57
@ZauberNerd
Copy link
Contributor

ZauberNerd commented Jan 27, 2022

There's symlink error handling in the io.Copy block:

if fi.Mode()&os.ModeSymlink == os.ModeSymlink {
shoudn't that be removed as we will never reach it?

Also: How is this change macOS only? I don't have a mac handy so I tested on Linux and could reproduce it just as well.
Edit: Ok, there are differences between macOS and Linux. I don't get the Error: open /.../act/dangling/baz: no such file or directory but act still exits with a non-zero status when running on act@master. With this change the reproducer workflow succeeds.

@jsoref
Copy link
Contributor Author

jsoref commented Jan 27, 2022

I use act on macOS, and thus describe things based on this. It seemed unlikely to me that such a thing wouldn't have been tripped on already by Linux users, so I described things based on how I encountered them. That there is some difference between how it behaves on Linux may have contributed to how I described it.

If the problem happens for everyone, then I can change the description to not specify.

I'll remove the code you've identified.

@ZauberNerd
Copy link
Contributor

Thanks, yeah looks like it happens on all platforms. With the fix #971 from Christopher applied the error is now also printed for me (DEBU[0000] open /.../act-dangling/baz: no such file or directory).

@jsoref jsoref changed the title fix: docker cp of symlink on macOS (#943) fix: docker cp of symlink (#943) Jan 27, 2022
@jsoref jsoref changed the title fix: docker cp of symlink (#943) fix: docker cp of dangling symlink (#943) Jan 27, 2022
@mergify mergify bot requested a review from a team January 27, 2022 16:39
@mergify mergify bot merged commit c802064 into nektos:master Jan 27, 2022
@jsoref jsoref deleted the issue-943 branch January 27, 2022 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue: act actions/checkout fails for dangling symlinks
4 participants