From 1003d722bd3b685ec50a4781227e4a5a413a462d Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 3 Aug 2018 18:38:30 +0200 Subject: [PATCH 1/6] Use `--` to separate download name and version. --- Library/Homebrew/compat.rb | 1 + Library/Homebrew/compat/download_strategy.rb | 16 ++++++++++++++++ Library/Homebrew/download_strategy.rb | 2 +- Library/Homebrew/test/cmd/fetch_spec.rb | 4 ++-- .../Homebrew/test/download_strategies_spec.rb | 4 ++-- 5 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 Library/Homebrew/compat/download_strategy.rb diff --git a/Library/Homebrew/compat.rb b/Library/Homebrew/compat.rb index fbb74cb546..163a666aa6 100644 --- a/Library/Homebrew/compat.rb +++ b/Library/Homebrew/compat.rb @@ -1,6 +1,7 @@ require "compat/os/mac" require "compat/dependable" require "compat/dependency_collector" +require "compat/download_strategy" require "compat/fileutils" require "compat/formula_support" require "compat/hbc" diff --git a/Library/Homebrew/compat/download_strategy.rb b/Library/Homebrew/compat/download_strategy.rb new file mode 100644 index 0000000000..38d8464758 --- /dev/null +++ b/Library/Homebrew/compat/download_strategy.rb @@ -0,0 +1,16 @@ +class AbstractFileDownloadStrategy + # TODO: This can be removed after a month because downloads + # will be outdated anyways at that point. + module Compat + def initialize(url, name, version, **meta) + super + + old_cached_location = @cache/"#{name}-#{version}#{ext}" + + return unless old_cached_location.exist? + FileUtils.mv old_cached_location, cached_location, force: true + end + end + + prepend Compat +end diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb index a9dacad7e9..cfccde511a 100644 --- a/Library/Homebrew/download_strategy.rb +++ b/Library/Homebrew/download_strategy.rb @@ -181,7 +181,7 @@ class AbstractFileDownloadStrategy < AbstractDownloadStrategy def initialize(url, name, version, **meta) super - @cached_location = @cache/"#{name}-#{version}#{ext}" + @cached_location = @cache/"#{name}--#{version}#{ext}" @temporary_path = Pathname.new("#{cached_location}.incomplete") end diff --git a/Library/Homebrew/test/cmd/fetch_spec.rb b/Library/Homebrew/test/cmd/fetch_spec.rb index 9e8d0bbf43..da5c5ce413 100644 --- a/Library/Homebrew/test/cmd/fetch_spec.rb +++ b/Library/Homebrew/test/cmd/fetch_spec.rb @@ -2,10 +2,10 @@ describe "brew fetch", :integration_test do it "downloads the Formula's URL" do setup_test_formula "testball" - expect(HOMEBREW_CACHE/"testball-0.1.tbz").not_to exist + expect(HOMEBREW_CACHE/"testball--0.1.tbz").not_to exist expect { brew "fetch", "testball" }.to be_a_success - expect(HOMEBREW_CACHE/"testball-0.1.tbz").to exist + expect(HOMEBREW_CACHE/"testball--0.1.tbz").to exist end end diff --git a/Library/Homebrew/test/download_strategies_spec.rb b/Library/Homebrew/test/download_strategies_spec.rb index 317eda99b7..8c1a887e1e 100644 --- a/Library/Homebrew/test/download_strategies_spec.rb +++ b/Library/Homebrew/test/download_strategies_spec.rb @@ -238,13 +238,13 @@ describe CurlDownloadStrategy do subject { described_class.new(url, name, version, **specs).cached_location } context "when URL ends with file" do - it { is_expected.to eq(HOMEBREW_CACHE/"foo-.tar.gz") } + it { is_expected.to eq(HOMEBREW_CACHE/"foo--.tar.gz") } end context "when URL file is in middle" do let(:url) { "http://example.com/foo.tar.gz/from/this/mirror" } - it { is_expected.to eq(HOMEBREW_CACHE/"foo-.tar.gz") } + it { is_expected.to eq(HOMEBREW_CACHE/"foo--.tar.gz") } end end end From 4065c1742dbcfb1105827f89a0026fe8b4578da5 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 3 Aug 2018 22:15:16 +0200 Subject: [PATCH 2/6] Add update migration for double dashes. --- Library/Homebrew/cmd/update-report.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Library/Homebrew/cmd/update-report.rb b/Library/Homebrew/cmd/update-report.rb index 97ba6e9279..4debfeca36 100644 --- a/Library/Homebrew/cmd/update-report.rb +++ b/Library/Homebrew/cmd/update-report.rb @@ -85,6 +85,7 @@ module Homebrew end migrate_legacy_cache_if_necessary + migrate_cache_entries_to_double_dashes migrate_legacy_keg_symlinks_if_necessary if !updated @@ -183,6 +184,21 @@ module Homebrew end end + def migrate_cache_entries_to_double_dashes + HOMEBREW_CACHE.children.each do |child| + next unless child.file? + + next unless /^(?[^\.]+[^\-])\-(?[^\-].*)/ =~ child.basename.to_s + target = HOMEBREW_CACHE/"#{prefix}--#{suffix}" + + if target.exist? + FileUtils.rm_rf child + else + FileUtils.mv child, target, force: true + end + end + end + def migrate_legacy_repository_if_necessary return unless HOMEBREW_PREFIX.to_s == "/usr/local" return unless HOMEBREW_REPOSITORY.to_s == "/usr/local" From 8158f045c485c2cff8f38bef8c3a0d3a53d50710 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 3 Aug 2018 22:49:22 +0200 Subject: [PATCH 3/6] Only run migration when updating from older version. --- Library/Homebrew/cmd/update-report.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/cmd/update-report.rb b/Library/Homebrew/cmd/update-report.rb index 4debfeca36..dce22af977 100644 --- a/Library/Homebrew/cmd/update-report.rb +++ b/Library/Homebrew/cmd/update-report.rb @@ -62,6 +62,11 @@ module Homebrew updated = true end + initial_version = Version.new(system_command!("git", + args: ["describe", "--tags", "--abbrev=0", initial_revision], + chdir: HOMEBREW_REPOSITORY, + print_stderr: false).stdout) + updated_taps = [] Tap.each do |tap| next unless tap.git? @@ -85,7 +90,7 @@ module Homebrew end migrate_legacy_cache_if_necessary - migrate_cache_entries_to_double_dashes + migrate_cache_entries_to_double_dashes(initial_version) migrate_legacy_keg_symlinks_if_necessary if !updated @@ -184,7 +189,9 @@ module Homebrew end end - def migrate_cache_entries_to_double_dashes + def migrate_cache_entries_to_double_dashes(initial_version) + return if initial_version > "1.7.1" + HOMEBREW_CACHE.children.each do |child| next unless child.file? From e0e750e458943751e06a66fdcce2bf475ecdca5b Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 3 Aug 2018 22:21:20 +0200 Subject: [PATCH 4/6] Remove `compat/download_strategy`. --- Library/Homebrew/compat.rb | 1 - Library/Homebrew/compat/download_strategy.rb | 16 ---------------- 2 files changed, 17 deletions(-) delete mode 100644 Library/Homebrew/compat/download_strategy.rb diff --git a/Library/Homebrew/compat.rb b/Library/Homebrew/compat.rb index 163a666aa6..fbb74cb546 100644 --- a/Library/Homebrew/compat.rb +++ b/Library/Homebrew/compat.rb @@ -1,7 +1,6 @@ require "compat/os/mac" require "compat/dependable" require "compat/dependency_collector" -require "compat/download_strategy" require "compat/fileutils" require "compat/formula_support" require "compat/hbc" diff --git a/Library/Homebrew/compat/download_strategy.rb b/Library/Homebrew/compat/download_strategy.rb deleted file mode 100644 index 38d8464758..0000000000 --- a/Library/Homebrew/compat/download_strategy.rb +++ /dev/null @@ -1,16 +0,0 @@ -class AbstractFileDownloadStrategy - # TODO: This can be removed after a month because downloads - # will be outdated anyways at that point. - module Compat - def initialize(url, name, version, **meta) - super - - old_cached_location = @cache/"#{name}-#{version}#{ext}" - - return unless old_cached_location.exist? - FileUtils.mv old_cached_location, cached_location, force: true - end - end - - prepend Compat -end From dca5dc817617717ff7843ac969f23d83b73d079f Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 5 Aug 2018 16:03:21 +0200 Subject: [PATCH 5/6] Add warnings for permission exceptions. --- Library/Homebrew/cmd/update-report.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/cmd/update-report.rb b/Library/Homebrew/cmd/update-report.rb index dce22af977..25c4cb1bad 100644 --- a/Library/Homebrew/cmd/update-report.rb +++ b/Library/Homebrew/cmd/update-report.rb @@ -199,9 +199,17 @@ module Homebrew target = HOMEBREW_CACHE/"#{prefix}--#{suffix}" if target.exist? - FileUtils.rm_rf child + begin + FileUtils.rm_rf child + rescue Errno::EACCES + opoo "Could not remove #{child}, please do so manually." + end else - FileUtils.mv child, target, force: true + begin + FileUtils.mv child, target + rescue Errno::EACCES + opoo "Could not move #{child} to #{target}, please do so manually." + end end end end From dcf5d8d79b5958cd904e0ef27af94f27108250b9 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 5 Aug 2018 18:23:08 +0200 Subject: [PATCH 6/6] Add test for `migrate_cache_entries_to_double_dashes`. --- .../test/cmd/update-report/reporter_spec.rb | 129 +++++++++++++++++ .../Homebrew/test/cmd/update-report_spec.rb | 133 ++---------------- 2 files changed, 144 insertions(+), 118 deletions(-) create mode 100644 Library/Homebrew/test/cmd/update-report/reporter_spec.rb diff --git a/Library/Homebrew/test/cmd/update-report/reporter_spec.rb b/Library/Homebrew/test/cmd/update-report/reporter_spec.rb new file mode 100644 index 0000000000..50b33bf70a --- /dev/null +++ b/Library/Homebrew/test/cmd/update-report/reporter_spec.rb @@ -0,0 +1,129 @@ +require "cmd/update-report" +require "formula_versions" +require "yaml" + +describe Reporter do + def perform_update(fixture_name = "") + allow(Formulary).to receive(:factory).and_return(double(pkg_version: "1.0")) + allow(FormulaVersions).to receive(:new).and_return(double(formula_at_revision: "2.0")) + + diff = YAML.load_file("#{TEST_FIXTURE_DIR}/updater_fixture.yaml")[fixture_name] + allow(subject).to receive(:diff).and_return(diff || "") + + hub.add(subject) if subject.updated? + end + + subject { reporter_class.new(tap) } + + let(:reporter_class) do + Class.new(described_class) do + def initialize(tap) + @tap = tap + + ENV["HOMEBREW_UPDATE_BEFORE#{tap.repo_var}"] = "12345678" + ENV["HOMEBREW_UPDATE_AFTER#{tap.repo_var}"] = "abcdef00" + + super(tap) + end + end + end + + let(:tap) { CoreTap.new } + let(:hub) { ReporterHub.new } + + specify "without revision variable" do + ENV.delete_if { |k, _v| k.start_with? "HOMEBREW_UPDATE" } + + expect { + described_class.new(tap) + }.to raise_error(Reporter::ReporterRevisionUnsetError) + end + + specify "without any changes" do + perform_update + expect(hub).to be_empty + end + + specify "without Formula changes" do + perform_update("update_git_diff_output_without_formulae_changes") + + expect(hub.select_formula(:M)).to be_empty + expect(hub.select_formula(:A)).to be_empty + expect(hub.select_formula(:D)).to be_empty + end + + specify "with Formula changes" do + perform_update("update_git_diff_output_with_formulae_changes") + + expect(hub.select_formula(:M)).to eq(%w[xar yajl]) + expect(hub.select_formula(:A)).to eq(%w[antiword bash-completion ddrescue dict lua]) + end + + specify "with removed Formulae" do + perform_update("update_git_diff_output_with_removed_formulae") + + expect(hub.select_formula(:D)).to eq(%w[libgsasl]) + end + + specify "with changed file type" do + perform_update("update_git_diff_output_with_changed_filetype") + + expect(hub.select_formula(:M)).to eq(%w[elixir]) + expect(hub.select_formula(:A)).to eq(%w[libbson]) + expect(hub.select_formula(:D)).to eq(%w[libgsasl]) + end + + specify "with renamed Formula" do + allow(tap).to receive(:formula_renames).and_return("cv" => "progress") + perform_update("update_git_diff_output_with_formula_rename") + + expect(hub.select_formula(:A)).to be_empty + expect(hub.select_formula(:D)).to be_empty + expect(hub.select_formula(:R)).to eq([["cv", "progress"]]) + end + + context "when updating a Tap other than the core Tap" do + let(:tap) { Tap.new("foo", "bar") } + + before do + (tap.path/"Formula").mkpath + end + + after do + tap.path.parent.rmtree + end + + specify "with restructured Tap" do + perform_update("update_git_diff_output_with_restructured_tap") + + expect(hub.select_formula(:A)).to be_empty + expect(hub.select_formula(:D)).to be_empty + expect(hub.select_formula(:R)).to be_empty + end + + specify "with renamed Formula and restructured Tap" do + allow(tap).to receive(:formula_renames).and_return("xchat" => "xchat2") + perform_update("update_git_diff_output_with_formula_rename_and_restructuring") + + expect(hub.select_formula(:A)).to be_empty + expect(hub.select_formula(:D)).to be_empty + expect(hub.select_formula(:R)).to eq([%w[foo/bar/xchat foo/bar/xchat2]]) + end + + specify "with simulated 'homebrew/php' restructuring" do + perform_update("update_git_diff_simulate_homebrew_php_restructuring") + + expect(hub.select_formula(:A)).to be_empty + expect(hub.select_formula(:D)).to be_empty + expect(hub.select_formula(:R)).to be_empty + end + + specify "with Formula changes" do + perform_update("update_git_diff_output_with_tap_formulae_changes") + + expect(hub.select_formula(:A)).to eq(%w[foo/bar/lua]) + expect(hub.select_formula(:M)).to eq(%w[foo/bar/git]) + expect(hub.select_formula(:D)).to be_empty + end + end +end diff --git a/Library/Homebrew/test/cmd/update-report_spec.rb b/Library/Homebrew/test/cmd/update-report_spec.rb index 50b33bf70a..dbe1938cdc 100644 --- a/Library/Homebrew/test/cmd/update-report_spec.rb +++ b/Library/Homebrew/test/cmd/update-report_spec.rb @@ -1,129 +1,26 @@ require "cmd/update-report" -require "formula_versions" -require "yaml" -describe Reporter do - def perform_update(fixture_name = "") - allow(Formulary).to receive(:factory).and_return(double(pkg_version: "1.0")) - allow(FormulaVersions).to receive(:new).and_return(double(formula_at_revision: "2.0")) +describe "brew update-report" do + describe "::migrate_cache_entries_to_double_dashes" do + let(:legacy_cache_file) { HOMEBREW_CACHE/"foo-1.2.3.tar.gz" } + let(:renamed_cache_file) { HOMEBREW_CACHE/"foo--1.2.3.tar.gz" } - diff = YAML.load_file("#{TEST_FIXTURE_DIR}/updater_fixture.yaml")[fixture_name] - allow(subject).to receive(:diff).and_return(diff || "") - - hub.add(subject) if subject.updated? - end - - subject { reporter_class.new(tap) } - - let(:reporter_class) do - Class.new(described_class) do - def initialize(tap) - @tap = tap - - ENV["HOMEBREW_UPDATE_BEFORE#{tap.repo_var}"] = "12345678" - ENV["HOMEBREW_UPDATE_AFTER#{tap.repo_var}"] = "abcdef00" - - super(tap) - end - end - end - - let(:tap) { CoreTap.new } - let(:hub) { ReporterHub.new } - - specify "without revision variable" do - ENV.delete_if { |k, _v| k.start_with? "HOMEBREW_UPDATE" } - - expect { - described_class.new(tap) - }.to raise_error(Reporter::ReporterRevisionUnsetError) - end - - specify "without any changes" do - perform_update - expect(hub).to be_empty - end - - specify "without Formula changes" do - perform_update("update_git_diff_output_without_formulae_changes") - - expect(hub.select_formula(:M)).to be_empty - expect(hub.select_formula(:A)).to be_empty - expect(hub.select_formula(:D)).to be_empty - end - - specify "with Formula changes" do - perform_update("update_git_diff_output_with_formulae_changes") - - expect(hub.select_formula(:M)).to eq(%w[xar yajl]) - expect(hub.select_formula(:A)).to eq(%w[antiword bash-completion ddrescue dict lua]) - end - - specify "with removed Formulae" do - perform_update("update_git_diff_output_with_removed_formulae") - - expect(hub.select_formula(:D)).to eq(%w[libgsasl]) - end - - specify "with changed file type" do - perform_update("update_git_diff_output_with_changed_filetype") - - expect(hub.select_formula(:M)).to eq(%w[elixir]) - expect(hub.select_formula(:A)).to eq(%w[libbson]) - expect(hub.select_formula(:D)).to eq(%w[libgsasl]) - end - - specify "with renamed Formula" do - allow(tap).to receive(:formula_renames).and_return("cv" => "progress") - perform_update("update_git_diff_output_with_formula_rename") - - expect(hub.select_formula(:A)).to be_empty - expect(hub.select_formula(:D)).to be_empty - expect(hub.select_formula(:R)).to eq([["cv", "progress"]]) - end - - context "when updating a Tap other than the core Tap" do - let(:tap) { Tap.new("foo", "bar") } - - before do - (tap.path/"Formula").mkpath + before(:each) do + FileUtils.touch legacy_cache_file end - after do - tap.path.parent.rmtree + it "moves old files to use double dashes when upgrading from <= 1.7.1" do + Homebrew.migrate_cache_entries_to_double_dashes(Version.new("1.7.1")) + + expect(legacy_cache_file).not_to exist + expect(renamed_cache_file).to exist end - specify "with restructured Tap" do - perform_update("update_git_diff_output_with_restructured_tap") + it "does not move files if upgrading from > 1.7.1" do + Homebrew.migrate_cache_entries_to_double_dashes(Version.new("1.7.2")) - expect(hub.select_formula(:A)).to be_empty - expect(hub.select_formula(:D)).to be_empty - expect(hub.select_formula(:R)).to be_empty - end - - specify "with renamed Formula and restructured Tap" do - allow(tap).to receive(:formula_renames).and_return("xchat" => "xchat2") - perform_update("update_git_diff_output_with_formula_rename_and_restructuring") - - expect(hub.select_formula(:A)).to be_empty - expect(hub.select_formula(:D)).to be_empty - expect(hub.select_formula(:R)).to eq([%w[foo/bar/xchat foo/bar/xchat2]]) - end - - specify "with simulated 'homebrew/php' restructuring" do - perform_update("update_git_diff_simulate_homebrew_php_restructuring") - - expect(hub.select_formula(:A)).to be_empty - expect(hub.select_formula(:D)).to be_empty - expect(hub.select_formula(:R)).to be_empty - end - - specify "with Formula changes" do - perform_update("update_git_diff_output_with_tap_formulae_changes") - - expect(hub.select_formula(:A)).to eq(%w[foo/bar/lua]) - expect(hub.select_formula(:M)).to eq(%w[foo/bar/git]) - expect(hub.select_formula(:D)).to be_empty + expect(legacy_cache_file).to exist + expect(renamed_cache_file).not_to exist end end end