(PUP-6134) Explicitly locate gemcmd windows

[tkishel]

On Windows:

Puppet is unable to install a ruby gem anywhere other than Puppet's ruby.
Caused by Puppet prepending its paths (including our gem) to PATH.
This fix removes Puppet's paths from PATH when setting :gemcmd.

(A larger-scale solution would involve a path/target parameter to Package.)

[puppetcla]

CLA signed by all contributors.

[tkishel]

Resubmitted this PR to recover from a rebase error with the previous PR.

[geoffnichols]

Replaces #5460.

[Iristyle]

The method of constructing the PATH is a bit imprecise - a few problems I see:

• What if a user relocated the installation to something without puppet in it?
• What if the Ruby installation path contains puppet in it? (admittedly less likely)

We can add a little bit of code to find the actual current running Ruby like this:
https://stackoverflow.com/a/25929455

I can help out with putting that in the right spot within Puppet.

[tkishel]

Thanks, I made an assumption that users might change INSTALLDIR from the default of C:\Program Files\Puppet Labs\Puppet but not change that last Puppet directory.

Since we already find and set RUBY_DIR, could we reuse it as follows?

if Puppet.features.microsoft_windows?
  ruby_bin_dir = Puppet::Util.get_env('RUBY_DIR') + '\bin'
  path_array = Puppet::Util.get_env('PATH').split(File::PATH_SEPARATOR)
  path_array.reject! {|p| p == ruby_bin_dir}
  path_without_puppet_ruby_bin_dir = path_array.join(File::PATH_SEPARATOR)
  commands :gemcmd => Puppet::Util.withenv({:PATH => path_without_puppet_ruby_bin_dir}, :windows) { which('gem') }
else
  commands :gemcmd => 'gem'
end

[tkishel]

But if you make that available in another spot, I'll use it.

[tkishel]

I have rebased onto 4.10.x but have not been able to clear the AppVeyor error. I thought it might be related to line 103 of spec/integration/type/package_spec.rb specifying the provider as :gem which, with this commit, will find no gem command if there is no gem in the path (outside of ours, which should be specified via the puppet_gemprovider) but changing it to :puppet_gemresults in another error when running bundle exec rake spec:

Puppet::Error: 
Could not list gems: Execution of '/opt/puppetlabs/puppet/bin/gem list --local' returned 1: 
/Users/tom.kishel/.rvm/gems/ruby-2.3.0/gems/bundler-1.15.4/lib/bundler/spec_set.rb:87:in 
`block in materialize': Could not find rake-10.1.1 in any of the sources (Bundler::GemNotFound)

A similar error occurs when running bundle exec rake spec in Windows. In both cases, running that same gem list or gem.cmd list command actually lists gems and does not set an error in $?

I will need help addressing that. Note that manually testing this commit on Windows with puppet apply correctly manages a gem package, and puppet resource package correctly lists the gems of the non-puppet vendored ruby.

[theshanx]

@Iristyle @geoffnichols any advice on how we can troubleshoot the AppVeyor check or who we can ask to help?

[tkishel]

The AppVeyor Windows environment, unlike the Puppet Agent Windows environment, does not define a RUBY_DIR environment variable, causing the following error:

Failure/Error: ruby_bin_dir = Puppet::Util.get_env('RUBY_DIR') + '\bin'                                                                           
                                                                                                                                                  
Puppet::Error:                                                                                                                                    
  Could not autoload puppet/type/package: Could not autoload puppet/provider/package/gem: undefined method `+' for nil:NilClass                   

Resolved for AppVeyor by checking for nil before concatenation.

Note that RUBY_DIR should always be defined and prepended to PATH in a Puppet Agent Windows environment, via environment.bat: it's that prepending that causes the problem documented in PUP-6134. Also note that if RUBY_DIR is not defined, this provider would not be the first point of failure!

[melissa]

@caseywilliams @branan is this still an issue with the updates that happened to the MSI paths in puppet 6?

[mcdonaldseanp]

@melissa yeah, nothing about the path updates would have changed this. It would still be an issue.

[melissa]

@Iristyle could you take another look at this?

[tkishel]

I understand we may consider this to be a breaking change, as we may have users using this bug as a feature to manage puppet gems on Windows (normally managed via the puppet_gem provider) via the gem provider?

[jtappa]

Could someone from @puppetlabs/windows please take a look at this?

[shrug]

Adding @puppetlabs/night-s-watch to review

[joshcooper]

@tkishel since you added targetable package providers to 5.5.16 and up (which applies to the gem provider), I believe we can close this PR and the ticket PUP-6134, correct?

[joshcooper]

Looks like this is superseded by the targetable provider work in PUP-6488, so I'm going to close this PR. Please reopen if I've got that wrong.