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

[3007.x] Make file.symlink/_symlink_check() respect follow_symlinks #66981

Open
wants to merge 3 commits into
base: 3007.x
Choose a base branch
from

Conversation

tomdoherty
Copy link

What does this PR do?

See issue for details

What issues does this PR fix or reference?

Fixes: #66980

Previous Behavior

_symlink_check() always uses os.path.exists regardless of follow_symlinks

New Behavior

follow_symlinks=False causes os.path.lexists() to be used.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices, including the
PR Guidelines.

See GitHub's page on GPG signing for more information about signing commits with GPG.

Copy link
Contributor

@twangboy twangboy left a comment

Choose a reason for hiding this comment

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

Would you mind writing a test for this?

@tomdoherty tomdoherty force-pushed the tomdoherty-_symlink_check-follow_symlinks branch from 8ed8317 to 8865c2e Compare October 22, 2024 12:44
@tomdoherty
Copy link
Author

Would you mind writing a test for this?

Done!

@tomdoherty
Copy link
Author

@dwoz @twangboy any hints on getting this merged?

@twangboy
Copy link
Contributor

twangboy commented Nov 1, 2024

Our infra is broken right now... once we get it fixed, we'll have you rebase and let the tests run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test:full Run the full test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants