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

windows: added Ansible.IO c# code to work on paths greater than 260 #43666

Closed
wants to merge 6 commits into from

Conversation

jborean93
Copy link
Contributor

@jborean93 jborean93 commented Aug 4, 2018

SUMMARY

This PR adds the namespace Ansible.IO to the Ansible.ModuleUtils.FileUtil that allows modules to interact with paths that exceeds MAX_PATH (~260 chars). This namespace is designed to replicate the functionality of System.IO.Directory/DirectoryInfo/File/FileInfo/Path but also to add support for the following;

  • Interact with paths that exceed MAX_PATH when using the \\?\ prefix
  • Automatic privilege enablement when setting ACLs that require the relevant privileges
  • Added Compress() and Decompress() methods to enable the toggling of the Compress attribute
  • Added the ability to manipulate sparse files and clear ranges within a sparse file
  • More functions to enumerate alternative streams and read/write to these streams

Further changes to this PR are to LinkUtil which changes;

  • Support for links greater than MAX_PATH
  • Support for creating symlinks/junction points against a missing target
  • Support for creating relative symlinks
  • Support for creating symlinks on newer Windows versions without having admin privileges
  • Only enable the SeBackupPrivilege when required instead of doing it globally

First part to achieve #43060.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

lib/ansible/module_utils/powershell/Ansible.ModuleUtils.FileUtil.psm1
lib/ansible/module_utils/powershell/Ansible.ModuleUtils.LinkUtil.psm1
win_stat
win_command
win_shell
win_wait_for

ANSIBLE VERSION
devel
ADDITIONAL INFORMATION

There is a lot of code in the Ansible.ModuleUtils.FileUtil.psm1 is slightly modified from https://github.com/dotnet/corefx which is licensed under the MIT license. When doing some testing against devel, the win_stat tests only took an extra 1-3 seconds so the extra code does not seem to slow anything down dramatically.

@ansibot
Copy link
Contributor

ansibot commented Aug 4, 2018

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 core_review In order to be merged, this PR must follow the core review workflow. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. windows Windows community needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Aug 4, 2018
@ansible ansible deleted a comment from ansibot Aug 5, 2018
@ansibot
Copy link
Contributor

ansibot commented Aug 5, 2018

@jborean93 jborean93 force-pushed the win_max_path branch 3 times, most recently from 86c3ec0 to b98b544 Compare August 6, 2018 01:32
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Aug 6, 2018
@ryansb ryansb removed the needs_triage Needs a first human triage before being processed. label Aug 7, 2018
# Test-Path -IsValid failed when the long path prefix is there, we don't want
# to check this ourselves and just rely on Windows to validate the path when
# using it
if ((-not ($value.StartsWith("\\?\"))) -and (-not (Test-Path -Path $value -IsValid))) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm wondering whether we should not even run Expand-Environment on the value if it starts with \\?\ to allow people to force a file/dir name with a value that's like an env var.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Aug 8, 2018
@mattclay
Copy link
Member

I've restarted the failing CI job multiple times, but it keeps coming back unstable:

https://app.shippable.com/github/ansible/ansible/runs/77931/12/tests

@mattclay mattclay added the ci_verified Changes made in this PR are causing tests to fail. label Aug 10, 2018
@jborean93
Copy link
Contributor Author

@mattclay thanks, looks like there's still an issue with async. Will look into it.

@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Aug 10, 2018
@ansibot ansibot added the core_review In order to be merged, this PR must follow the core review workflow. label Aug 10, 2018
@ansibot ansibot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Aug 12, 2018
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Aug 29, 2018
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Sep 6, 2018
@ansibot
Copy link
Contributor

ansibot commented Feb 17, 2019

cc @if-meaton
click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Jun 1, 2019

@jborean93 jborean93 closed this Nov 14, 2019
@ansible ansible locked and limited conversation to collaborators Dec 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 feature This issue/PR relates to a feature request. has_issue module This issue/PR relates to a module. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. windows Windows community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants