Fix setting Path for Windows #1876

alex-slynko · 2014-08-21T10:31:02Z

ENV['PATH'] is broken after changing path in windows. It replaces Path in Ruby to path with system variables.
That leads to broken recipes that use executables in System32.

Variant 1 is proposed in this pull request is to replace all systemroot variables to new ones.
Variant 2 would be to change ENV['Path'] separately from changing actual path.

adamedx · 2014-08-21T13:55:22Z

@alex-slynko-wonga, can you give an example recipe that exhibits bad behavior without this change and describe the resulting behavior? Also, is this a regression from a particular Chef release, or has this behavior always been present?

adamedx · 2014-08-21T13:58:58Z

lib/chef/provider/env/windows.rb

@@ -33,7 +33,9 @@ def create_env
          end
          obj.variablevalue = @new_resource.value
          obj.put_
-          ENV[@new_resource.key_name] = @new_resource.value
+          value =  @new_resource.value
+          value.gsub!(/%SystemRoot%/, ENV['SystemRoot']) if @new_resource.key_name.upcase == 'PATH'


Theoretically PATH could be specified as lowercase path, or mixed case, correct? So we'd need to handle that scenario. Same for %SystemRoot% in the environment variable value. And there may be parts of the path that contain other environment variables (e.g. %systemdrive%, %windir%, %programfiles%, etc.).

yes, that is why I check key_name.upcase
windows_path recipe uses 'Path' at the moment.

And as for other variables - can be done.

alex-slynko · 2014-08-21T15:09:27Z

Hi @adamedx
Here is the issue.
We are including some directory in the path using windows_path recipe.
Then in next recipe we are installing other package using msiexec.
Result is

 ---- Begin output of msiexec /qn /i "c:\chef\cache\node-v0.10.18-x64.msi" ----
 172.30.183.24
STDOUT:
172.30.183.24
 STDERR: 'msiexec' is not recognized as an internal or external command,

That happens because in mixlib-shellout there is a search for executable candidates using ENV['PATH']

And this happened only with latest version.

alex-slynko · 2014-08-21T15:33:28Z

fixing tests for replacing multiple variables

adamedx · 2014-08-21T15:52:09Z

@alex-slynko-wonga very interesting. Can you see if reverting to an old version of the windows cookbook, 1.34.0, resolves this? We had some fun with 'path' due to a regression in Chef that was fixed in 11.14.6.
Also, can you confirm that you reproduce this in Chef 11.14.6?

alex-slynko · 2014-08-21T17:00:06Z

@Adamex that breaks only 11.14.6
Windows 1.34.0 doesn't help

alex-slynko · 2014-08-25T16:48:15Z

@adamedx happens with 11.14.2 and 11.14.6.
Windows 1.34.0 doesn't help, just breaks it earlier.
I believe that happened because of saving changed ENV variable.

jaym · 2014-09-08T19:43:24Z

Verified this is an issue.

Ruby does not look like it expands the variables, and the new representation does not expand them for us. Our implementation needs to take care of this in https://github.com/opscode/chef/blob/master/lib/chef/provider/env/windows.rb

If we want to get this out to people ASAP, we can patch this in the windows cookbook for the meantime.

lamont-granquist · 2014-09-08T19:55:05Z

So it looks like mixlib-shellout has had this bug for > 2 years?

https://github.com/opscode/mixlib-shellout/blob/master/lib/mixlib/shellout/windows.rb#L290

It seems like that's the appropriate place to fix it?

lamont-granquist · 2014-09-08T20:07:29Z

^^^ okay so it looks like that is incorrect, but it wasn't clear.

the behavior in pry looks like ENV['PATH'] has its %-variables expanded by ruby on startup and then the defacto 'API' expects any changes to the ENV['PATH'] to also be expanded.

it smells like somewhere in ruby there should be a function to do that %-variable expansion and we should try to reuse that if possible, also we can't possibly be the only people to run into this... (any relevant stackoverflow questions?)

btm · 2014-09-08T20:08:48Z

It looks like reading ENV['PATH'] in Ruby returns the same format as echo $env:PATH in powershell (no embedded variables):

