core: BoxCollection#all returns versions

lib/vagrant/box_collection.rb

@@ -188,9 +188,8 @@ module Vagrant
     # This returns an array of all the boxes on the system, given by
     # their name and their provider.
     #
-    # @return [Array] Array of `[name, provider]` pairs of the boxes
-    #   installed on this system. An optional third element in the array
-    #   may specify `:v1` if the box is a version 1 box.
+    # @return [Array] Array of `[name, version, provider]` of the boxes
+    #   installed on this system.
     def all
       results = []
 
@@ -203,25 +202,21 @@ module Vagrant
 
           box_name = child.basename.to_s
 
-          # If this is a V1 box, we still return that name, but specify
-          # that the box is a V1 box.
-          if v1_box?(child)
-            @logger.debug("V1 box found: #{box_name}")
-            results << [box_name, :virtualbox, :v1]
-            next
-          end
-
-          # Otherwise, traverse the subdirectories and see what providers
+          # Otherwise, traverse the subdirectories and see what versions
           # we have.
-          child.children(true).each do |provider|
-            # Verify this is a potentially valid box. If it looks
-            # correct enough then include it.
-            if provider.directory? && provider.join("metadata.json").file?
-              provider_name = provider.basename.to_s.to_sym
-              @logger.debug("Box: #{box_name} (#{provider_name})")
-              results << [box_name, provider_name]
-            else
-              @logger.debug("Invalid box, ignoring: #{provider}")
+          child.children(true).each do |versiondir|
+            version = versiondir.basename.to_s
+
+            versiondir.children(true).each do |provider|
+              # Verify this is a potentially valid box. If it looks
+              # correct enough then include it.
+              if provider.directory? && provider.join("metadata.json").file?
+                provider_name = provider.basename.to_s.to_sym
+                @logger.debug("Box: #{box_name} (#{provider_name})")
+                results << [box_name, version, provider_name]
+              else
+                @logger.debug("Invalid box, ignoring: #{provider}")
+              end
             end
           end
         end

test/unit/support/isolated_environment.rb

@@ -89,6 +89,29 @@ module Unit
       box_dir
     end
 
+    # Creates a fake box to exist in this environment according
+    # to the "gen-3" box format.
+    #
+    # @param [String] name
+    # @param [String] version
+    # @param [String] provider
+    # @return [Pathname]
+    def box3(name, version, provider, **opts)
+      # Create the directory for the box
+      box_dir = boxes_dir.join(name, version, provider.to_s)
+      box_dir.mkpath
+
+      # Create the metadata.json for it
+      box_metadata_file = box_dir.join("metadata.json")
+      box_metadata_file.open("w") do |f|
+        f.write(JSON.generate({
+          :provider => provider.to_s
+        }))
+      end
+
+      box_dir
+    end
+
     # This creates a "box" file that is a valid V1 box.
     #
     # @return [Pathname] Path to the newly created box.

test/unit/vagrant/box_collection_test.rb

@@ -16,6 +16,65 @@ describe Vagrant::BoxCollection do
     instance.directory.should == environment.boxes_dir
   end
 
+  describe "#all" do
+    it "should return an empty array when no boxes are there" do
+      instance.all.should == []
+    end
+
+    it "should return the boxes and their providers" do
+      # Create some boxes
+      environment.box3("foo", "1.0", :virtualbox)
+      environment.box3("foo", "1.0", :vmware)
+      environment.box3("bar", "0", :ec2)
+
+      # Verify some output
+      results = instance.all
+      results.length.should == 3
+      results.include?(["foo", "1.0", :virtualbox]).should be
+      results.include?(["foo", "1.0", :vmware]).should be
+      results.include?(["bar", "0", :ec2]).should be
+    end
+
+    it 'does not raise an exception when a file appears in the boxes dir' do
+      Tempfile.new('a_file', environment.boxes_dir)
+      expect { instance.all }.to_not raise_error
+    end
+  end
+
+  describe "finding" do
+    it "should return nil if the box does not exist" do
+      instance.find("foo", :i_dont_exist).should be_nil
+    end
+
+    it "should return a box if the box does exist" do
+      # Create the "box"
+      environment.box2("foo", :virtualbox)
+
+      # Actual test
+      result = instance.find("foo", :virtualbox)
+      result.should_not be_nil
+      result.should be_kind_of(box_class)
+      result.name.should == "foo"
+    end
+
+    it "should throw an exception if it is a v1 box" do
+      # Create a V1 box
+      environment.box1("foo")
+
+      # Test!
+      expect { instance.find("foo", :virtualbox) }.
+        to raise_error(Vagrant::Errors::BoxUpgradeRequired)
+    end
+
+    it "should return nil if there is a V1 box but we're looking for another provider" do
+      # Create a V1 box
+      environment.box1("foo")
+
+      # Test
+      instance.find("foo", :another_provider).should be_nil
+    end
+  end
+
   describe "adding" do
     it "should add a valid box to the system" do
       box_path = environment.box2_file(:virtualbox)
@@ -141,75 +200,6 @@ describe Vagrant::BoxCollection do
     end
   end
 
-  describe "listing all" do
-    it "should return an empty array when no boxes are there" do
-      instance.all.should == []
-    end
-
-    it "should return the boxes and their providers" do
-      # Create some boxes
-      environment.box2("foo", :virtualbox)
-      environment.box2("foo", :vmware)
-      environment.box2("bar", :ec2)
-
-      # Verify some output
-      results = instance.all
-      results.length.should == 3
-      results.include?(["foo", :virtualbox]).should be
-      results.include?(["foo", :vmware]).should be
-      results.include?(["bar", :ec2]).should be
-    end
-
-    it "should return V1 boxes as well" do
-      # Create some boxes, including a V1 box
-      environment.box1("bar")
-      environment.box2("foo", :vmware)
-
-      # Verify some output
-      results = instance.all.sort
-      results.should == [["bar", :virtualbox, :v1], ["foo", :vmware]]
-    end
-
-    it 'does not raise an exception when a file appears in the boxes dir' do
-      Tempfile.new('a_file', environment.boxes_dir)
-      expect { instance.all }.to_not raise_error
-    end
-  end
-
-  describe "finding" do
-    it "should return nil if the box does not exist" do
-      instance.find("foo", :i_dont_exist).should be_nil
-    end
-
-    it "should return a box if the box does exist" do
-      # Create the "box"
-      environment.box2("foo", :virtualbox)
-
-      # Actual test
-      result = instance.find("foo", :virtualbox)
-      result.should_not be_nil
-      result.should be_kind_of(box_class)
-      result.name.should == "foo"
-    end
-
-    it "should throw an exception if it is a v1 box" do
-      # Create a V1 box
-      environment.box1("foo")
-
-      # Test!
-      expect { instance.find("foo", :virtualbox) }.
-        to raise_error(Vagrant::Errors::BoxUpgradeRequired)
-    end
-
-    it "should return nil if there is a V1 box but we're looking for another provider" do
-      # Create a V1 box
-      environment.box1("foo")
-
-      # Test
-      instance.find("foo", :another_provider).should be_nil
-    end
-  end
-
   describe "upgrading" do
     it "should upgrade a V1 box to V2" do
       # Create a V1 box