From c879297112ba71f0c80e4008746a01ef81c5f334 Mon Sep 17 00:00:00 2001 From: Tim Meusel Date: Sun, 4 Aug 2024 13:13:31 +0200 Subject: [PATCH 1/2] Remove broken pacman command for installing packages This is a regression from https://github.com/puppetlabs/puppet/commit/d23d6dbfd2e578016441ab292d934b4d74ab436f During some cleanup, the command ``` pacman pacman --noconfirm --noprogressbar -Sy ``` got modified to: ``` pacman pacman --noconfirm --noprogressbar -S ``` But this isn't a valid command anymore: ``` root@bastelfreak-nb ~ # pacman --noconfirm --noprogressbar -S error: no targets specified (use -h for help) root@bastelfreak-nb ~ # ``` The idea was to prevent partial system upgrades while installing new packages. To do this, the `sync` option was removed from the installation routine. The correct fix is to remove the command completely. --- lib/puppet/provider/package/pacman.rb | 1 - spec/unit/provider/package/pacman_spec.rb | 11 ----------- 2 files changed, 12 deletions(-) diff --git a/lib/puppet/provider/package/pacman.rb b/lib/puppet/provider/package/pacman.rb index 1295a0fc5d9..2ae1562b8f0 100644 --- a/lib/puppet/provider/package/pacman.rb +++ b/lib/puppet/provider/package/pacman.rb @@ -248,7 +248,6 @@ def install_from_file else fail _("Source %{source} is not supported by pacman") % { source: source } end - pacman "--noconfirm", "--noprogressbar", "-S" pacman "--noconfirm", "--noprogressbar", "-U", source end diff --git a/spec/unit/provider/package/pacman_spec.rb b/spec/unit/provider/package/pacman_spec.rb index 57784e53dd1..3adbe5edb34 100644 --- a/spec/unit/provider/package/pacman_spec.rb +++ b/spec/unit/provider/package/pacman_spec.rb @@ -97,11 +97,6 @@ it "should install #{source} directly" do resource[:source] = source - expect(executor).to receive(:execute). - with(include("-S") & include("--noprogressbar"), no_extra_options). - ordered. - and_return("") - expect(executor).to receive(:execute). with(include("-U") & include(source), no_extra_options). ordered. @@ -120,12 +115,6 @@ end it "should install from the path segment of the URL" do - expect(executor).to receive(:execute). - with(include("-S") & include("--noprogressbar") & include("--noconfirm"), - no_extra_options). - ordered. - and_return("") - expect(executor).to receive(:execute). with(include("-U") & include(actual_file_path), no_extra_options). ordered. From cf789b7a708f51eb6f6d7db1294eaa543b7805c7 Mon Sep 17 00:00:00 2001 From: Tim Meusel Date: Sun, 4 Aug 2024 13:21:12 +0200 Subject: [PATCH 2/2] pacman: switch to long options for more readability I think the long command options makes it a bit easier to understand what's actually happening. ``` usage: pacman [...] operations: pacman {-h --help} pacman {-V --version} pacman {-D --database} pacman {-F --files} [options] [file(s)] pacman {-Q --query} [options] [package(s)] pacman {-R --remove} [options] pacman {-S --sync} [options] [package(s)] pacman {-T --deptest} [options] [package(s)] pacman {-U --upgrade} [options] use 'pacman {-h --help}' with an operation for available options ``` ``` usage: pacman {-Q --query} [options] [package(s)] options: -b, --dbpath set an alternate database location -c, --changelog view the changelog of a package -d, --deps list packages installed as dependencies [filter] -e, --explicit list packages explicitly installed [filter] -g, --groups view all members of a package group -i, --info view package information (-ii for backup files) -k, --check check that package files exist (-kk for file properties) -l, --list list the files owned by the queried package -m, --foreign list installed packages not found in sync db(s) [filter] -n, --native list installed packages only found in sync db(s) [filter] -o, --owns query the package that owns -p, --file query a package file instead of the database -q, --quiet show less information for query and search -r, --root set an alternate installation root -s, --search search locally-installed packages for matching strings -t, --unrequired list packages not (optionally) required by any package (-tt to ignore optdepends) [filter] -u, --upgrades list outdated packages [filter] -v, --verbose be verbose --arch set an alternate architecture --cachedir set an alternate package cache location --color colorize the output --config set an alternate configuration file --confirm always ask for confirmation --debug display debug messages --disable-download-timeout use relaxed timeouts for download --gpgdir set an alternate home directory for GnuPG --hookdir set an alternate hook location --logfile set an alternate log file --noconfirm do not ask for any confirmation --sysroot operate on a mounted guest system (root-only) ``` ``` usage: pacman {-S --sync} [options] [package(s)] options: -b, --dbpath set an alternate database location -c, --clean remove old packages from cache directory (-cc for all) -d, --nodeps skip dependency version checks (-dd to skip all checks) -g, --groups view all members of a package group (-gg to view all groups and members) -i, --info view package information (-ii for extended information) -l, --list view a list of packages in a repo -p, --print print the targets instead of performing the operation -q, --quiet show less information for query and search -r, --root set an alternate installation root -s, --search search remote repositories for matching strings -u, --sysupgrade upgrade installed packages (-uu enables downgrades) -v, --verbose be verbose -w, --downloadonly download packages but do not install/upgrade anything -y, --refresh download fresh package databases from the server (-yy to force a refresh even if up to date) --arch set an alternate architecture --asdeps install packages as non-explicitly installed --asexplicit install packages as explicitly installed --assume-installed add a virtual package to satisfy dependencies --cachedir set an alternate package cache location --color colorize the output --config set an alternate configuration file --confirm always ask for confirmation --dbonly only modify database entries, not package files --debug display debug messages --disable-download-timeout use relaxed timeouts for download --gpgdir set an alternate home directory for GnuPG --hookdir set an alternate hook location --ignore ignore a package upgrade (can be used more than once) --ignoregroup ignore a group upgrade (can be used more than once) --logfile set an alternate log file --needed do not reinstall up to date packages --noconfirm do not ask for any confirmation --noprogressbar do not show a progress bar when downloading files --noscriptlet do not execute the install scriptlet if one exists --overwrite overwrite conflicting files (can be used more than once) --print-format specify how the targets should be printed --sysroot operate on a mounted guest system (root-only) ``` ``` usage: pacman {-R --remove} [options] options: -b, --dbpath set an alternate database location -c, --cascade remove packages and all packages that depend on them -d, --nodeps skip dependency version checks (-dd to skip all checks) -n, --nosave remove configuration files -p, --print print the targets instead of performing the operation -r, --root set an alternate installation root -s, --recursive remove unnecessary dependencies (-ss includes explicitly installed dependencies) -u, --unneeded remove unneeded packages -v, --verbose be verbose --arch set an alternate architecture --assume-installed add a virtual package to satisfy dependencies --cachedir set an alternate package cache location --color colorize the output --config set an alternate configuration file --confirm always ask for confirmation --dbonly only modify database entries, not package files --debug display debug messages --disable-download-timeout use relaxed timeouts for download --gpgdir set an alternate home directory for GnuPG --hookdir set an alternate hook location --logfile set an alternate log file --noconfirm do not ask for any confirmation --noprogressbar do not show a progress bar when downloading files --noscriptlet do not execute the install scriptlet if one exists --print-format specify how the targets should be printed --sysroot operate on a mounted guest system (root-only) ``` --- lib/puppet/provider/package/pacman.rb | 18 +++--- spec/unit/provider/package/pacman_spec.rb | 70 +++++++++++------------ 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/lib/puppet/provider/package/pacman.rb b/lib/puppet/provider/package/pacman.rb index 2ae1562b8f0..6c93851e528 100644 --- a/lib/puppet/provider/package/pacman.rb +++ b/lib/puppet/provider/package/pacman.rb @@ -29,7 +29,7 @@ def self.yaourt? # Checks if a given name is a group def self.group?(name) - !pacman("-Sg", name).empty? + !pacman('--sync', '--groups', name).empty? rescue Puppet::ExecutionFailure # pacman returns an expected non-zero exit code when the name is not a group false @@ -74,7 +74,7 @@ def self.instances # returns a hash package => version of installed packages def self.get_installed_packages packages = {} - execpipe([command(:pacman), "-Q"]) do |pipe| + execpipe([command(:pacman), "--query"]) do |pipe| # pacman -Q output is 'packagename version-rel' regex = /^(\S+)\s(\S+)/ pipe.each_line do |line| @@ -96,7 +96,7 @@ def self.get_installed_groups(installed_packages, filter = nil) groups = {} begin # Build a hash of group name => list of packages - command = [command(:pacman), "-Sgg"] + command = [command(:pacman), '--sync', '-gg'] command << filter if filter execpipe(command) do |pipe| pipe.each_line do |line| @@ -134,14 +134,14 @@ def latest resource_name = @resource[:name] # If target is a group, construct the group version - return pacman("-Sp", "--print-format", "%n %v", resource_name).lines.map(&:chomp).sort.join(', ') if self.class.group?(resource_name) + return pacman("--sync", "--print", "--print-format", "%n %v", resource_name).lines.map(&:chomp).sort.join(', ') if self.class.group?(resource_name) # Start by querying with pacman first # If that fails, retry using yaourt against the AUR pacman_check = true begin if pacman_check - output = pacman "-Sp", "--print-format", "%v", resource_name + output = pacman "--sync", "--print", "--print-format", "%v", resource_name output.chomp else output = yaourt "-Qma", resource_name @@ -210,8 +210,8 @@ def remove_package(purge_configs = false) cmd = %w[--noconfirm --noprogressbar] cmd += uninstall_options if @resource[:uninstall_options] - cmd << "-R" - cmd << '-s' if is_group + cmd << "--remove" + cmd << '--recursive' if is_group cmd << '--nosave' if purge_configs cmd << resource_name @@ -248,7 +248,7 @@ def install_from_file else fail _("Source %{source} is not supported by pacman") % { source: source } end - pacman "--noconfirm", "--noprogressbar", "-U", source + pacman "--noconfirm", "--noprogressbar", "--update", source end def install_from_repo @@ -259,7 +259,7 @@ def install_from_repo cmd = %w[--noconfirm --needed --noprogressbar] cmd += install_options if @resource[:install_options] - cmd << "-S" << resource_name + cmd << "--sync" << resource_name if self.class.yaourt? yaourt(*cmd) diff --git a/spec/unit/provider/package/pacman_spec.rb b/spec/unit/provider/package/pacman_spec.rb index 3adbe5edb34..0971541e9b6 100644 --- a/spec/unit/provider/package/pacman_spec.rb +++ b/spec/unit/provider/package/pacman_spec.rb @@ -26,7 +26,7 @@ end it "should call pacman to install the right package quietly when yaourt is not installed" do - args = ['--noconfirm', '--needed', '--noprogressbar', '-S', resource[:name]] + args = ['--noconfirm', '--needed', '--noprogressbar', '--sync', resource[:name]] expect(provider).to receive(:pacman).at_least(:once).with(*args).and_return('') provider.install end @@ -34,7 +34,7 @@ it "should call yaourt to install the right package quietly when yaourt is installed" do without_partial_double_verification do allow(described_class).to receive(:yaourt?).and_return(true) - args = ['--noconfirm', '--needed', '--noprogressbar', '-S', resource[:name]] + args = ['--noconfirm', '--needed', '--noprogressbar', '--sync', resource[:name]] expect(provider).to receive(:yaourt).at_least(:once).with(*args).and_return('') provider.install end @@ -70,7 +70,7 @@ end it "should call pacman to install the right package quietly when yaourt is not installed" do - args = ['--noconfirm', '--needed', '--noprogressbar', '-x', '--arg=value', '-S', resource[:name]] + args = ['--noconfirm', '--needed', '--noprogressbar', '-x', '--arg=value', '--sync', resource[:name]] expect(provider).to receive(:pacman).at_least(:once).with(*args).and_return('') provider.install end @@ -78,7 +78,7 @@ it "should call yaourt to install the right package quietly when yaourt is installed" do without_partial_double_verification do expect(described_class).to receive(:yaourt?).and_return(true) - args = ['--noconfirm', '--needed', '--noprogressbar', '-x', '--arg=value', '-S', resource[:name]] + args = ['--noconfirm', '--needed', '--noprogressbar', '-x', '--arg=value', '--sync', resource[:name]] expect(provider).to receive(:yaourt).at_least(:once).with(*args).and_return('') provider.install end @@ -98,7 +98,7 @@ resource[:source] = source expect(executor).to receive(:execute). - with(include("-U") & include(source), no_extra_options). + with(include("--update") & include(source), no_extra_options). ordered. and_return("") @@ -116,7 +116,7 @@ it "should install from the path segment of the URL" do expect(executor).to receive(:execute). - with(include("-U") & include(actual_file_path), no_extra_options). + with(include("--update") & include(actual_file_path), no_extra_options). ordered. and_return("") @@ -159,7 +159,7 @@ describe "when purging" do it "should call pacman to remove the right package and configs quietly" do - args = ["/usr/bin/pacman", "--noconfirm", "--noprogressbar", "-R", "--nosave", resource[:name]] + args = ["/usr/bin/pacman", "--noconfirm", "--noprogressbar", "--remove", "--nosave", resource[:name]] expect(executor).to receive(:execute).with(args, no_extra_options).and_return("") provider.purge end @@ -167,7 +167,7 @@ describe "when uninstalling" do it "should call pacman to remove the right package quietly" do - args = ["/usr/bin/pacman", "--noconfirm", "--noprogressbar", "-R", resource[:name]] + args = ["/usr/bin/pacman", "--noconfirm", "--noprogressbar", "--remove", resource[:name]] expect(executor).to receive(:execute).with(args, no_extra_options).and_return("") provider.uninstall end @@ -175,7 +175,7 @@ it "should call yaourt to remove the right package quietly" do without_partial_double_verification do allow(described_class).to receive(:yaourt?).and_return(true) - args = ["--noconfirm", "--noprogressbar", "-R", resource[:name]] + args = ["--noconfirm", "--noprogressbar", "--remove", resource[:name]] expect(provider).to receive(:yaourt).with(*args) provider.uninstall end @@ -183,14 +183,14 @@ it "adds any uninstall_options" do resource[:uninstall_options] = ['-x', {'--arg' => 'value'}] - args = ["/usr/bin/pacman", "--noconfirm", "--noprogressbar", "-x", "--arg=value", "-R", resource[:name]] + args = ["/usr/bin/pacman", "--noconfirm", "--noprogressbar", "-x", "--arg=value", "--remove", resource[:name]] expect(executor).to receive(:execute).with(args, no_extra_options).and_return("") provider.uninstall end it "should recursively remove packages when given a package group" do allow(described_class).to receive(:group?).and_return(true) - args = ["/usr/bin/pacman", "--noconfirm", "--noprogressbar", "-R", "-s", resource[:name]] + args = ["/usr/bin/pacman", "--noconfirm", "--noprogressbar", "--remove", "--recursive", resource[:name]] expect(executor).to receive(:execute).with(args, no_extra_options).and_return("") provider.uninstall end @@ -198,19 +198,19 @@ describe "when querying" do it "should query pacman" do - expect(executor).to receive(:execpipe).with(["/usr/bin/pacman", '-Q']) - expect(executor).to receive(:execpipe).with(["/usr/bin/pacman", '-Sgg', 'package']) + expect(executor).to receive(:execpipe).with(["/usr/bin/pacman", '--query']) + expect(executor).to receive(:execpipe).with(["/usr/bin/pacman", '--sync', '-gg', 'package']) provider.query end it "should return the version" do expect(executor).to receive(:execpipe). - with(["/usr/bin/pacman", "-Q"]).and_yield(< 'package', :ensure => '1.01.3-2', :provider => :pacman, }) end @@ -228,8 +228,8 @@ describe 'when querying a group' do before :each do - expect(executor).to receive(:execpipe).with(['/usr/bin/pacman', '-Q']).and_yield('foo 1.2.3') - expect(executor).to receive(:execpipe).with(['/usr/bin/pacman', '-Sgg', 'package']).and_yield('package foo') + expect(executor).to receive(:execpipe).with(['/usr/bin/pacman', '--query']).and_yield('foo 1.2.3') + expect(executor).to receive(:execpipe).with(['/usr/bin/pacman', '--sync', '-gg', 'package']).and_yield('package foo') end it 'should warn when allow_virtual is false' do @@ -248,14 +248,14 @@ describe "when determining instances" do it "should retrieve installed packages and groups" do - expect(described_class).to receive(:execpipe).with(["/usr/bin/pacman", '-Q']) - expect(described_class).to receive(:execpipe).with(["/usr/bin/pacman", '-Sgg']) + expect(described_class).to receive(:execpipe).with(["/usr/bin/pacman", '--query']) + expect(described_class).to receive(:execpipe).with(["/usr/bin/pacman", '--sync', '-gg']) described_class.instances end it "should return installed packages" do - expect(described_class).to receive(:execpipe).with(["/usr/bin/pacman", '-Q']).and_yield(StringIO.new("package1 1.23-4\npackage2 2.00\n")) - expect(described_class).to receive(:execpipe).with(["/usr/bin/pacman", '-Sgg']).and_yield("") + expect(described_class).to receive(:execpipe).with(["/usr/bin/pacman", '--query']).and_yield(StringIO.new("package1 1.23-4\npackage2 2.00\n")) + expect(described_class).to receive(:execpipe).with(["/usr/bin/pacman", '--sync', '-gg']).and_yield("") instances = described_class.instances expect(instances.length).to eq(2) @@ -274,11 +274,11 @@ end it "should return completely installed groups with a virtual version together with packages" do - expect(described_class).to receive(:execpipe).with(["/usr/bin/pacman", '-Q']).and_yield(< true, :combine => true, :custom_environment => {}}).and_raise(Puppet::ExecutionFailure, 'error') + allow(executor).to receive(:execute).with(['/usr/bin/pacman', '--sync', '--groups', 'git'], {:failonfail => true, :combine => true, :custom_environment => {}}).and_raise(Puppet::ExecutionFailure, 'error') expect(described_class.group?('git')).to eq(false) end it 'should return false on empty pacman output' do - allow(executor).to receive(:execute).with(['/usr/bin/pacman', '-Sg', 'git'], {:failonfail => true, :combine => true, :custom_environment => {}}).and_return('') + allow(executor).to receive(:execute).with(['/usr/bin/pacman', '--sync', '--groups', 'git'], {:failonfail => true, :combine => true, :custom_environment => {}}).and_return('') expect(described_class.group?('git')).to eq(false) end it 'should return true on non-empty pacman output' do - allow(executor).to receive(:execute).with(['/usr/bin/pacman', '-Sg', 'vim-plugins'], {:failonfail => true, :combine => true, :custom_environment => {}}).and_return('vim-plugins vim-a') + allow(executor).to receive(:execute).with(['/usr/bin/pacman', '--sync', '--groups', 'vim-plugins'], {:failonfail => true, :combine => true, :custom_environment => {}}).and_return('vim-plugins vim-a') expect(described_class.group?('vim-plugins')).to eq(true) end end @@ -403,13 +403,13 @@ let(:groups) { [['foo package1'], ['foo package2'], ['bar package3'], ['bar package4'], ['baz package5']] } it 'should raise an error on non-zero pacman exit without a filter' do - expect(executor).to receive(:open).with('| /usr/bin/pacman -Sgg 2>&1').and_return('error!') + expect(executor).to receive(:open).with('| /usr/bin/pacman --sync -gg 2>&1').and_return('error!') expect(Puppet::Util::Execution).to receive(:exitstatus).and_return(1) expect { described_class.get_installed_groups(installed_packages) }.to raise_error(Puppet::ExecutionFailure, 'error!') end it 'should return empty groups on non-zero pacman exit with a filter' do - expect(executor).to receive(:open).with('| /usr/bin/pacman -Sgg git 2>&1').and_return('') + expect(executor).to receive(:open).with('| /usr/bin/pacman --sync -gg git 2>&1').and_return('') expect(Puppet::Util::Execution).to receive(:exitstatus).and_return(1) expect(described_class.get_installed_groups(installed_packages, 'git')).to eq({}) end @@ -417,7 +417,7 @@ it 'should return empty groups on empty pacman output' do pipe = double() expect(pipe).to receive(:each_line) - expect(executor).to receive(:open).with('| /usr/bin/pacman -Sgg 2>&1').and_yield(pipe).and_return('') + expect(executor).to receive(:open).with('| /usr/bin/pacman --sync -gg 2>&1').and_yield(pipe).and_return('') expect(Puppet::Util::Execution).to receive(:exitstatus).and_return(0) expect(described_class.get_installed_groups(installed_packages)).to eq({}) end @@ -427,7 +427,7 @@ pipe_expectation = receive(:each_line) groups.each { |group| pipe_expectation = pipe_expectation.and_yield(*group) } expect(pipe).to pipe_expectation - expect(executor).to receive(:open).with('| /usr/bin/pacman -Sgg 2>&1').and_yield(pipe).and_return('') + expect(executor).to receive(:open).with('| /usr/bin/pacman --sync -gg 2>&1').and_yield(pipe).and_return('') expect(Puppet::Util::Execution).to receive(:exitstatus).and_return(0) expect(described_class.get_installed_groups(installed_packages)).to eq({'foo' => 'package1 1.0, package2 2.0'}) end