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

Use salt.utils.path.readlink to handle junctions on Windows #61326

Merged
merged 6 commits into from
Jan 21, 2022

Conversation

twangboy
Copy link
Contributor

@twangboy twangboy commented Dec 3, 2021

What does this PR do?

Attempts to handle the readlink function properly by using the salt util instead of os.readlink directly. Python 3.8 added support for directory junctions on Windows which prefixes the return with \\?\. The salt util strips that off and gives you back a clean path.

Fixes: #61458

Merge requirements satisfied?

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

Commits signed with GPG?

Yes

@twangboy twangboy requested a review from a team as a code owner December 3, 2021 22:48
@twangboy twangboy requested review from waynew and removed request for a team December 3, 2021 22:48
Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me, but needs a changelog entry and test(s).

@twangboy twangboy force-pushed the fix_61170 branch 7 times, most recently from 5436f8f to e75d627 Compare January 14, 2022 23:32
@twangboy
Copy link
Contributor Author

@waynew After looking more closely at the issue, this does not fix #61170 . This addresses an issue with symlinks. Issue #61170 is an issue with shortcuts, not symlinks.

@twangboy twangboy force-pushed the fix_61170 branch 3 times, most recently from 7d1d693 to fca0494 Compare January 18, 2022 23:53
@twangboy twangboy added the Phosphorus v3005.0 Release code name and version label Jan 18, 2022
@twangboy twangboy force-pushed the fix_61170 branch 3 times, most recently from 772aa83 to 9fb2572 Compare January 20, 2022 16:49
salt/modules/file.py Show resolved Hide resolved
salt/modules/file.py Outdated Show resolved Hide resolved
tests/pytests/functional/modules/file/test_replace.py Outdated Show resolved Hide resolved
Add functional tests for the following:
- file.readlink
- file.replace
- file.symlink

Remove unit tests for file.replace as they are duplicated in the added
functional test
@twangboy twangboy force-pushed the fix_61170 branch 2 times, most recently from dd8f268 to b45d710 Compare January 20, 2022 21:56
@Ch3LL Ch3LL merged commit eded695 into saltstack:master Jan 21, 2022
@twangboy twangboy deleted the fix_61170 branch March 23, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Phosphorus v3005.0 Release code name and version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] User salt.utils.path.readlink on everywhere
3 participants