From 8a92ca64c1b19c1c759ff2afd9fc9c1ae1a6b63c Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Fri, 12 Feb 2016 12:56:39 -0800 Subject: extend deprecation warnings more broadly Cookbook shadowing has been deprecated since 2011 in Chef 0.10.4 https://github.com/chef/chef/commit/5a9fee8edaede311ac03a15d52fbc66dad83b576 That warning has only been emitted when using 'knife cookbook upload' (and only that command). This extends that deprecation warning more broadly into all consumers of Chef::CookbookLoader that have been using that feature. --- lib/chef/cookbook_loader.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/chef/cookbook_loader.rb b/lib/chef/cookbook_loader.rb index 9bab6f01ce..977c65211a 100644 --- a/lib/chef/cookbook_loader.rb +++ b/lib/chef/cookbook_loader.rb @@ -66,11 +66,19 @@ class Chef merged_cookbook_paths end + def warn_about_cookbook_shadowing + unless merged_cookbooks.empty? + Chef::Log.deprecation "The cookbooks: #{merged_cookbooks.join(', ')} exist in multiple places in your cookbook_path. " + + "A composite version has been compiled. In a future version of Chef this behavior will be removed." + end + end + def load_cookbooks preload_cookbooks @loaders_by_name.each do |cookbook_name, _loaders| load_cookbook(cookbook_name) end + warn_about_cookbook_shadowing @cookbooks_by_name end -- cgit v1.2.1 From 23e71b551db1908603345ff196e0dddeff01ba4f Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Fri, 12 Feb 2016 13:09:23 -0800 Subject: might be singular --- lib/chef/cookbook_loader.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/chef/cookbook_loader.rb b/lib/chef/cookbook_loader.rb index 977c65211a..4668521817 100644 --- a/lib/chef/cookbook_loader.rb +++ b/lib/chef/cookbook_loader.rb @@ -68,7 +68,7 @@ class Chef def warn_about_cookbook_shadowing unless merged_cookbooks.empty? - Chef::Log.deprecation "The cookbooks: #{merged_cookbooks.join(', ')} exist in multiple places in your cookbook_path. " + + Chef::Log.deprecation "The cookbook(s): #{merged_cookbooks.join(', ')} exist in multiple places in your cookbook_path. " + "A composite version has been compiled. In a future version of Chef this behavior will be removed." end end -- cgit v1.2.1 From 5959add42e911e14918c41d0212f6ba8e0425d11 Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Fri, 12 Feb 2016 14:10:29 -0800 Subject: fix specs --- lib/chef/cookbook_loader.rb | 2 ++ spec/unit/cookbook_loader_spec.rb | 11 ++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/chef/cookbook_loader.rb b/lib/chef/cookbook_loader.rb index 4668521817..c0fd515bf2 100644 --- a/lib/chef/cookbook_loader.rb +++ b/lib/chef/cookbook_loader.rb @@ -85,6 +85,8 @@ class Chef def load_cookbook(cookbook_name) preload_cookbooks + return @cookbooks_by_name[cookbook_name] if @cookbooks_by_name.has_key?(cookbook_name) + return nil unless @loaders_by_name.key?(cookbook_name.to_s) cookbook_loaders_for(cookbook_name).each do |loader| diff --git a/spec/unit/cookbook_loader_spec.rb b/spec/unit/cookbook_loader_spec.rb index 3f4a7486ce..db10a4eb4f 100644 --- a/spec/unit/cookbook_loader_spec.rb +++ b/spec/unit/cookbook_loader_spec.rb @@ -45,14 +45,23 @@ describe Chef::CookbookLoader do once. and_call_original end + expect(Chef::Log).to receive(:deprecation).with(/The cookbook\(s\): openldap exist in multiple places in your cookbook_path./) cookbook_loader.load_cookbooks end context "after loading all cookbooks" do before(:each) do + expect(Chef::Log).to receive(:deprecation).with(/The cookbook\(s\): openldap exist in multiple places in your cookbook_path./) cookbook_loader.load_cookbooks end + it "should be possible to reload all the cookbooks without triggering deprecation warnings on all of them" do + start_merged_cookbooks = cookbook_loader.merged_cookbooks + expect(Chef::Log).to receive(:deprecation).with(/The cookbook\(s\): openldap exist in multiple places in your cookbook_path./) + cookbook_loader.load_cookbooks + expect(cookbook_loader.merged_cookbooks).to eql(start_merged_cookbooks) + end + describe "[]" do it "should return cookbook objects with []" do expect(cookbook_loader[:openldap]).to be_a_kind_of(Chef::CookbookVersion) @@ -98,7 +107,6 @@ describe Chef::CookbookLoader do describe "referencing cookbook files" do it "should find all the cookbooks in the cookbook path" do - cookbook_loader.load_cookbooks expect(cookbook_loader).to have_key(:openldap) expect(cookbook_loader).to have_key(:apache2) end @@ -260,6 +268,7 @@ describe Chef::CookbookLoader do describe "loading all cookbooks after loading only one cookbook" do before(:each) do + expect(Chef::Log).to receive(:deprecation).with(/The cookbook\(s\): openldap exist in multiple places in your cookbook_path./) cookbook_loader.load_cookbooks end -- cgit v1.2.1 From d4a4f181c32310043e9279cdf11d37013e89e9ca Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Tue, 16 Feb 2016 15:23:58 -0800 Subject: don't warn about shadow cookbooks twice knife cookbook upload already warns once --- lib/chef/cookbook_loader.rb | 12 ++++++++++-- lib/chef/knife/cookbook_upload.rb | 4 ++-- spec/unit/knife/cookbook_upload_spec.rb | 4 +++- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/chef/cookbook_loader.rb b/lib/chef/cookbook_loader.rb index c0fd515bf2..6b80fbd662 100644 --- a/lib/chef/cookbook_loader.rb +++ b/lib/chef/cookbook_loader.rb @@ -73,15 +73,23 @@ class Chef end end - def load_cookbooks + # Will be removed when cookbook shadowing is removed, do NOT create new consumers of this API. + # + # @api private + def load_cookbooks_without_shadow_warning preload_cookbooks @loaders_by_name.each do |cookbook_name, _loaders| load_cookbook(cookbook_name) end - warn_about_cookbook_shadowing @cookbooks_by_name end + def load_cookbooks + ret = load_cookbooks_without_shadow_warning + warn_about_cookbook_shadowing + ret + end + def load_cookbook(cookbook_name) preload_cookbooks diff --git a/lib/chef/knife/cookbook_upload.rb b/lib/chef/knife/cookbook_upload.rb index e16e21ae64..f1e27fb8ac 100644 --- a/lib/chef/knife/cookbook_upload.rb +++ b/lib/chef/knife/cookbook_upload.rb @@ -103,7 +103,7 @@ class Chef @server_side_cookbooks = Chef::CookbookVersion.list_all_versions justify_width = @server_side_cookbooks.map { |name| name.size }.max.to_i + 2 if config[:all] - cookbook_repo.load_cookbooks + cookbook_repo.load_cookbooks_without_shadow_warning cookbooks_for_upload = [] cookbook_repo.each do |cookbook_name, cookbook| cookbooks_for_upload << cookbook @@ -166,7 +166,7 @@ class Chef def cookbooks_to_upload @cookbooks_to_upload ||= if config[:all] - cookbook_repo.load_cookbooks + cookbook_repo.load_cookbooks_without_shadow_warning else upload_set = {} @name_args.each do |cookbook_name| diff --git a/spec/unit/knife/cookbook_upload_spec.rb b/spec/unit/knife/cookbook_upload_spec.rb index 277da14011..9e07497c57 100644 --- a/spec/unit/knife/cookbook_upload_spec.rb +++ b/spec/unit/knife/cookbook_upload_spec.rb @@ -32,7 +32,7 @@ describe Chef::Knife::CookbookUpload do let(:cookbook_loader) do cookbook_loader = cookbooks_by_name.dup allow(cookbook_loader).to receive(:merged_cookbooks).and_return([]) - allow(cookbook_loader).to receive(:load_cookbooks).and_return(cookbook_loader) + allow(cookbook_loader).to receive(:load_cookbooks_without_shadow_warning).and_return(cookbook_loader) cookbook_loader end @@ -145,6 +145,7 @@ E it "should not read all cookbooks" do expect(cookbook_loader).not_to receive(:load_cookbooks) + expect(cookbook_loader).not_to receive(:load_cookbooks_without_shadow_warning) knife.run end @@ -208,6 +209,7 @@ E it "should exit and not upload the cookbook" do expect(cookbook_loader).to receive(:[]).once.with("test_cookbook") expect(cookbook_loader).not_to receive(:load_cookbooks) + expect(cookbook_loader).not_to receive(:load_cookbooks_without_shadow_warning) expect(cookbook_uploader).not_to receive(:upload_cookbooks) expect { knife.run }.to raise_error(SystemExit) end -- cgit v1.2.1 From b01b75fd0ea20be903b1bcfe7608e2b307bc1f44 Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Tue, 16 Feb 2016 15:42:46 -0800 Subject: be a bit more forceful about the message again for reference, dan deprecated this behavior in 0.10.4 back in 2011: https://github.com/chef/chef/commit/5a9fee8edaede311ac03a15d52fbc66dad83b576 --- lib/chef/cookbook_loader.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/chef/cookbook_loader.rb b/lib/chef/cookbook_loader.rb index 6b80fbd662..9367936c32 100644 --- a/lib/chef/cookbook_loader.rb +++ b/lib/chef/cookbook_loader.rb @@ -69,7 +69,7 @@ class Chef def warn_about_cookbook_shadowing unless merged_cookbooks.empty? Chef::Log.deprecation "The cookbook(s): #{merged_cookbooks.join(', ')} exist in multiple places in your cookbook_path. " + - "A composite version has been compiled. In a future version of Chef this behavior will be removed." + "A composite version has been compiled. This has been deprecated since 0.10.4, in Chef 13 this behavior will be REMOVED." end end -- cgit v1.2.1