-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clundquist/dpkg install options #9202
base: main
Are you sure you want to change the base?
Clundquist/dpkg install options #9202
Conversation
Can one of the admins verify this patch? |
if @resource[:uninstall_options] | ||
flags += @resource[:uninstall_options] | ||
end | ||
dpkg *flags, @resource[:name] | ||
end | ||
|
||
def purge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure if we want :uninstall_options
for purge
too, or if that would make things more difficult.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @clundquist-stripe thanks for your contribution. This line:
flags += @resource[:uninstall_options]
will append options without any spaces between them. Probably easier to switch flags
to an array like args
is in the install
case and then splat flags
like you're doing.
Great catch @joshcooper ! |
Thanks! Sorry forgot to mention earlier could you squash your commits and add tests? I just merged a pacman provider change that is similar to this one. |
Hi @clundquist-stripe could you squash your commits? |
can do! |
[dpkg] add uninstall_options feature [dpkg] uninstall_options, flags -> args array [dpkg] install_options, is an array, += args
f9ab915
to
f517b41
Compare
Thanks @clundquist-stripe! Could you add some unit tests to make sure install and uninstall options are getting passed through to the underying dpkg commands as expected? For example, see how this is done for yum:
puppet/spec/unit/provider/package/yum_spec.rb Line 110 in 47f7f3b
|
I have a custom
.deb
that is a little wonky and installs into/
so we have/memcached
.We need to set
--instdir
so that it ends up in the place we expect, however, currentlyinstall_options
has no effect fordpkg
.I want this to work as you expect:
Currently,
install_options
is ignored.This should honor
install_options
so that my resource works as intended.This change is difficult for me to test as we are behind on puppet versions, so I'll need to rely on CI testing and additional testing from maintainers.