Clean up code, add tests, fix bugs found in tests

Author: ericmj

lib/mix/lib/mix/deps.ex

@@ -93,7 +93,9 @@ defmodule Mix.Deps do
   changing the current working directory and loading the given
   project into the project stack.
   """
-  def in_dependency(Mix.Dep[app: app, opts: opts], post_config // [], fun) do
+  def in_dependency(dep, post_config // [], fun)
+
+  def in_dependency(Mix.Dep[app: app, opts: opts, rebar: nil], post_config, fun) do
     env     = opts[:env] || :prod
     old_env = Mix.env
 
@@ -105,6 +107,17 @@ defmodule Mix.Deps do
     end
   end
 
+  def in_dependency(Mix.Dep[opts: opts], post_config, fun) do
+    # Use post_config for rebar deps
+    Mix.Project.post_config(post_config)
+    Mix.Project.push(Mix.Rebar.Mixproject)
+    try do
+      File.cd!(opts[:dest], fn -> fun.(nil) end)
+    after
+      Mix.Project.pop
+    end
+  end
+
   @doc """
   Formats the status of a dependency.
   """
@@ -229,11 +242,11 @@ defmodule Mix.Deps do
         Enum.uniq paths
       rebar?(dep) ->
         # Add root dir and all sub dirs with ebin/ directory
-           [ opts[:dest] | dep.rebar[:sub_dirs] ]
-        |> Enum.map(Path.wildcard(&1))
-        |> List.concat
-        |> Enum.map(fn path -> Path.join([opts[:dest], path, "ebin"]) end)
-        |> Enum.filter(File.dir?(&1))
+        [ opts[:dest] | (dep.rebar[:sub_dirs] || []) ]
+          |> Enum.map(Path.wildcard(&1))
+          |> List.concat
+          |> Enum.map(fn path -> Path.join([opts[:dest], path, "ebin"]) end)
+          |> Enum.filter(File.dir?(&1))
       true ->
         [ Path.join(opts[:dest], "ebin") ]
     end

lib/mix/lib/mix/deps/retriever.ex

@@ -14,7 +14,7 @@ defmodule Mix.Deps.Retriever do
   provides a dependency in the wrong format.
   """
   def all(post_config // []) do
-    { deps, _ } = all(nil, children, post_config, fn(dep, acc) -> { dep, acc } end)
+    { deps, _ } = all(nil, children(post_config), post_config, fn(dep, acc) -> { dep, acc } end)
     deps
   end
 
@@ -24,7 +24,7 @@ defmodule Mix.Deps.Retriever do
   in case some processing is done.
   """
   def all(rest, post_config // [], callback) do
-    all(rest, children, post_config, callback)
+    all(rest, children(post_config), post_config, callback)
   end
 
   defp all(rest, childs, post_config, callback) do
@@ -34,7 +34,7 @@ defmodule Mix.Deps.Retriever do
       cond do
         Mix.Deps.available?(dep) and mixfile?(dep) ->
           Mix.Deps.in_dependency(dep, post_config, fn project ->
-            { deps, rest } = all(rest, children, [], callback)
+            { deps, rest } = all(rest, children(post_config), post_config, callback)
 
             # We need to call with_mix_project once again
             # here in case the dependency was not available
@@ -44,10 +44,12 @@ defmodule Mix.Deps.Retriever do
           end)
 
         Mix.Deps.available?(dep) and rebarconfig?(dep) ->
-          dir = dep.opts[:dest]
-          config = Mix.Rebar.load_config(dir)
-          { deps, rest } = all(rest, rebar_children(dir), [], callback)
-          { dep.rebar(config).deps(deps), rest }
+          dep = rebar_dep(dep)
+
+          Mix.Deps.in_dependency(dep, post_config, fn _ ->
+            { deps, rest } = all(rest, rebar_children("."), post_config, callback)
+            { dep.deps(deps), rest }
+          end)
 
         true ->
           { dep, rest }
@@ -60,8 +62,8 @@ defmodule Mix.Deps.Retriever do
   as a `Mix.Dep` record. Unlike with `all` the `deps`
   field is not populated.
   """
-  def children() do
-    Mix.Project.recur(fn _ ->
+  def children(post_config // []) do
+    Mix.Project.recur(post_config, fn _ ->
       (Mix.project[:deps] || []) |> setup_deps
     end) |> List.concat
   end
@@ -87,22 +89,20 @@ defmodule Mix.Deps.Retriever do
     Enum.map deps, fn dep ->
       dep = with_scm_and_status(dep, scms)
 
-      cond do
-        Mix.Deps.available?(dep) and mixfile?(dep) ->
-          Mix.Deps.in_dependency(dep, fn project ->
-            with_mix_project(dep, project)
-          end)
-        Mix.Deps.available?(dep) and rebarconfig?(dep) ->
-          config = Mix.Rebar.load_config(dep.opts[:dest])
-          with_rebar_config(dep, config)
-        true ->
-          dep
+      if Mix.Deps.available?(dep) and mixfile?(dep) do
+        dep = Mix.Deps.in_dependency(dep, fn project ->
+          with_mix_project(dep, project)
+        end)
       end
+
+      if Mix.Deps.available?(dep) and rebarconfig?(dep) do
+        dep = rebar_dep(dep)
+      end
+
+      dep
     end
   end
 
-  defp with_rebar_config(dep, _config), do: dep
-
   defp with_mix_project(Mix.Dep[project: nil] = dep, project) do
     if match?({ :noappfile, _ }, dep.status) and Mix.Project.umbrella? do
       dep = dep.update_opts(Keyword.put(&1, :app, false))
@@ -113,6 +113,13 @@ defmodule Mix.Deps.Retriever do
 
   defp with_mix_project(dep, _project), do: dep
 
+  defp rebar_dep(Mix.Dep[rebar: nil, opts: opts] = dep) do
+    config = Mix.Rebar.load_config(opts[:dest])
+    dep.rebar(config)
+  end
+
+  defp rebar_dep(dep), do: dep
+
   defp with_scm_and_status({ app, opts }, scms) when is_atom(app) and is_list(opts) do
     with_scm_and_status({ app, nil, opts }, scms)
   end

lib/mix/lib/mix/project.ex

@@ -149,7 +149,7 @@ defmodule Mix.Project do
   use this function to transparently go through the project, regardless
   if it is an umbrella project or not.
   """
-  def recur(fun) do
+  def recur(post_config // [], fun) do
     if apps_path = config[:apps_path] do
       paths = Path.wildcard(Path.join(apps_path, "*"))
       paths = Enum.filter(paths, File.dir?(&1))
@@ -163,11 +163,12 @@ defmodule Mix.Project do
       projects = topsort_projects(projects, Path.expand(apps_path))
 
       results = Enum.map projects, fn { app, app_path } ->
-        in_project(app, app_path, fun)
+        in_project(app, app_path, post_config, fun)
       end
 
       results
     else
+      # Note that post_config isnt used for this case
       [fun.(get)]
     end
   end

lib/mix/lib/mix/rebar.ex

@@ -31,29 +31,8 @@ defmodule Mix.Rebar do
   """
   def deps(config) do
     if deps = config[:deps] do
-      Enum.map(deps, fn({ app, req, source }) ->
-        scm = case source do
-          { scm, url }    -> [{ scm, to_binary(url) }]
-          { scm, url, _ } -> [{ scm, to_binary(url) }]
-        end
-
-        ref = case source do
-          { _, _, "" }                  -> [branch: "HEAD"]
-          { _, _, { :branch, branch } } -> [branch: to_binary(branch)]
-          { _, _, { :tag, tag } }       -> [tag: to_binary(tag)]
-          { _, _, ref }                 -> [ref: to_binary(ref)]
-          _                             -> []
-        end
-
-        raw = case source do
-          { _, _, _, [:raw] }      -> [app: false]
-          { _, _, _, [raw: true] } -> [app: false]
-          _                        -> []
-        end
-
-        opts = scm ++ ref ++ raw
-        { app, to_binary(req), opts }
-      end)
+      deps_dir = config[:deps_dir] || "deps"
+      Enum.map(deps, parse_dep(&1, deps_dir))
     else
       []
     end
@@ -67,10 +46,10 @@ defmodule Mix.Rebar do
     config = load_config(dir)
 
     if sub_dirs = config[:sub_dirs] do
-      sub_dirs =  sub_dirs
-               |> Enum.map(Path.wildcard(&1))
-               |> List.concat
-               |> Enum.filter(File.dir?(&1))
+      sub_dirs = sub_dirs
+       |> Enum.map(Path.wildcard(&1))
+       |> List.concat
+       |> Enum.filter(File.dir?(&1))
 
       Enum.map(sub_dirs, fn(dir) ->
         recur(dir, fun)
@@ -80,8 +59,46 @@ defmodule Mix.Rebar do
     [File.cd!(dir, fn -> fun.(config) end)]
   end
 
+  defp parse_dep({ app, req }, deps_dir) do
+    { app, compile_req(req), [path: Path.join(deps_dir, app)]}
+  end
+
+  defp parse_dep({ app, req, source }, _deps_dir) do
+    [ scm, url | source ] = tuple_to_list(source)
+
+    { ref, source } = case source do
+      [""|s]                  -> { [branch: "HEAD"], s }
+      [{ :branch, branch }|s] -> { [branch: to_binary(branch)], s }
+      [{ :tag, tag }|s]       -> { [tag: to_binary(tag)], s }
+      [ref|s]                 -> { [ref: to_binary(ref)], s }
+      _                       -> { [], [] }
+    end
+
+    raw = case source do
+      [[:raw]|_]      -> [app: false]
+      [[raw: true]|_] -> [app: false]
+      _               -> []
+    end
+
+    opts = [{ scm, to_binary(url) }] ++ ref ++ raw
+    { app, compile_req(req), opts }
+  end
+
+  defp parse_dep(app, deps_dir) do
+    parse_dep({ app, ".*" }, deps_dir)
+  end
+
+  defp compile_req(req) do
+    case to_binary(req) |> Regex.compile do
+      { :ok, re } ->
+        re
+      { :error, reason } ->
+        raise Mix.Error, message: "Unable to compile version regex: \"#{req}\", #{reason}"
+    end
+  end
+
   defp eval_script(path, config) do
-    script = Path.basename(path)
+    script = Path.basename(path) |> binary_to_list
     case :file.script(path, eval_binds(CONFIG: config, SCRIPT: script)) do
       { :ok, config } ->
         config
@@ -97,3 +114,11 @@ defmodule Mix.Rebar do
     end)
   end
 end
+
+# Used when pushing a rebar dependency to the project stack
+defmodule Mix.Rebar.Mixproject do
+  @moduledoc false
+
+  @doc false
+  def project, do: []
+end

lib/mix/test/fixtures/.gitignore

@@ -1,2 +1,3 @@
 git_repo
-deps_on_git_repo
+deps_on_git_repo
+git_rebar

lib/mix/test/fixtures/rebar_dep/rebar.config

@@ -0,0 +1,5 @@
+{ sub_dirs, ["apps/*"] }.
+
+{ deps, [
+    { git_rebar, "0.1..*", { git, "../git_rebar", master } }
+] }.

lib/mix/test/fixtures/rebar_dep/rebar.config.script

@@ -0,0 +1 @@
+CONFIG ++ [{'SCRIPT', SCRIPT}].

lib/mix/test/mix/rebar_test.exs

@@ -0,0 +1,65 @@
+Code.require_file "../test_helper.exs", __DIR__
+
+defmodule Mix.RebarTest do
+  use MixTest.Case
+
+  defmodule RebarAsDep do
+    def project do
+      [ app: :rebar_as_dep,
+        version: "0.1.0",
+        deps: [
+          { :rebar_dep, %r/.*/, path: MixTest.Case.fixture_path("rebar_dep"), app: false }
+        ]
+      ]
+    end
+  end
+
+  test "load rebar config" do
+    path = MixTest.Case.fixture_path("rebar_dep")
+    config = Mix.Rebar.load_config(path)
+    assert config[:sub_dirs] == ['apps/*']
+    assert config[:SCRIPT] == 'rebar.config.script'
+  end
+
+  test "rebar sub_dirs" do
+    path = MixTest.Case.fixture_path("rebar_dep")
+    assert Mix.Rebar.recur(path, fn _ -> File.cwd end)
+           [path, Path.join([path, "apps", "rebar_sub_dep"])]
+  end
+
+  test "parse rebar dependencies" do
+    Mix.Project.push(RebarAsDep)
+
+    in_tmp "parse rebar dependencies", fn ->
+      deps = Mix.Deps.all
+      assert Enum.find(deps, match?(Mix.Dep[app: :rebar_dep], &1))
+
+      assert Enum.find(deps, fn dep ->
+        Mix.Dep[app: app, opts: opts] = dep
+        if app == :git_rebar do
+          assert Enum.find(opts, match?({:git, "../git_rebar"}, &1))
+          assert Enum.find(opts, match?({:ref, "master"}, &1))
+          true
+        end
+      end)
+    end
+  after
+    Mix.Project.pop
+  end
+
+  test "get and compile dependencies for rebar" do
+    Mix.Project.push(RebarAsDep)
+
+    in_tmp "dependencies for rebar", fn ->
+      Mix.Tasks.Deps.Get.run []
+      assert_received { :mix_shell, :info, ["* Getting git_rebar [git: \"../git_rebar\"]"] }
+
+      Mix.Tasks.Compile.run []
+      assert_received { :mix_shell, :info, ["* Compiling git_rebar"] }
+      assert_received { :mix_shell, :info, ["* Compiling rebar_dep"] }
+      assert :git_rebar.any_function == :ok
+    end
+  after
+    Mix.Project.pop
+  end
+end

lib/mix/test/mix/umbrella_test.exs

@@ -19,6 +19,18 @@ defmodule Mix.UmbrellaTest do
     purge [Umbrella.Mixfile, Foo, Foo.Mix, Bar, Bar.Mix]
   end
 
+  test "dependency in umbrella" do
+    in_fixture "umbrella_dep/deps/umbrella", fn ->
+      Mix.Project.in_project(:umbrella, ".", fn _ ->
+        Mix.Task.run "deps"
+        assert_received { :mix_shell, :info, ["==> bar"] }
+        assert_received { :mix_shell, :info, ["* foo [path: \"../foo\"]"] }
+      end)
+    end
+  after
+    purge [Umbrella.Mixfile, Foo.Mix, Bar.Mix]
+  end
+
   test "umbrella as dependency" do
     in_fixture "umbrella_dep", fn ->
       Mix.Project.in_project(:umbrella_dep, ".", fn _ ->