From 86ca65f7dee8ed94032be01de7d6b9634676d9bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 3 Aug 2018 12:27:09 +0200 Subject: [PATCH 1/3] Accordingly mark top level optional dependencies from child deps Closes #7990 --- lib/mix/lib/mix/dep.ex | 24 +++++++++++++++++++++--- lib/mix/test/mix/dep_test.exs | 25 +++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/lib/mix/lib/mix/dep.ex b/lib/mix/lib/mix/dep.ex index b94245126f1..6df8c513206 100644 --- a/lib/mix/lib/mix/dep.ex +++ b/lib/mix/lib/mix/dep.ex @@ -124,11 +124,29 @@ defmodule Mix.Dep do defp load_and_cache(config, _top, bottom, _env) do {_, deps} = Mix.ProjectStack.read_cache({:cached_deps, bottom}) app = Keyword.fetch!(config, :app) - top_level = for dep <- deps, dep.app == app, child <- dep.deps, do: child.app - seen = populate_seen(MapSet.new(), [app]) children = get_deps(deps, tl(Enum.uniq(get_children(deps, seen, [app])))) - Enum.map(children, &%{&1 | top_level: &1.app in top_level}) + + top_level = + for dep <- deps, + dep.app == app, + child <- dep.deps, + do: {child.app, child.opts}, + into: %{} + + Enum.map(children, fn %{app: app} = dep -> + case top_level do + %{^app => child_opts} -> + # Only top level dependencies can be optional. + # Any non-top level dependency that is optional and + # is still available means it has been fulfilled. + optional = Keyword.get(child_opts, :optional, false) + %{dep | top_level: true, opts: Keyword.put(dep.opts, :optional, optional)} + + %{} -> + %{dep | top_level: false} + end + end) end defp read_cached_deps(project, env) do diff --git a/lib/mix/test/mix/dep_test.exs b/lib/mix/test/mix/dep_test.exs index 36401fd1b4e..f49d8b3092e 100644 --- a/lib/mix/test/mix/dep_test.exs +++ b/lib/mix/test/mix/dep_test.exs @@ -509,6 +509,31 @@ defmodule Mix.DepTest do end) end + test "nested deps with optional matching" do + Process.put(:custom_deps_git_repo_opts, optional: true) + + # deps_repo brings git_repo but it is optional + deps = [ + {:deps_repo, "0.1.0", path: "custom/deps_repo"}, + {:git_repo, "0.1.0", git: MixTest.Case.fixture_path("git_repo")} + ] + + with_deps(deps, fn -> + in_fixture("deps_status", fn -> + File.mkdir_p!("custom/deps_repo/lib") + + File.write!("custom/deps_repo/lib/a.ex", """ + # Check that the child dependency is top_level and optional + [%Mix.Dep{app: :git_repo, top_level: true, opts: opts}] = Mix.Dep.cached() + true = Keyword.fetch!(opts, :optional) + """) + + Mix.Tasks.Deps.Get.run([]) + Mix.Tasks.Deps.Compile.run([]) + end) + end) + end + test "nested deps with overrides" do # deps_repo brings git_repo but it is overriden deps = [ From eda4b65aae7809cb18f264dd57fac8fc917ab495 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 3 Aug 2018 14:47:50 +0200 Subject: [PATCH 2/3] Also keep :runtime option --- lib/mix/lib/mix/dep.ex | 12 +++++++++--- lib/mix/test/mix/dep_test.exs | 25 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/lib/mix/lib/mix/dep.ex b/lib/mix/lib/mix/dep.ex index 6df8c513206..0090c505441 100644 --- a/lib/mix/lib/mix/dep.ex +++ b/lib/mix/lib/mix/dep.ex @@ -117,6 +117,8 @@ defmodule Mix.Dep do end end + @child_keep_opts [:optional, :runtime] + defp load_and_cache(_config, top, top, env) do converge(env: env) end @@ -131,7 +133,7 @@ defmodule Mix.Dep do for dep <- deps, dep.app == app, child <- dep.deps, - do: {child.app, child.opts}, + do: {child.app, Keyword.take(child.opts, @child_keep_opts)}, into: %{} Enum.map(children, fn %{app: app} = dep -> @@ -140,8 +142,12 @@ defmodule Mix.Dep do # Only top level dependencies can be optional. # Any non-top level dependency that is optional and # is still available means it has been fulfilled. - optional = Keyword.get(child_opts, :optional, false) - %{dep | top_level: true, opts: Keyword.put(dep.opts, :optional, optional)} + opts = + dep.opts + |> Keyword.drop(@child_keep_opts) + |> Keyword.merge(child_opts) + + %{dep | top_level: true, opts: opts} %{} -> %{dep | top_level: false} diff --git a/lib/mix/test/mix/dep_test.exs b/lib/mix/test/mix/dep_test.exs index f49d8b3092e..2b89af1c752 100644 --- a/lib/mix/test/mix/dep_test.exs +++ b/lib/mix/test/mix/dep_test.exs @@ -534,6 +534,31 @@ defmodule Mix.DepTest do end) end + test "nested deps with runtime matching" do + Process.put(:custom_deps_git_repo_opts, runtime: false) + + # deps_repo brings git_repo but it is optional + deps = [ + {:deps_repo, "0.1.0", path: "custom/deps_repo"}, + {:git_repo, "0.1.0", git: MixTest.Case.fixture_path("git_repo")} + ] + + with_deps(deps, fn -> + in_fixture("deps_status", fn -> + File.mkdir_p!("custom/deps_repo/lib") + + File.write!("custom/deps_repo/lib/a.ex", """ + # Check that the child dependency is top_level and optional + [%Mix.Dep{app: :git_repo, top_level: true, opts: opts}] = Mix.Dep.cached() + false = Keyword.fetch!(opts, :runtime) + """) + + Mix.Tasks.Deps.Get.run([]) + Mix.Tasks.Deps.Compile.run([]) + end) + end) + end + test "nested deps with overrides" do # deps_repo brings git_repo but it is overriden deps = [ From 1d4af75ac45e6c2cb14592b89f51ab7d096b9e60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 3 Aug 2018 15:57:33 +0200 Subject: [PATCH 3/3] Improve coverage --- lib/mix/lib/mix/dep.ex | 6 +++--- lib/mix/test/mix/dep_test.exs | 25 +++++++++++++++++++++++-- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/lib/mix/lib/mix/dep.ex b/lib/mix/lib/mix/dep.ex index 0090c505441..168c193f916 100644 --- a/lib/mix/lib/mix/dep.ex +++ b/lib/mix/lib/mix/dep.ex @@ -117,6 +117,9 @@ defmodule Mix.Dep do end end + # optional and runtime only matter at the top level. + # Any non-top level dependency that is optional and + # is still available means it has been fulfilled. @child_keep_opts [:optional, :runtime] defp load_and_cache(_config, top, top, env) do @@ -139,9 +142,6 @@ defmodule Mix.Dep do Enum.map(children, fn %{app: app} = dep -> case top_level do %{^app => child_opts} -> - # Only top level dependencies can be optional. - # Any non-top level dependency that is optional and - # is still available means it has been fulfilled. opts = dep.opts |> Keyword.drop(@child_keep_opts) diff --git a/lib/mix/test/mix/dep_test.exs b/lib/mix/test/mix/dep_test.exs index 2b89af1c752..3a7aa48cd72 100644 --- a/lib/mix/test/mix/dep_test.exs +++ b/lib/mix/test/mix/dep_test.exs @@ -534,10 +534,9 @@ defmodule Mix.DepTest do end) end - test "nested deps with runtime matching" do + test "nested deps with runtime override on parent" do Process.put(:custom_deps_git_repo_opts, runtime: false) - # deps_repo brings git_repo but it is optional deps = [ {:deps_repo, "0.1.0", path: "custom/deps_repo"}, {:git_repo, "0.1.0", git: MixTest.Case.fixture_path("git_repo")} @@ -559,6 +558,28 @@ defmodule Mix.DepTest do end) end + test "nested deps with runtime override on child" do + deps = [ + {:deps_repo, "0.1.0", path: "custom/deps_repo"}, + {:git_repo, "0.1.0", git: MixTest.Case.fixture_path("git_repo"), runtime: false} + ] + + with_deps(deps, fn -> + in_fixture("deps_status", fn -> + File.mkdir_p!("custom/deps_repo/lib") + + File.write!("custom/deps_repo/lib/a.ex", """ + # Check that the child dependency is top_level and optional + [%Mix.Dep{app: :git_repo, top_level: true, opts: opts}] = Mix.Dep.cached() + false = Keyword.has_key?(opts, :runtime) + """) + + Mix.Tasks.Deps.Get.run([]) + Mix.Tasks.Deps.Compile.run([]) + end) + end) + end + test "nested deps with overrides" do # deps_repo brings git_repo but it is overriden deps = [