Skip to content

Ticket/2.7.x/9636 onlyif unless windows exec parameters#168

Merged
jhelwig merged 3 commits intopuppetlabs:2.7.xfrom
joshcooper:ticket/2.7.x/9636-onlyif-unless-windows-exec
Oct 13, 2011
Merged

Ticket/2.7.x/9636 onlyif unless windows exec parameters#168
jhelwig merged 3 commits intopuppetlabs:2.7.xfrom
joshcooper:ticket/2.7.x/9636-onlyif-unless-windows-exec

Conversation

@joshcooper
Copy link
Contributor

This patch series fixes issues with Puppet::Util.which and Puppet::Util.execute on Windows. In doing so it resolves, issue #9636 whereby the onlyif and unless exec parameters were not working on windows.

Previously, Puppet::Util.which could only resolve executables that
were located in the first PATH directory entry, typically
c:\windows\system32.

This was introduced in 0258096 due to the inner loop overwriting the
'bin' variable that was used in the outer loop. And it wasn't noticed
during testing, because I was always using executables from
c:\windows\system32, e.g. cmd.exe.

This commit changes that the inner loop to use a different variable so
as to not clobber the 'bin' variable. It also updates the spec test.
Previously, Puppet::Util.execute did not set $CHILD_STATUS on Windows,
including the Windows exec provider. This caused problems for the
'onlyif' and 'unless' exec parameters, among others.

When executing processes, the general approach on Unix, is to map
stdin, stdout, stderr of the child process to /dev/null or a file
descriptor, depending on the options specified, e.g. 'squelch',
'combine'. The use of a file, as opposed to creating a pipe, avoids
having to create reader threads, and the potential for deadlock that
can occur.

On Windows, we take the same approach. But this is complicated by the
lack of 'fork' and that $CHILD_STATUS is read-only. Ideally, we could
just use Process.spawn (as this allows redirection of stdin, etc), but
the method is only available in ruby 1.9.

So instead we use the win32/process gem, which uses the native Win32
API CreateProcess method. And the fix is to create a subprocess that
simply exits with the desired exit status, thereby setting
$CHILD_STATUS for us.

Also as part of this commit, the child_status variable was renamed to
exit_status, since that is what it is really, and to avoid confusion
with $CHILD_STATUS, which is something else entirely.
Previously, the Windows exec provider had incorrect logic for
resolving an executable using a hard-coded list of extensions,
e.g. '.cmd' wasn't in the list.

Recently, Puppet::Util.which was modified to perform path resolution
taking into account the PATHEXT environment variable on Windows. See
0258096. As a result, the duplicate
logic in the Windows exec provider has been removed.
@jhelwig jhelwig merged commit 5721ab9 into puppetlabs:2.7.x Oct 13, 2011
hlindberg pushed a commit to hlindberg/puppet that referenced this pull request Oct 16, 2014
fix hiera.yml for 1.9.x yaml parser
melissa pushed a commit to melissa/puppet that referenced this pull request Mar 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants