From 38263ac768349bf0eab56194aa70f4a78fb664af Mon Sep 17 00:00:00 2001 From: Nick Berard Date: Fri, 1 Jul 2016 17:31:05 -0700 Subject: [PATCH 1/2] After the repository is added, the ${::apt::sources_list_d}/${sources_list_d_filename} file is created as an empty file. The unless condition of Exec["add-apt-repository-${name}"] calls test -s, which returns 1 if the file is empty. Because the file is empty, the unless condition is never true and the repository is added on every execution. This commit replaces the -s test condition with -f, which is true if the file exists or false otherwise. --- manifests/ppa.pp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/ppa.pp b/manifests/ppa.pp index dfa3d08..6b025d7 100644 --- a/manifests/ppa.pp +++ b/manifests/ppa.pp @@ -48,7 +48,7 @@ define apt::ppa( exec { "add-apt-repository-${name}": environment => $_proxy_env, command => "/usr/bin/add-apt-repository ${options} ${name}", - unless => "/usr/bin/test -s ${::apt::sources_list_d}/${sources_list_d_filename}", + unless => "/usr/bin/test -f ${::apt::sources_list_d}/${sources_list_d_filename}", user => 'root', logoutput => 'on_failure', notify => Class['apt::update'], From fcdd54de1d041a9f2d2a0b13bf3d64ccbea49eed Mon Sep 17 00:00:00 2001 From: Nick Berard Date: Tue, 5 Jul 2016 17:30:23 -0700 Subject: [PATCH 2/2] Replaced `-s` with `-f` in ppa rspec tests See 38263ac768349bf0eab56194aa70f4a78fb664af. The previous commit makes the tests fail. --- spec/defines/ppa_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/defines/ppa_spec.rb b/spec/defines/ppa_spec.rb index 74b52ea..a4c3014 100644 --- a/spec/defines/ppa_spec.rb +++ b/spec/defines/ppa_spec.rb @@ -21,7 +21,7 @@ describe 'apt::ppa' do it { is_expected.to contain_exec('add-apt-repository-ppa:needs/such.substitution/wow+type').that_notifies('Class[Apt::Update]').with({ :environment => [], :command => '/usr/bin/add-apt-repository -y ppa:needs/such.substitution/wow+type', - :unless => '/usr/bin/test -s /etc/apt/sources.list.d/needs-such_substitution-wow_type-natty.list', + :unless => '/usr/bin/test -f /etc/apt/sources.list.d/needs-such_substitution-wow_type-natty.list', :user => 'root', :logoutput => 'on_failure', }) @@ -44,7 +44,7 @@ describe 'apt::ppa' do it { is_expected.to contain_exec('add-apt-repository-ppa:user/foo').that_notifies('Class[Apt::Update]').with({ :environment => [], :command => '/usr/bin/add-apt-repository -y ppa:user/foo', - :unless => '/usr/bin/test -s /etc/apt/sources.list.d/user-ubuntu-foo-wily.list', + :unless => '/usr/bin/test -f /etc/apt/sources.list.d/user-ubuntu-foo-wily.list', :user => 'root', :logoutput => 'on_failure', }) @@ -83,7 +83,7 @@ describe 'apt::ppa' do it { is_expected.to contain_exec('add-apt-repository-ppa:needs/such.substitution/wow').that_notifies('Class[Apt::Update]').with({ 'environment' => [], 'command' => '/usr/bin/add-apt-repository -y ppa:needs/such.substitution/wow', - 'unless' => '/usr/bin/test -s /etc/apt/sources.list.d/needs-such_substitution-wow-natty.list', + 'unless' => '/usr/bin/test -f /etc/apt/sources.list.d/needs-such_substitution-wow-natty.list', 'user' => 'root', 'logoutput' => 'on_failure', }) @@ -123,7 +123,7 @@ describe 'apt::ppa' do it { is_expected.to contain_exec('add-apt-repository-ppa:user/bar').that_notifies('Class[Apt::Update]').with({ 'environment' => [], 'command' => '/usr/bin/add-apt-repository -y ppa:user/bar', - 'unless' => '/usr/bin/test -s /etc/apt/sources.list.d/user-bar-natty.list', + 'unless' => '/usr/bin/test -f /etc/apt/sources.list.d/user-bar-natty.list', 'user' => 'root', 'logoutput' => 'on_failure', }) @@ -160,7 +160,7 @@ describe 'apt::ppa' do it { is_expected.to contain_exec('add-apt-repository-ppa:needs/such.substitution/wow').that_notifies('Class[Apt::Update]').with({ 'environment' => [], 'command' => '/usr/bin/add-apt-repository -y ppa:needs/such.substitution/wow', - 'unless' => '/usr/bin/test -s /etc/apt/sources.list.d/needs-such_substitution-wow-natty.list', + 'unless' => '/usr/bin/test -f /etc/apt/sources.list.d/needs-such_substitution-wow-natty.list', 'user' => 'root', 'logoutput' => 'on_failure', }) @@ -201,7 +201,7 @@ describe 'apt::ppa' do it { is_expected.to contain_exec('add-apt-repository-ppa:user/foo').that_notifies('Class[Apt::Update]').with({ :environment => [], :command => '/usr/bin/add-apt-repository ppa:user/foo', - :unless => '/usr/bin/test -s /etc/apt/sources.list.d/user-foo-trusty.list', + :unless => '/usr/bin/test -f /etc/apt/sources.list.d/user-foo-trusty.list', :user => 'root', :logoutput => 'on_failure', }) @@ -235,7 +235,7 @@ describe 'apt::ppa' do it { is_expected.to contain_exec('add-apt-repository-ppa:user/foo').that_notifies('Class[Apt::Update]').with({ :environment => ['http_proxy=http://localhost:8080'], :command => '/usr/bin/add-apt-repository ppa:user/foo', - :unless => '/usr/bin/test -s /etc/apt/sources.list.d/user-foo-trusty.list', + :unless => '/usr/bin/test -f /etc/apt/sources.list.d/user-foo-trusty.list', :user => 'root', :logoutput => 'on_failure', }) @@ -269,7 +269,7 @@ describe 'apt::ppa' do it { is_expected.to contain_exec('add-apt-repository-ppa:user/foo').that_notifies('Class[Apt::Update]').with({ :environment => ['http_proxy=http://localhost:8180'], :command => '/usr/bin/add-apt-repository ppa:user/foo', - :unless => '/usr/bin/test -s /etc/apt/sources.list.d/user-foo-trusty.list', + :unless => '/usr/bin/test -f /etc/apt/sources.list.d/user-foo-trusty.list', :user => 'root', :logoutput => 'on_failure', }) @@ -303,7 +303,7 @@ describe 'apt::ppa' do it { is_expected.to contain_exec('add-apt-repository-ppa:user/foo').that_notifies('Class[Apt::Update]').with({ :environment => ['http_proxy=http://localhost:8180', 'https_proxy=https://localhost:8180'], :command => '/usr/bin/add-apt-repository ppa:user/foo', - :unless => '/usr/bin/test -s /etc/apt/sources.list.d/user-foo-trusty.list', + :unless => '/usr/bin/test -f /etc/apt/sources.list.d/user-foo-trusty.list', :user => 'root', :logoutput => 'on_failure', })