Fixes #10663: Ignore boxes in collection with malformed version numbers
Prior to this commit, if a box some how got on disk that had an incorrect or invalid version number that did not match Gem::Version, Vagrant would throw an exception when attempting to generate a list of the boxes on disk. This commit fixes that by looking at the version from the path generated, and shows a warning to the user about the box and skips it from the list so they at least know about the problematic box and can still get a list of boxes.
This commit is contained in:
parent
7bfb41e71d
commit
6b9cdb4e48
|
@ -233,14 +233,23 @@ module Vagrant
|
||||||
version = versiondir.basename.to_s
|
version = versiondir.basename.to_s
|
||||||
|
|
||||||
versiondir.children(true).each do |provider|
|
versiondir.children(true).each do |provider|
|
||||||
|
# Ensure version of box is correct before continuing
|
||||||
|
if !Gem::Version.correct?(version)
|
||||||
|
ui = Vagrant::UI::Prefixed.new(Vagrant::UI::Colored.new, "vagrant")
|
||||||
|
ui.warn(I18n.t("vagrant.box_version_malformed",
|
||||||
|
version: version, box_name: box_name))
|
||||||
|
@logger.debug("Invalid version #{version} for box #{box_name}")
|
||||||
|
next
|
||||||
|
end
|
||||||
|
|
||||||
# Verify this is a potentially valid box. If it looks
|
# Verify this is a potentially valid box. If it looks
|
||||||
# correct enough then include it.
|
# correct enough then include it.
|
||||||
if provider.directory? && provider.join("metadata.json").file?
|
if provider.directory? && provider.join("metadata.json").file?
|
||||||
provider_name = provider.basename.to_s.to_sym
|
provider_name = provider.basename.to_s.to_sym
|
||||||
@logger.debug("Box: #{box_name} (#{provider_name})")
|
@logger.debug("Box: #{box_name} (#{provider_name}, #{version})")
|
||||||
results << [box_name, version, provider_name]
|
results << [box_name, version, provider_name]
|
||||||
else
|
else
|
||||||
@logger.debug("Invalid box, ignoring: #{provider}")
|
@logger.debug("Invalid box #{box_name}, ignoring: #{provider}")
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -82,6 +82,8 @@ en:
|
||||||
* '%{name}' for '%{provider}' (v%{version}) is up to date
|
* '%{name}' for '%{provider}' (v%{version}) is up to date
|
||||||
box_up_to_date_single: |-
|
box_up_to_date_single: |-
|
||||||
Box '%{name}' (v%{version}) is running the latest version.
|
Box '%{name}' (v%{version}) is running the latest version.
|
||||||
|
box_version_malformed:
|
||||||
|
Invalid version '%{version}' for '%{box_name}', ignoring...
|
||||||
cfengine_bootstrapping: |-
|
cfengine_bootstrapping: |-
|
||||||
Bootstrapping CFEngine with policy server: %{policy_server}...
|
Bootstrapping CFEngine with policy server: %{policy_server}...
|
||||||
cfengine_bootstrapping_policy_hub: |-
|
cfengine_bootstrapping_policy_hub: |-
|
||||||
|
|
|
@ -16,6 +16,8 @@ describe Vagrant::BoxCollection, :skip_windows do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe "#all" do
|
describe "#all" do
|
||||||
|
let(:ui) { double("ui", warn: true) }
|
||||||
|
|
||||||
it "should return an empty array when no boxes are there" do
|
it "should return an empty array when no boxes are there" do
|
||||||
expect(subject.all).to eq([])
|
expect(subject.all).to eq([])
|
||||||
end
|
end
|
||||||
|
@ -38,6 +40,27 @@ describe Vagrant::BoxCollection, :skip_windows do
|
||||||
expect(results.include?(["foo:colon", "1.0", :virtualbox])).to be
|
expect(results.include?(["foo:colon", "1.0", :virtualbox])).to be
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it "should return the boxes and their providers even if box has wrong version" do
|
||||||
|
allow(Vagrant::UI::Prefixed).to receive(:new).and_return(ui)
|
||||||
|
# Create some boxes
|
||||||
|
environment.box3("foo", "fake-invalid-version", :virtualbox)
|
||||||
|
environment.box3("foo", "1.0", :vmware)
|
||||||
|
environment.box3("bar", "0", :ec2)
|
||||||
|
environment.box3("foo-VAGRANTSLASH-bar", "1.0", :virtualbox)
|
||||||
|
environment.box3("foo-VAGRANTCOLON-colon", "1.0", :virtualbox)
|
||||||
|
|
||||||
|
expect(ui).to receive(:warn).once
|
||||||
|
|
||||||
|
# Verify some output
|
||||||
|
results = subject.all
|
||||||
|
expect(results.length).to eq(4)
|
||||||
|
expect(results.include?(["foo", "1.0", :virtualbox])).not_to be
|
||||||
|
expect(results.include?(["foo", "1.0", :vmware])).to be
|
||||||
|
expect(results.include?(["bar", "0", :ec2])).to be
|
||||||
|
expect(results.include?(["foo/bar", "1.0", :virtualbox])).to be
|
||||||
|
expect(results.include?(["foo:colon", "1.0", :virtualbox])).to be
|
||||||
|
end
|
||||||
|
|
||||||
it 'does not raise an exception when a file appears in the boxes dir' do
|
it 'does not raise an exception when a file appears in the boxes dir' do
|
||||||
Tempfile.open('vagrant-a_file', environment.boxes_dir) do
|
Tempfile.open('vagrant-a_file', environment.boxes_dir) do
|
||||||
expect { subject.all }.to_not raise_error
|
expect { subject.all }.to_not raise_error
|
||||||
|
|
Loading…
Reference in New Issue