Skip to content

Commit ac947a5

Browse files
author
José Valim
committed
Revert "Include optional dependencies in extra_applications (#8263)"
Unfortunately adding optional dependencies doesn't work for umbrella apps where each app has a different optional dependency. For example, Ecto 3.0 has both jason and poison as optional deps. Imagine the two umbrella children below: foo * ecto * jason bar * ecto * poison * jason Because ecto is shared with both, Ecto will include both poison and jason, which makes`foo` fail to boot when running in isolation. Closes #7930.
1 parent 82a511a commit ac947a5

File tree

4 files changed

+39
-8
lines changed

4 files changed

+39
-8
lines changed

lib/mix/lib/mix/dep.ex

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,13 +132,18 @@ defmodule Mix.Dep do
132132
for dep <- deps,
133133
dep.app == app,
134134
child <- dep.deps,
135-
do: {child.app, true},
135+
do: {child.app, Keyword.get(child.opts, :optional, false)},
136136
into: %{}
137137

138-
Enum.map(children, fn %{app: app} = dep ->
138+
Enum.map(children, fn %{app: app, opts: opts} = dep ->
139+
# optional only matters at the top level. Any non-top level dependency
140+
# that is optional and is still available means it has been fulfilled.
139141
case top_level do
140-
%{^app => _} -> %{dep | top_level: true}
141-
%{} -> %{dep | top_level: false}
142+
%{^app => optional} ->
143+
%{dep | top_level: true, opts: Keyword.put(opts, :optional, optional)}
144+
145+
%{} ->
146+
%{dep | top_level: false, opts: Keyword.delete(opts, :optional)}
142147
end
143148
end)
144149
end

lib/mix/lib/mix/tasks/compile.app.ex

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ defmodule Mix.Tasks.Compile.App do
212212
apps =
213213
properties
214214
|> Keyword.get(:applications)
215-
|> Kernel.||(apps_from_prod_deps(properties, config))
215+
|> Kernel.||(apps_from_prod_non_optional_deps(properties, config))
216216
|> normalize_apps(extra, config)
217217

218218
Keyword.put(properties, :applications, apps)
@@ -317,11 +317,12 @@ defmodule Mix.Tasks.Compile.App do
317317
end)
318318
end
319319

320-
defp apps_from_prod_deps(properties, config) do
320+
defp apps_from_prod_non_optional_deps(properties, config) do
321321
included_applications = Keyword.get(properties, :included_applications, [])
322322
non_runtime_deps = non_runtime_deps(config)
323323

324-
for %{app: app, top_level: true} <- Mix.Dep.cached(),
324+
for %{app: app, opts: opts, top_level: true} <- Mix.Dep.cached(),
325+
not Keyword.get(opts, :optional, false),
325326
not Map.has_key?(non_runtime_deps, app),
326327
app not in included_applications,
327328
do: app

lib/mix/test/mix/dep_test.exs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,31 @@ defmodule Mix.DepTest do
196196
end)
197197
end
198198

199+
test "nested deps with optional matching" do
200+
Process.put(:custom_deps_git_repo_opts, optional: true)
201+
202+
# deps_repo brings git_repo but it is optional
203+
deps = [
204+
{:deps_repo, "0.1.0", path: "custom/deps_repo"},
205+
{:git_repo, "0.1.0", git: MixTest.Case.fixture_path("git_repo")}
206+
]
207+
208+
with_deps(deps, fn ->
209+
in_fixture("deps_status", fn ->
210+
File.mkdir_p!("custom/deps_repo/lib")
211+
212+
File.write!("custom/deps_repo/lib/a.ex", """
213+
# Check that the child dependency is top_level and optional
214+
[%Mix.Dep{app: :git_repo, top_level: true, opts: opts}] = Mix.Dep.cached()
215+
true = Keyword.fetch!(opts, :optional)
216+
""")
217+
218+
Mix.Tasks.Deps.Get.run([])
219+
Mix.Tasks.Deps.Compile.run([])
220+
end)
221+
end)
222+
end
223+
199224
test "nested deps with convergence and optional dependencies" do
200225
deps = [
201226
{:deps_repo, "0.1.0", path: "custom/deps_repo"},

lib/mix/test/mix/tasks/compile.app_test.exs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ defmodule Mix.Tasks.Compile.AppTest do
9999
properties = parse_resource_file(:custom_deps)
100100

101101
assert properties[:applications] ==
102-
[:kernel, :stdlib, :elixir, :logger, :ok1, :ok3, :ok4, :ok6, :ok7]
102+
[:kernel, :stdlib, :elixir, :logger, :ok1, :ok3, :ok4, :ok7]
103103
end)
104104
end
105105

0 commit comments

Comments
 (0)