Fix inconsistent link_path behavior #194
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When calling `path` on a file using the local transport, it would call
Train::Extras::LinuxFile.path which would in turn call read_target_path
if the file was a symlink (which calls the `readlink` OS program). The
return value would be stored in `@link_path` which is then returned whenever
`link_path` is called on the file.
`readlink` does not operate the same way on macOS like it does on Linux
variants. Specifically, in the case of cascading symlinks (a -> b -> c), it will
return "c" on Linux but "b" on macOS. This leads to inconsistent behavior
depending on if you call `path` before `link_path` since `path` stores a value
in `@link_path`. For example:
Example 1 (calling path before link_path):
```
inspec> a = file('/tmp/top_link')
=> File /tmp/top_link
inspec> a.path
=> "/tmp/middle_link"
inspec> a.link_path
=> "/tmp/middle_link"
```
Example 2 (calling link_path before path):
```
inspec> a = file('/tmp/top_link')
=> File /tmp/top_link
inspec> a.link_path
=> "/private/tmp/actual_file"
inspec> a.path
=> "/private/tmp/actual_file"
```
Plus, we have the Ruby stdlib calls we can and should use when
local so Windows, Linux, and macOS users have a consistent experience. Therefore,
this change implements a `path` method override on the Local File class to
avoid LinuxFile from getting in the way.
Signed-off-by: Adam Leff <adam@leff.co>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When calling
pathon a file using the local transport, it would call Train::Extras::LinuxFile.path which would in turn call read_target_path if the file was a symlink (which calls thereadlinkOS program). The return value would be stored in@link_pathwhich is then returned wheneverlink_pathis called on the file.readlinkdoes not operate the same way on macOS like it does on Linux variants. Specifically, in the case of cascading symlinks (a -> b -> c), it will return "c" on Linux but "b" on macOS. This leads to inconsistent behavior depending on if you callpathbeforelink_pathsincepathstores a value in@link_path. For example:Example 1 (calling path before link_path):
Example 2 (calling link_path before path):
Plus, we have the Ruby stdlib calls we can and should use when local so Windows, Linux, and macOS users have a consistent experience. Therefore, this change implements a
pathmethod override on the Local File class to avoid LinuxFile from getting in the way.