Skip to content

Fix symlink handling in CopyDirectory method #3914

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

black-desk
Copy link

Fixes #3234

This change adds proper support for symbolic links
in the CopyDirectory method.

NOTE: This patch was developed with GitHub Copilot
assistance. I has NO C# experience at all and
is not familiar with actions/runner. This change
has not been tested in a local environment. Please
review this patch carefully.

Signed-off-by: Chen Linxuan me@black-desk.cn

@Copilot Copilot AI review requested due to automatic review settings June 17, 2025 16:40
@black-desk black-desk requested a review from a team as a code owner June 17, 2025 16:40
Copilot

This comment was marked as outdated.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the CopyDirectory method to properly detect and preserve symbolic links when copying directories and files.

  • Adds early detection for directory symlinks and creates matching links in the target.
  • Updates file-copy logic to skip already up-to-date files and supports file symlinks.
  • Removes the duplicate sourceDir declaration and centralizes its initialization.
Comments suppressed due to low confidence (1)

src/Runner.Sdk/Util/IOUtil.cs:392

  • The new symlink-handling branches (for directories at lines 395–408 and files at lines 426–434) lack unit tests. Please add non-trivial L0 tests under Test/L0 to verify both directory and file symlink scenarios.
            // Get the file contents of the directory to copy.

}

// Check if source is a symlink
if (!sourceFile.Attributes.HasFlag(FileAttributes.ReparsePoint))
Copy link
Preview

Copilot AI Jun 17, 2025

Choose a reason for hiding this comment

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

[nitpick] For file symlinks, add a check to skip recreating an existing link when the target matches, similar to the directory case, to avoid unnecessary filesystem operations.

Copilot uses AI. Check for mistakes.

Copy link
Author

@black-desk black-desk Jun 17, 2025

Choose a reason for hiding this comment

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

I think this logic may have been covered by the section above that checks for the existence of target

Fixes actions#3234

This change adds proper support for symbolic links
in the CopyDirectory method.

NOTE: This patch was developed with GitHub Copilot
assistance. I has **NO** C# experience at all and
is not familiar with actions/runner. This change
has not been tested in a local environment. Please
review this patch carefully.

Signed-off-by: Chen Linxuan <me@black-desk.cn>
@black-desk
Copy link
Author

I don't even have a full C# development environment, so if there's anything the project maintainers would like to change, it's perfectly fine to push it directly to my branch and merge it, and I hope this will be fixed soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When action repository contains a broken symbolic link, "Set up job" stage will fail
1 participant