From 4940073e9223f419dafd0538cc157d9e8f7ca7b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 9 Oct 2018 20:35:34 +0200 Subject: [PATCH] Include optional dependencies in extra_applications Previously when using optional dependencies, they would not be automatically included by compile.app in extra_applications. This meant that an optional dependency would not be guaranteed to be started before the application. Forcing users to handle this logic in an ad-hoc fashion that is incompatible with releases. --- lib/mix/lib/mix/dep.ex | 13 ++++------- lib/mix/lib/mix/tasks/compile.app.ex | 7 +++--- lib/mix/test/mix/dep_test.exs | 25 --------------------- lib/mix/test/mix/tasks/compile.app_test.exs | 2 +- 4 files changed, 8 insertions(+), 39 deletions(-) diff --git a/lib/mix/lib/mix/dep.ex b/lib/mix/lib/mix/dep.ex index 16b89680845..c14e2be82d7 100644 --- a/lib/mix/lib/mix/dep.ex +++ b/lib/mix/lib/mix/dep.ex @@ -131,18 +131,13 @@ defmodule Mix.Dep do for dep <- deps, dep.app == app, child <- dep.deps, - do: {child.app, Keyword.get(child.opts, :optional, false)}, + do: {child.app, true}, into: %{} - Enum.map(children, fn %{app: app, opts: opts} = dep -> - # optional only matters at the top level. Any non-top level dependency - # that is optional and is still available means it has been fulfilled. + Enum.map(children, fn %{app: app} = dep -> case top_level do - %{^app => optional} -> - %{dep | top_level: true, opts: Keyword.put(opts, :optional, optional)} - - %{} -> - %{dep | top_level: false, opts: Keyword.delete(opts, :optional)} + %{^app => _} -> %{dep | top_level: true} + %{} -> %{dep | top_level: false} end end) end diff --git a/lib/mix/lib/mix/tasks/compile.app.ex b/lib/mix/lib/mix/tasks/compile.app.ex index d02f5cb92ac..dbe6dd4e043 100644 --- a/lib/mix/lib/mix/tasks/compile.app.ex +++ b/lib/mix/lib/mix/tasks/compile.app.ex @@ -208,7 +208,7 @@ defmodule Mix.Tasks.Compile.App do apps = properties |> Keyword.get(:applications) - |> Kernel.||(apps_from_prod_non_optional_deps(properties, config)) + |> Kernel.||(apps_from_prod_deps(properties, config)) |> normalize_apps(extra, config) Keyword.put(properties, :applications, apps) @@ -313,12 +313,11 @@ defmodule Mix.Tasks.Compile.App do end) end - defp apps_from_prod_non_optional_deps(properties, config) do + defp apps_from_prod_deps(properties, config) do included_applications = Keyword.get(properties, :included_applications, []) non_runtime_deps = non_runtime_deps(config) - for %{app: app, opts: opts, top_level: true} <- Mix.Dep.cached(), - not Keyword.get(opts, :optional, false), + for %{app: app, top_level: true} <- Mix.Dep.cached(), not Map.has_key?(non_runtime_deps, app), app not in included_applications, do: app diff --git a/lib/mix/test/mix/dep_test.exs b/lib/mix/test/mix/dep_test.exs index afa7de6fdcb..71cbfd0c152 100644 --- a/lib/mix/test/mix/dep_test.exs +++ b/lib/mix/test/mix/dep_test.exs @@ -525,31 +525,6 @@ 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 considers runtime from current app" do Process.put(:custom_deps_git_repo_opts, runtime: false) diff --git a/lib/mix/test/mix/tasks/compile.app_test.exs b/lib/mix/test/mix/tasks/compile.app_test.exs index 618b5af8418..1caaf502e40 100644 --- a/lib/mix/test/mix/tasks/compile.app_test.exs +++ b/lib/mix/test/mix/tasks/compile.app_test.exs @@ -99,7 +99,7 @@ defmodule Mix.Tasks.Compile.AppTest do properties = parse_resource_file(:custom_deps) assert properties[:applications] == - [:kernel, :stdlib, :elixir, :logger, :ok1, :ok3, :ok4, :ok7] + [:kernel, :stdlib, :elixir, :logger, :ok1, :ok3, :ok4, :ok6, :ok7] end) end