PS C:\Users\btm_000> echo $env:PATH
C:\Program Files (x86)\NVIDIA Corporation\PhysX\Common;C:\ansicon;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wb
em;C:\WINDOWS\System32\WindowsPowerShell\v1.0\;C:\Program Files\Amazon\AWSCLI\;C:\Program Files (x86)\OpenSSH\bin;C:\Pr
ogram Files (x86)\Windows Live\Shared;C:\Program Files (x86)\Git\cmd;C:\HashiCorp\Vagrant\bin;C:\Program Files (x86)\7-
Zip;C:\Program Files (x86)\WiX Toolset v3.8\bin;C:\Program Files\Oracle\VirtualBox;C:\opscode\chef\bin;C:\opscode\chef\
embedded\bin;C:\opscode\chef\bin;C:\opscode\chef\embedded\bin;C:\Ruby193\bin

irb(main):003:0> ENV['PATH']
=> "C:\\Program Files (x86)\\NVIDIA Corporation\\PhysX\\Common;C:\\ansicon;C:\\WINDOWS\\system32;C:\\WINDOWS;C:\\WINDOWS
\\System32\\Wbem;C:\\WINDOWS\\System32\\WindowsPowerShell\\v1.0\\;C:\\Program Files\\Amazon\\AWSCLI\\;C:\\Program Files
(x86)\\OpenSSH\\bin;C:\\Program Files (x86)\\Windows Live\\Shared;C:\\Program Files (x86)\\Git\\cmd;C:\\HashiCorp\\Vagra
nt\\bin;C:\\Program Files (x86)\\7-Zip;C:\\Program Files (x86)\\WiX Toolset v3.8\\bin;C:\\Program Files\\Oracle\\Virtual
Box;C:\\opscode\\chef\\bin;C:\\opscode\\chef\\embedded\\bin;C:\\opscode\\chef\\bin;C:\\opscode\\chef\\embedded\\bin;C:\\
Ruby193\\bin"

But the data we get back from WmiLite has embedded variables:

irb(main):002:0> WmiLite::Wmi.new.query("select * from Win32_Environment where name = 'PATH'")
=> [#<WmiLite::Wmi::Instance:0x2590158 @wmi_ole_object=#<WIN32OLE:0x2590170>, @property_map={"caption"=>"<SYSTEM>\\Path"
, "description"=>"<SYSTEM>\\Path", "installdate"=>nil, "name"=>"Path", "status"=>"OK", "systemvariable"=>true, "username
"=>"<SYSTEM>", "variablevalue"=>"C:\\Program Files (x86)\\NVIDIA Corporation\\PhysX\\Common;C:\\ansicon;%SystemRoot%\\sy
stem32;%SystemRoot%;%SystemRoot%\\System32\\Wbem;%SYSTEMROOT%\\System32\\WindowsPowerShell\\v1.0\\;C:\\Program Files\\Am
azon\\AWSCLI\\;C:\\Program Files (x86)\\OpenSSH\\bin;C:\\Program Files (x86)\\Windows Live\\Shared;C:\\Program Files (x8
6)\\Git\\cmd;C:\\HashiCorp\\Vagrant\\bin;C:\\Program Files (x86)\\7-Zip;C:\\Program Files (x86)\\WiX Toolset v3.8\\bin;C
:\\Program Files\\Oracle\\VirtualBox;C:\\opscode\\chef\\bin;C:\\opscode\\chef\\embedded\\bin"}>, #<WmiLite::Wmi::Instanc
e:0x257f238 @wmi_ole_object=#<WIN32OLE:0x257f268>, @property_map={"caption"=>"BTM-DESKTOP\\btm_000\\Path", "description"
=>"BTM-DESKTOP\\btm_000\\Path", "installdate"=>nil, "name"=>"Path", "status"=>"OK", "systemvariable"=>false, "username"=
>"BTM-DESKTOP\\btm_000", "variablevalue"=>"C:\\opscode\\chef\\bin;C:\\opscode\\chef\\embedded\\bin;C:\\Ruby193\\bin"}>]

And the problem is when we modify that and then save it to ENV['PATH'].

If I set something like $env:PATH = "$env:PATH;%SYSTEMROOT%\foo" in powershell, then %SYSTEMROOT% isn't evaluated, but I think that's user error. Setting $env:PATH = "$env:PATH;$env:SYSTEMROOT\foo" works fine.

So I think fixing mixlib-shellout#which is probably a separate bug, and we do need to make sure we don't save these unexpanded variables back to ENV['PATH'] but I'm feeling that we need to do it without hardcoding a list of potential variable names.

