(PUP-1526) Add environment settings for package type #4928
Conversation
|
Note - this PR still only provides the |
|
CLA signed by all contributors. |
|
this replaces pr #4862 |
|
Hey @jearls, thank you for the contribution! We're going to try and pull this into our next sprint so we can get some eyes on it and hopefully get it merged. |
| newparam(:environment) do | ||
| desc "Any additional environment variables you want to set for operations | ||
| against this package. The value should be a single string of the form | ||
| `NAME=value` or an array of such strings. |
There was a problem hiding this comment.
This makes me sad, but I agree that consistency with exec is good here :(
There was a problem hiding this comment.
Yes, I had originally made this accept a hash. I guess I could try to let it accept either, but I thought that would be more confusing than just keeping it consistent.
|
I don't really like moving all the providers to use a common helper if you're not going to enable the |
branan
added
Waiting on Contributor
and removed
blocked
|
The reason I didn't add the custom environment to any other provider is because I have no expertise in any other packaging system and no experience with non-RedHat Linux OSes. I originally had just added this to only to the |
|
@jearls That makes sense. This is a useful feature for us to get in, and the work you've done building a framework for the other providers looks like it's in the right direction to me, so I'm not going to push for its removal. It looks like this has picked up some conflicts with the base branch in the interim. If you could get it rebased that'd be awesome. You'll also probably have to squash your second commit in to your first, or at the very least add the ticket number to it, in order to make our commit message checker happy. |
|
Sorry it's taken so long. I completely missed the update from July. I just rebased and force-pushed, but I don't know if I've got the spec file correct, since I haven't seen/used the "shared examples" thing before. |
This patch adds to the package type the `:settable_environment` feature and the `:environment` parameter used by that feature. `:environment` is either a string of the form "NAME=VALUE" or an array of such strings. The Puppet::Provider::Package is modified to include wrapped versions of the `execute`, `execpipe`, and `has_command` methods which include the custom environment if the calling resource defines it. This includes a required modification to the `CommandDefiner` class in Puppet::Provider to allow the executor and resolver to be overridden. All package providers are modified to use the Puppet::Provider::Package versions of `execute`, `execpipe`, and `has_command`. The `yum` package provider is updated to actually support the `environment` parameter and `:settable_environment` feature. Finally, the `spec` tests for the `package` type and `yum` provider are updated to test for the `:settable_environment` feature and the `environment` parameter.
|
once this merges I'll update the portage package provider, this is useful for overriding FEATURES and the like if needed. |
| @@ -239,6 +239,8 @@ def initialize(name, path, confiner) | |||
| @optional = false | |||
| @confiner = confiner | |||
| @custom_environment = {} | |||
| @command_resolver = Puppet::Util | |||
There was a problem hiding this comment.
The Puppet::Util module is mixed into the provider, so I don't think this is necessary.
| def command | ||
| if not @optional | ||
| @confiner.confine :exists => @path, :for_binary => true | ||
| end | ||
|
|
||
| Puppet::Provider::Command.new(@name, @path, Puppet::Util, Puppet::Util::Execution, { :failonfail => true, :combine => true, :custom_environment => @custom_environment }) | ||
| Puppet::Provider::Command.new(@name, @path, @command_resolver, @command_executor, { :failonfail => true, :combine => true, :custom_environment => @custom_environment }) |
There was a problem hiding this comment.
Similarly, I think you can just pass self here, e.g. Command.new(@name, @path, self, ...)
|
This PR hasn't been updated in some time, and I'm afraid the team won't have time to review it in the near future. Let's track future conversation in the ticket, PUP-1526, and we can re-open this PR if/when there's bandwidth to support it. Closing for now. |
|
I commented into PUP-1526. We can definitely use this feature to work around a bug in pip3.6 |
This patch adds to the package type the
:settable_environmentfeatureand the
:environmentparameter used by that feature.:environmentis either a string of the form "NAME=VALUE" or an array of such strings.
The Puppet::Provider::Package is modified to include wrapped versions
of the
execute,execpipe, andhas_commandmethods which includethe custom environment if the calling resource defines it. This
includes a required modification to the
CommandDefinerclass inPuppet::Provider to allow the executor and resolver to be overridden.
All package providers are modified to use the Puppet::Provider::Package
versions of
execute,execpipe, andhas_command.The
yumpackage provider is updated to actually support theenvironmentparameter and:settable_environmentfeature.Finally, the
spectests for thepackagetype andyumproviderare updated to test for the
:settable_environmentfeature and theenvironmentparameter.