Skip to content

Commit

Permalink
Fix converger order (elixir-lang#5452)
Browse files Browse the repository at this point in the history
Only reverse at the end instead of doing multiple reverses that will
mess with the full order of dependencies.
  • Loading branch information
ericmj authored Nov 20, 2016
1 parent c907598 commit cf3939c
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
3 changes: 2 additions & 1 deletion lib/mix/lib/mix/dep/converger.ex
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ defmodule Mix.Dep.Converger do

defp all(main, apps, callback, rest, lock, env, cache) do
{deps, rest, lock} = all(main, [], [], apps, callback, rest, lock, env, cache)
deps = Enum.reverse(deps)
# When traversing dependencies, we keep skipped ones to
# find conflicts. We remove them now after traversal.
{deps, _} = Mix.Dep.Loader.partition_by_env(deps, env)
Expand Down Expand Up @@ -191,7 +192,7 @@ defmodule Mix.Dep.Converger do
end

defp all([], acc, _upper, _current, _callback, rest, lock, _env, _cache) do
{Enum.reverse(acc), rest, lock}
{acc, rest, lock}
end

defp put_lock(%Mix.Dep{app: app} = dep, lock) do
Expand Down
10 changes: 6 additions & 4 deletions lib/mix/test/mix/dep_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,8 @@ defmodule Mix.DepTest do
defmodule IdentityRemoteConverger do
@behaviour Mix.RemoteConverger

def remote?(_app), do: true
def remote?(%Mix.Dep{app: :deps_repo}), do: false
def remote?(%Mix.Dep{}), do: true
def deps(_dep, _lock), do: []
def post_converge, do: :ok

Expand Down Expand Up @@ -280,10 +281,11 @@ defmodule Mix.DepTest do

test "pass dependencies to remote converger in defined order" do
deps = [
{:ok, "0.1.0", path: "deps/ok"},
{:ok, "0.1.0", path: "deps/ok"},
{:invalidvsn, "0.2.0", path: "deps/invalidvsn"},
{:deps_repo, "0.1.0", path: "custom/deps_repo"},
{:invalidapp, "0.1.0", path: "deps/invalidapp"},
{:noappfile, "0.1.0", path: "deps/noappfile"}
{:noappfile, "0.1.0", path: "deps/noappfile"}
]

with_deps deps, fn ->
Expand All @@ -293,7 +295,7 @@ defmodule Mix.DepTest do
Mix.Tasks.Deps.Get.run([])

deps = Process.get(:remote_converger) |> Enum.map(& &1.app)
assert deps == [:ok, :invalidvsn, :invalidapp, :noappfile]
assert deps == [:ok, :invalidvsn, :deps_repo, :invalidapp, :noappfile, :git_repo]
end
end
after
Expand Down

0 comments on commit cf3939c

Please sign in to comment.