(PUP-11184) Expand to long paths #8768
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
|
I need to add an integration test for this using short paths |
joshcooper
force-pushed
the
expand_long_path
branch
2 times, most recently
from
c94d415 to
48b58a3
Compare
joshcooper
commented
Sep 27, 2021
|
jenkins please test this on redhat7-64a,windows2019-64a |
Dir.glob behaves differently across platforms and ruby versions, don't stub it.
Previously, puppet failed to load if the current working directory was a short
Windows path. This was due to trying to require `Puppet::Util::Uniquefile` using
its long and short path. This regression was introduced with the move to
require_relative.
The `Autoload.files_in_dir` method is expected to return relative paths that are
contained within the `dir` argument. For example, returning an array containing
`puppet/provider/exec/posix`.
However, if the `dir` argument contained a short Windows path and the short path
wasn't the last directory component, then the `files_in_dir` method returned a
relative path of the form, because `Dir.glob` always returns long paths:
../../../../../../../Program Files/Puppet Labs/Puppet/puppet/lib/ruby/vendor_ruby/puppet/provider/exec/posix
Later the autoloader joined `Puppet[:libdir]` and the relative path above,
resulting in a non-canonical path:
C:/ProgramData/PuppetLabs/puppet/cache/lib/../../../../../../../Program Files/Puppet Labs/Puppet/puppet/lib/ruby/vendor_ruby/puppet/provider/exec/posix.rb
The autoloader then called `Kernel.load` with the long path:
C:/Program Files/Puppet Labs/Puppet/puppet/lib/ruby/vendor_ruby/puppet/provider/exec/posix.rb
Which eventually tried to require relatively uniquefile using a long path.
However, that would fail because it had already been required using its 8.3 path:
irb(main):002:0> $LOADED_FEATURES.grep(/\/uniquefile/)
=> ["C:/PROGRA~1/PUPPET~1/Puppet/puppet/lib/ruby/vendor_ruby/puppet/file_system/uniquefile.rb"]
This issue wasn't an issue prior to require_relative, because we called `require
'puppet/util/uniquefile'` and ruby knew it had already required that file.
This commit modifies `files_in_dir` to convert the base `dir` to a long path, so
that `dir` is always a prefix for the globbed child.
joshcooper
force-pushed
the
expand_long_path
branch
from
48b58a3 to
ca7fac8
Compare
|
|
|
jenkins please test this on redhat7-64a,windows2019-64a |
|
To verify this fix on Windows:
|
|
Something went weird here and it seems that Jenkins took your last comment as the test trigger string (probably due to closing/reopening the PR): I'm just gonna kick things off again. |
|
jenkins please test this on redhat7-64a,windows2019-64a |
|
@joshcooper I cannot reproduce this with the method you suggested. I'm getting the following warnings, but no failures: Microsoft Windows [Version 6.3.9600]
(c) 2013 Microsoft Corporation. All rights reserved.
C:\Users\Administrator>cmd /c start /w msiexec /qn /i https://downloads.puppetlabs.com/windows/puppet7/puppet-agent-7.11.0-x64.msi
C:\Users\Administrator>cd C:\PROGRA~1\Puppet~1\Puppet\bin
C:\PROGRA~1\PUPPET~1\Puppet\bin>puppet --version
C:/Program Files/Puppet Labs/Puppet/puppet/lib/ruby/vendor_ruby/facter/util/file_helper.rb:9: warning: already initialized constant Class::DEBUG_MESSAGE
C:/PROGRA~1/PUPPET~1/Puppet/puppet/lib/ruby/vendor_ruby/facter/util/file_helper.rb:9: warning: previous definition of DEBUG_MESSAGE was here
C:/PROGRA~1/PUPPET~1/Puppet/puppet/lib/ruby/vendor_ruby/facter/framework/core/options/options_validator.rb:5: warning: already initialized constant Facter::OptionsValidator::INVALID_PAIRS_RULES
C:/Program Files/Puppet Labs/Puppet/puppet/lib/ruby/vendor_ruby/facter/framework/core/options/options_validator.rb:5: warning: previous definition of INVALID_PAIRS_RULES was here
C:/PROGRA~1/PUPPET~1/Puppet/puppet/lib/ruby/vendor_ruby/facter/framework/core/options/options_validator.rb:15: warning: already initialized constant Facter::OptionsValidator::DUPLICATED_OPTIONS_RULES
C:/Program Files/Puppet Labs/Puppet/puppet/lib/ruby/vendor_ruby/facter/framework/core/options/options_validator.rb:15: warning: previous definition of DUPLICATED_OPTIONS_RULES was here
C:/PROGRA~1/PUPPET~1/Puppet/puppet/lib/ruby/vendor_ruby/facter/framework/core/options/options_validator.rb:18: warning: already initialized constant Facter::OptionsValidator::LOG_LEVEL
C:/Program Files/Puppet Labs/Puppet/puppet/lib/ruby/vendor_ruby/facter/framework/core/options/options_validator.rb:18: warning: previous definition of LOG_LEVEL was here
7.11.0
C:\PROGRA~1\PUPPET~1\Puppet\bin>Installed the build from this PR and I'm getting the same warnings (this is on a Windows 2012R2 box). |
|
Sorry @GabrielNagy, my bad. You'll have to trigger type/provider loading such as: With the fix, there are still a few warnings coming from facter due to constant redefinitions, but the command should list users on the system. |
GabrielNagy
approved these changes
Oct 1, 2021
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.
Previously, puppet failed to load if the current working directory was a short
Windows path. This was due to trying to require
Puppet::Util::Uniquefileusingits long and short path. This regression was introduced with the move to
require_relative.
The
Autoload.files_in_dirmethod is expected to return relative paths that arecontained within the
dirargument. For example, returning an array containingpuppet/provider/exec/posix.However, if the
dirargument contained a short Windows path and the short pathwasn't the last directory component, then the
files_in_dirmethod returned arelative path of the form, because
Dir.globalways returns long paths:Later the autoloader joined
Puppet[:libdir]and the relative path above,resulting in a non-canonical path:
The autoloader then called
Kernel.loadwith the long path:Which eventually tried to require relatively uniquefile using a long path.
However, that would fail because it had already been required using its 8.3 path:
This issue wasn't an issue prior to require_relative, because we called
require 'puppet/util/uniquefile'and ruby knew it had already required that file.This commit modifies
files_in_dirto convert the basedirto a long path, sothat
diris always a prefix for the globbed child.