jaym · 2014-09-08T20:59:13Z

lib/chef/provider/env/windows.rb

+        private
+
+        def expand_path(path)
+          system_vars = %w(HomeDrive HomePath ProgramFiles SystemDirectory SystemDrive SystemRoot Temp Tmp UserProfile WinDir)


We do not need to limit to these

alex-slynko · 2014-09-09T14:39:13Z

@adamedx still broken in 11.16 :(

sbotman · 2014-09-09T14:51:44Z

Indeed, just ran into this issue also. See #2016 for details.

btm · 2014-09-09T15:40:06Z

The fix that @jdmundrawala is working on in the windows cookbook should be released this week which will work around the bug: chef-boneyard/windows#112

Fixing this will close #2009 as well. I'm leaving it open as an "issue" for now since this is an individual pull request.

jaym · 2014-09-10T01:55:50Z

@btm @adamedx I've updated this PR to use the windows api:
#2024

btm · 2014-10-13T20:28:13Z

This was fixed by #2024. Anyone experiencing any environment issues should be sure they replicate on 11.16.4.

chef-supermarket added the Signed CLA label Aug 21, 2014

adamedx reviewed Aug 21, 2014
View reviewed changes

chef-supermarket added Signed CLA and removed Signed CLA labels Aug 21, 2014

alex-slynko closed this Aug 21, 2014

Fix setting Path for Windows

0d9f887

alex-slynko reopened this Aug 21, 2014

chef-supermarket added the Signed CLA label Aug 21, 2014

jjasghar force-pushed the master branch from de3e248 to 7e93d3f Compare September 5, 2014 17:00

p-palanisamy mentioned this pull request Sep 8, 2014

powershell_script failing with "powershell.exe"' is not recognized as an internal or external command #2009

Closed

sersut added the Windows label Sep 8, 2014

jaym reviewed Sep 8, 2014
View reviewed changes

jaym mentioned this pull request Sep 8, 2014

Expand environment variables in path chef-boneyard/windows#112

Merged

sbotman mentioned this pull request Sep 9, 2014

STDERR: 'msiexec' is not recognized as an internal or external command #2016

Closed

jaym mentioned this pull request Sep 10, 2014

Jdmundrawala/windows env path #2024

Merged

btm closed this Oct 13, 2014

thommay added Area: Windows and removed Windows labels Sep 22, 2017

chef locked and limited conversation to collaborators Nov 16, 2017

tas50 added Platform: Windows and removed Area: Windows labels Jan 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix setting Path for Windows #1876

Fix setting Path for Windows #1876

alex-slynko commented Aug 21, 2014

adamedx commented Aug 21, 2014

adamedx Aug 21, 2014

alex-slynko Aug 21, 2014

alex-slynko commented Aug 21, 2014

alex-slynko commented Aug 21, 2014

adamedx commented Aug 21, 2014

alex-slynko commented Aug 21, 2014

alex-slynko commented Aug 25, 2014

jaym commented Sep 8, 2014

lamont-granquist commented Sep 8, 2014

lamont-granquist commented Sep 8, 2014

btm commented Sep 8, 2014

jaym Sep 8, 2014

alex-slynko commented Sep 9, 2014

sbotman commented Sep 9, 2014

btm commented Sep 9, 2014

jaym commented Sep 10, 2014

btm commented Oct 13, 2014

Fix setting Path for Windows #1876

Fix setting Path for Windows #1876

Conversation

alex-slynko commented Aug 21, 2014

adamedx commented Aug 21, 2014

adamedx Aug 21, 2014

Choose a reason for hiding this comment

alex-slynko Aug 21, 2014

Choose a reason for hiding this comment

alex-slynko commented Aug 21, 2014

alex-slynko commented Aug 21, 2014

adamedx commented Aug 21, 2014

alex-slynko commented Aug 21, 2014

alex-slynko commented Aug 25, 2014

jaym commented Sep 8, 2014

lamont-granquist commented Sep 8, 2014

lamont-granquist commented Sep 8, 2014

btm commented Sep 8, 2014

jaym Sep 8, 2014

Choose a reason for hiding this comment

alex-slynko commented Sep 9, 2014

sbotman commented Sep 9, 2014

btm commented Sep 9, 2014

jaym commented Sep 10, 2014

btm commented Oct 13, 2014