From 7013aaf9688c41201fe58fd62527775b883f730e Mon Sep 17 00:00:00 2001 From: Richard Clamp Date: Wed, 14 Mar 2018 08:30:52 +0000 Subject: [PATCH 1/4] (maint) pin chefstyle Code isn't updated for latest version. Works around travis failures Signed-off-by: Richard Clamp --- omnibus.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/omnibus.gemspec b/omnibus.gemspec index 313a31c04..43cd4e6f9 100644 --- a/omnibus.gemspec +++ b/omnibus.gemspec @@ -37,7 +37,7 @@ Gem::Specification.new do |gem| gem.add_development_dependency "bundler" gem.add_development_dependency "artifactory", "~> 2.0" gem.add_development_dependency "aruba", "~> 0.5" - gem.add_development_dependency "chefstyle" + gem.add_development_dependency "chefstyle", "~> 0.5.0" gem.add_development_dependency "fauxhai", "~> 5.2" gem.add_development_dependency "rspec", "~> 3.0" gem.add_development_dependency "rspec-json_expectations" From b29532202f346f262aca619c96187a0292d7d247 Mon Sep 17 00:00:00 2001 From: Richard Clamp Date: Wed, 14 Mar 2018 09:07:06 +0000 Subject: [PATCH 2/4] Refactor examples to test git_cmd call rather than shellout! The tests in terms of shellout! were slightly at the wrong level, as it was also having to test what git_cmd passed onto shellout! Signed-off-by: Richard Clamp --- spec/unit/git_cache_spec.rb | 72 ++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/spec/unit/git_cache_spec.rb b/spec/unit/git_cache_spec.rb index 5361dcc27..c97cbc56b 100644 --- a/spec/unit/git_cache_spec.rb +++ b/spec/unit/git_cache_spec.rb @@ -52,8 +52,6 @@ module Omnibus described_class.new(zlib) end - let(:git_flags) { %Q{-c core.autocrlf=false -c core.ignorecase=false --git-dir="#{cache_path}" --work-tree="#{install_dir}"} } - describe "#cache_path" do it "returns the install path appended to the install_cache path" do expect(ipc.cache_path).to eq(cache_path) @@ -86,12 +84,12 @@ module Omnibus .and_return(false) expect(FileUtils).to receive(:mkdir_p) .with(File.dirname(ipc.cache_path)) - expect(ipc).to receive(:shellout!) - .with("git #{git_flags} init -q") - expect(ipc).to receive(:shellout!) - .with("git #{git_flags} config --local user.name \"Omnibus Git Cache\"") - expect(ipc).to receive(:shellout!) - .with("git #{git_flags} config --local user.email \"omnibus@localhost\"") + expect(ipc).to receive(:git_cmd) + .with("init -q") + expect(ipc).to receive(:git_cmd) + .with("config --local user.name \"Omnibus Git Cache\"") + expect(ipc).to receive(:git_cmd) + .with("config --local user.email \"omnibus@localhost\"") ipc.create_cache_path end @@ -102,14 +100,14 @@ module Omnibus allow(File).to receive(:directory?) .with(File.dirname(ipc.cache_path)) .and_return(true) - expect(ipc).to_not receive(:shellout!) + expect(ipc).to_not receive(:git_cmd) ipc.create_cache_path end end describe "#incremental" do before(:each) do - allow(ipc).to receive(:shellout!) + allow(ipc).to receive(:git_cmd) allow(ipc).to receive(:create_cache_path) end @@ -120,20 +118,20 @@ module Omnibus it "adds all the changes to git removing git directories" do expect(ipc).to receive(:remove_git_dirs) - expect(ipc).to receive(:shellout!) - .with("git #{git_flags} add -A -f") + expect(ipc).to receive(:git_cmd) + .with("add -A -f") ipc.incremental end it "commits the backup for the software" do - expect(ipc).to receive(:shellout!) - .with(%Q{git #{git_flags} commit -q -m "Backup of #{ipc.tag}"}) + expect(ipc).to receive(:git_cmd) + .with(%Q{commit -q -m "Backup of #{ipc.tag}"}) ipc.incremental end it "tags the software backup" do - expect(ipc).to receive(:shellout!) - .with(%Q{git #{git_flags} tag -f "#{ipc.tag}"}) + expect(ipc).to receive(:git_cmd) + .with(%Q{tag -f "#{ipc.tag}"}) ipc.incremental end end @@ -172,11 +170,11 @@ module Omnibus end before(:each) do - allow(ipc).to receive(:shellout!) - .with(%Q{git #{git_flags} tag -l "#{ipc.tag}"}) + allow(ipc).to receive(:git_cmd) + .with(%Q{tag -l "#{ipc.tag}"}) .and_return(tag_cmd) - allow(ipc).to receive(:shellout!) - .with(%Q{git #{git_flags} tag -f restore_here "#{ipc.tag}"}) + allow(ipc).to receive(:git_cmd) + .with(%Q{tag -f restore_here "#{ipc.tag}"}) allow(ipc).to receive(:create_cache_path) end @@ -186,11 +184,11 @@ module Omnibus end it "checks for a tag with the software and version, and if it finds it, marks it as restoration point" do - expect(ipc).to receive(:shellout!) - .with(%Q{git #{git_flags} tag -l "#{ipc.tag}"}) + expect(ipc).to receive(:git_cmd) + .with(%Q{tag -l "#{ipc.tag}"}) .and_return(tag_cmd) - expect(ipc).to receive(:shellout!) - .with(%Q{git #{git_flags} tag -f restore_here "#{ipc.tag}"}) + expect(ipc).to receive(:git_cmd) + .with(%Q{tag -f restore_here "#{ipc.tag}"}) ipc.restore end @@ -207,16 +205,16 @@ module Omnibus let(:git_restore_tag_output) { "restore_here\n" } it "checks out the last save restoration point and deletes the marker tag" do - expect(ipc).to receive(:shellout!) - .with(%Q{git #{git_flags} tag -l "restore_here"}) + expect(ipc).to receive(:git_cmd) + .with(%q{tag -l "restore_here"}) .and_return(restore_tag_cmd) - expect(ipc).to receive(:shellout!) - .with(%Q{git #{git_flags} tag -l "#{ipc.tag}"}) + expect(ipc).to receive(:git_cmd) + .with(%Q{tag -l "#{ipc.tag}"}) .and_return(tag_cmd) - expect(ipc).to receive(:shellout!) - .with(%Q{git #{git_flags} checkout -f restore_here}) - expect(ipc).to receive(:shellout!) - .with(%Q{git #{git_flags} tag -d restore_here}) + expect(ipc).to receive(:git_cmd) + .with(%q{checkout -f restore_here}) + expect(ipc).to receive(:git_cmd) + .with(%q{tag -d restore_here}) ipc.restore end end @@ -225,11 +223,11 @@ module Omnibus let(:git_restore_tag_output) { "\n" } it "does nothing" do - expect(ipc).to receive(:shellout!) - .with(%Q{git #{git_flags} tag -l "restore_here"}) + expect(ipc).to receive(:git_cmd) + .with(%q{tag -l "restore_here"}) .and_return(restore_tag_cmd) - expect(ipc).to receive(:shellout!) - .with(%Q{git #{git_flags} tag -l "#{ipc.tag}"}) + expect(ipc).to receive(:git_cmd) + .with(%Q{tag -l "#{ipc.tag}"}) .and_return(tag_cmd) ipc.restore end @@ -238,6 +236,8 @@ module Omnibus end describe "#git_cmd" do + let(:git_flags) { %Q{-c core.autocrlf=false -c core.ignorecase=false --git-dir="#{cache_path}" --work-tree="#{install_dir}"} } + let(:terrible_install_dir) { %q{/opt/why please don't do this} } before(:each) do From 819a35dede5e78a43b23bcc553f6663560769ffe Mon Sep 17 00:00:00 2001 From: Richard Clamp Date: Wed, 14 Mar 2018 10:54:49 +0000 Subject: [PATCH 3/4] Allow options to be specified to git_cmd Allows, for example, the commit message to be more interestingly generated. Signed-off-by: Richard Clamp --- lib/omnibus/git_cache.rb | 4 ++-- spec/unit/git_cache_spec.rb | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/omnibus/git_cache.rb b/lib/omnibus/git_cache.rb index 513f8134e..c4c33272c 100644 --- a/lib/omnibus/git_cache.rb +++ b/lib/omnibus/git_cache.rb @@ -195,7 +195,7 @@ def remove_git_dirs # # @return [Mixlib::Shellout] the underlying command object. # - def git_cmd(command) + def git_cmd(command, opts = {}) shellout!([ "git", "-c core.autocrlf=false", @@ -203,7 +203,7 @@ def git_cmd(command) "--git-dir=\"#{cache_path}\"", "--work-tree=\"#{install_dir}\"", command, - ].join(" ")) + ].join(" "), opts) end # diff --git a/spec/unit/git_cache_spec.rb b/spec/unit/git_cache_spec.rb index c97cbc56b..0d188fd55 100644 --- a/spec/unit/git_cache_spec.rb +++ b/spec/unit/git_cache_spec.rb @@ -244,16 +244,22 @@ module Omnibus allow(project).to receive(:install_dir) .and_return(terrible_install_dir) allow(ipc).to receive(:shellout!) - .with(%Q{git #{git_flags} version}) + .with(%Q{git #{git_flags} version}, {}) .and_return("git version 2.11.0") end it "doesn't mangle an #install_dir with spaces" do expect(ipc.send(:install_dir)).to eq(terrible_install_dir) expect(ipc).to receive(:shellout!) - .with(%Q{git #{git_flags} version}) + .with(%Q{git #{git_flags} version}, {}) ipc.send(:git_cmd, "version") end + + it "passes options" do + expect(ipc).to receive(:shellout!) + .with(%Q{git #{git_flags} commit -F -}, input: "Commit message") + ipc.send(:git_cmd, "commit -F -", input: "Commit message") + end end end end From 29dee9ed23ec4e52b912fca95a4d042b034a8523 Mon Sep 17 00:00:00 2001 From: Richard Clamp Date: Tue, 13 Mar 2018 17:04:09 +0000 Subject: [PATCH 4/4] Record the hardlinks in the commit message, so we can restore them git doesn't care about hardlinks; when it does the checkout it creates different files. Here we record the hardlinks in the install_dir as a json blob in the commit message, then use that to re-hardlink after cache restore. Signed-off-by: Richard Clamp --- lib/omnibus/git_cache.rb | 43 ++++++++++++++++++++- spec/unit/git_cache_spec.rb | 76 ++++++++++++++++++++++++++++++++++++- 2 files changed, 116 insertions(+), 3 deletions(-) diff --git a/lib/omnibus/git_cache.rb b/lib/omnibus/git_cache.rb index c4c33272c..842b6085a 100644 --- a/lib/omnibus/git_cache.rb +++ b/lib/omnibus/git_cache.rb @@ -32,7 +32,7 @@ class GitCache # will not have the generated content, so these snapshots would be # incompatible with the current omnibus codebase. Incrementing the serial # number ensures these old shapshots will not be used in subsequent builds. - SERIAL_NUMBER = 3 + SERIAL_NUMBER = 4 REQUIRED_GIT_FILES = %w{ HEAD @@ -126,11 +126,12 @@ def incremental create_cache_path remove_git_dirs + hardlinks = find_hardlinks git_cmd("add -A -f") begin - git_cmd(%Q{commit -q -m "Backup of #{tag}"}) + git_cmd("commit -q -F -", input: "Backup of #{tag}\n\n#{FFI_Yajl::Encoder.encode(hardlinks, pretty: true)}") rescue CommandFailed => e raise unless e.message.include?("nothing to commit") end @@ -159,6 +160,7 @@ def restore def restore_from_cache git_cmd("checkout -f restore_here") + restore_hardlinks ensure git_cmd("tag -d restore_here") end @@ -184,6 +186,43 @@ def remove_git_dirs true end + # Discover any hardlinked files in the install_dir + # + # @return [Hash{String => Array}] + def find_hardlinks + hardlink_sources = {} + hardlinks = {} + Omnibus::FileSyncer.all_files_under(install_dir).each do |path| + stat = File.stat(path) + if stat.ftype.to_sym == :file && stat.nlink > 1 + key = [stat.dev, stat.ino] + if source = hardlink_sources[key] + hardlinks[source] ||= [] + hardlinks[source] << path + else + hardlink_sources[key] = path + end + end + end + hardlinks + end + + # Restores any hardlinking from the commit message body. Body is assumed to + # be the JSON encoded return value from #find_hardlinks. + # + # @return true + def restore_hardlinks + body = git_cmd("log --format=%b -n 1").stdout + hardlinks = FFI_Yajl::Parser.parse(body) + + hardlinks.each do |source, dest| + dest.each do |path| + FileUtils.ln(source, path, force: true) + end + end + true + end + private # diff --git a/spec/unit/git_cache_spec.rb b/spec/unit/git_cache_spec.rb index 0d188fd55..f3c738cf2 100644 --- a/spec/unit/git_cache_spec.rb +++ b/spec/unit/git_cache_spec.rb @@ -105,10 +105,83 @@ module Omnibus end end + describe "#find_hardlinks" do + let(:regular_file_stat) do + stat = double(File::Stat) + allow(stat).to receive(:ftype).and_return(:file) + allow(stat).to receive(:nlink).and_return(1) + stat + end + + let(:hardlinked_file_stat) do + stat = double(File::Stat) + allow(stat).to receive(:ftype).and_return(:file) + allow(stat).to receive(:nlink).and_return(2) + allow(stat).to receive(:dev).and_return(5) + allow(stat).to receive(:ino).and_return(25) + stat + end + + before do + allow(File).to receive(:stat).and_return(regular_file_stat) + allow(File).to receive(:stat).with("foo").and_return(hardlinked_file_stat) + allow(File).to receive(:stat).with("bar").and_return(hardlinked_file_stat) + + allow(Omnibus::FileSyncer).to receive(:all_files_under).and_return( + %w{ foo bar baz quux } + ) + end + + it "returns some hardlinks" do + expect(ipc.find_hardlinks).to eq({ "foo" => ["bar"] }) + end + end + + describe "#restore_hardlinks" do + let(:hardlinks) do + { + "/opt/demo/bin/file1" => [ + "/opt/demo/bin/file2", + "/opt/demo/bin/file3", + ], + } + end + + let(:git_log_output) { FFI_Yajl::Encoder.encode(hardlinks) } + + let(:log_cmd) do + cmd_double = double(Mixlib::ShellOut) + allow(cmd_double).to receive(:stdout).and_return(git_log_output) + cmd_double + end + + before(:each) do + allow(ipc).to receive(:git_cmd) + .with("log --format=%b -n 1").and_return(log_cmd) + allow(FileUtils).to receive(:ln) + end + + it "checks the commit message" do + expect(ipc).to receive(:git_cmd) + .with("log --format=%b -n 1") + + ipc.restore_hardlinks + end + + it "recreates hardlinks" do + expect(FileUtils).to receive(:ln) + .with("/opt/demo/bin/file1", "/opt/demo/bin/file2", force: true) + expect(FileUtils).to receive(:ln) + .with("/opt/demo/bin/file1", "/opt/demo/bin/file3", force: true) + ipc.restore_hardlinks + end + end + describe "#incremental" do before(:each) do allow(ipc).to receive(:git_cmd) allow(ipc).to receive(:create_cache_path) + allow(ipc).to receive(:find_hardlinks).and_return({}) end it "creates the cache path" do @@ -125,7 +198,7 @@ module Omnibus it "commits the backup for the software" do expect(ipc).to receive(:git_cmd) - .with(%Q{commit -q -m "Backup of #{ipc.tag}"}) + .with("commit -q -F -", input: "Backup of #{ipc.tag}\n\n{\n\n}\n") ipc.incremental end @@ -176,6 +249,7 @@ module Omnibus allow(ipc).to receive(:git_cmd) .with(%Q{tag -f restore_here "#{ipc.tag}"}) allow(ipc).to receive(:create_cache_path) + allow(ipc).to receive(:restore_hardlinks) end it "creates the cache path" do