Ensure --no-optional-deps skips them even after they are compiled, closes elixir-lang#14856

Author: josevalim

lib/mix/lib/mix/tasks/compile.all.ex

@@ -37,7 +37,7 @@ defmodule Mix.Tasks.Compile.All do
     # the load paths before compiling, which means any SCM coming
     # from archives will be removed from the code path.
     deps = Mix.Dep.cached()
-    apps = project_apps(config)
+    apps = compile_apps(config, args)
 
     {loaded_paths, loaded_modules} =
       Mix.AppLoader.load_apps(apps, deps, config, {[], []}, fn {app, path}, {paths, mods} ->
@@ -97,7 +97,10 @@ defmodule Mix.Tasks.Compile.All do
     {status, diagnostics}
   end
 
-  def project_apps(config) do
+  @doc """
+  Returns all apps that should be available at compile time.
+  """
+  def compile_apps(config, args) do
     project = Mix.Project.get!()
 
     properties =
@@ -107,12 +110,17 @@ defmodule Mix.Tasks.Compile.All do
       Keyword.get(properties, :included_applications, []) ++
         Keyword.get(properties, :extra_applications, [])
 
-    {all, _optional} =
-      Mix.Tasks.Compile.App.project_apps(properties, config, extra, fn ->
-        # Include all deps by design
-        Enum.map(config[:deps], &elem(&1, 0))
+    {all, optional} =
+      Mix.Utils.project_apps(properties, config, extra, fn apps_opts ->
+        for {app, opts} <- apps_opts do
+          {app, if(Keyword.get(opts, :optional, false), do: :optional, else: :required)}
+        end
       end)
 
-    all
+    if "--no-optional-deps" in args do
+      all -- optional
+    else
+      all
+    end
   end
 end

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

@@ -447,19 +447,19 @@ defmodule Mix.Tasks.Compile.App do
     end
 
     {all, optional} =
-      project_apps(properties, config, extra, fn ->
-        apps_from_runtime_prod_deps(properties, config)
+      Mix.Utils.project_apps(properties, config, extra, fn apps_opts ->
+        apps_from_runtime_prod_deps(properties, apps_opts)
       end)
 
     properties
     |> Keyword.put(:applications, all)
     |> Keyword.put(:optional_applications, optional)
   end
 
-  defp apps_from_runtime_prod_deps(properties, config) do
+  defp apps_from_runtime_prod_deps(properties, apps_opts) do
     included_applications = Keyword.get(properties, :included_applications, [])
 
-    for {app, opts} <- deps_opts(config),
+    for {app, opts} <- apps_opts,
         runtime_app?(opts),
         app not in included_applications,
         do: {app, if(Keyword.get(opts, :optional, false), do: :optional, else: :required)}
@@ -483,44 +483,4 @@ defmodule Mix.Tasks.Compile.App do
       :error -> true
     end
   end
-
-  defp deps_opts(config) do
-    for config_dep <- Keyword.get(config, :deps, []),
-        do: {elem(config_dep, 0), dep_opts(config_dep)}
-  end
-
-  defp dep_opts({_app, opts}) when is_list(opts), do: opts
-  defp dep_opts({_app, _req, opts}) when is_list(opts), do: opts
-  defp dep_opts(_), do: []
-
-  ## Helpers for loading and manipulating apps
-
-  @doc false
-  def project_apps(properties, config, extra, deps_loader) do
-    apps = Keyword.get(properties, :applications) || deps_loader.()
-    {all, required, optional} = split_by_type(extra ++ apps)
-    all = Enum.uniq(language_apps(config) ++ Enum.reverse(all))
-    optional = Enum.uniq(Enum.reverse(optional -- required))
-    {all, optional}
-  end
-
-  defp split_by_type(apps) do
-    Enum.reduce(apps, {[], [], []}, fn
-      app, {all, required, optional} when is_atom(app) ->
-        {[app | all], [app | required], optional}
-
-      {app, :required}, {all, required, optional} ->
-        {[app | all], [app | required], optional}
-
-      {app, :optional}, {all, required, optional} ->
-        {[app | all], required, [app | optional]}
-    end)
-  end
-
-  defp language_apps(config) do
-    case Keyword.get(config, :language, :elixir) do
-      :elixir -> [:kernel, :stdlib, :elixir]
-      :erlang -> [:kernel, :stdlib]
-    end
-  end
 end

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

@@ -155,7 +155,7 @@ defmodule Mix.Tasks.Compile do
     # If we are in an umbrella project, now load paths from all children.
     if apps_paths = Mix.Project.apps_paths(config) do
       loaded_paths =
-        (Mix.Tasks.Compile.All.project_apps(config) ++ Map.keys(apps_paths))
+        (Mix.Tasks.Compile.All.compile_apps(config, args) ++ Map.keys(apps_paths))
         |> Mix.AppLoader.load_apps(Mix.Dep.cached(), config, [], fn
           {_app, path}, acc -> if path, do: [path | acc], else: acc
         end)

lib/mix/lib/mix/utils.ex

@@ -11,6 +11,49 @@ defmodule Mix.Utils do
   # For bootstrapping purposes
   @compile {:no_warn_undefined, Logger}
 
+  @doc """
+  Returns the apps of a project with no filtering.
+
+  `compile.all` calls it to extract all compile-time apps.
+  `compile.app` calls it to extract all runtime apps.
+  """
+  def project_apps(properties, config, extra, deps_loader) do
+    apps = Keyword.get(properties, :applications) || deps_loader.(deps_opts(config))
+    {all, required, optional} = split_by_type(extra ++ apps)
+    all = Enum.uniq(language_apps(config) ++ Enum.reverse(all))
+    optional = Enum.uniq(Enum.reverse(optional -- required))
+    {all, optional}
+  end
+
+  defp split_by_type(apps) do
+    Enum.reduce(apps, {[], [], []}, fn
+      app, {all, required, optional} when is_atom(app) ->
+        {[app | all], [app | required], optional}
+
+      {app, :required}, {all, required, optional} ->
+        {[app | all], [app | required], optional}
+
+      {app, :optional}, {all, required, optional} ->
+        {[app | all], required, [app | optional]}
+    end)
+  end
+
+  defp language_apps(config) do
+    case Keyword.get(config, :language, :elixir) do
+      :elixir -> [:kernel, :stdlib, :elixir]
+      :erlang -> [:kernel, :stdlib]
+    end
+  end
+
+  defp deps_opts(config) do
+    for config_dep <- Keyword.get(config, :deps, []),
+        do: {elem(config_dep, 0), dep_opts(config_dep)}
+  end
+
+  defp dep_opts({_app, opts}) when is_list(opts), do: opts
+  defp dep_opts({_app, _req, opts}) when is_list(opts), do: opts
+  defp dep_opts(_), do: []
+
   @doc """
   Gets the Mix home.
 

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

@@ -1702,6 +1702,7 @@ defmodule Mix.Tasks.Compile.ElixirTest do
   test "compiles without optional dependencies" do
     in_fixture("no_mixfile", fn ->
       Mix.Project.push(GitApp)
+      Mix.Tasks.Deps.Get.run([])
 
       File.write!("lib/a.ex", """
       defmodule A do
@@ -1712,15 +1713,18 @@ defmodule Mix.Tasks.Compile.ElixirTest do
       assert capture_io(:stderr, fn ->
                Mix.Tasks.Compile.run(["--no-optional-deps"]) == :ok
              end) =~ "GitRepo.hello/0 is undefined"
-    end)
-  end
 
-  test "recompiles if --no-optional-deps change" do
-    in_fixture("no_mixfile", fn ->
-      Mix.Project.push(MixTest.Case.Sample)
-      assert Mix.Tasks.Compile.Elixir.run([]) == {:ok, []}
-      assert Mix.Tasks.Compile.Elixir.run([]) == {:noop, []}
-      assert Mix.Tasks.Compile.Elixir.run(["--no-optional-deps"]) == {:ok, []}
+      Mix.Task.clear()
+      assert Mix.Tasks.Compile.run([]) == {:ok, []}
+
+      purge([GitRepo])
+      Mix.Task.clear()
+
+      assert capture_io(:stderr, fn ->
+               Mix.Tasks.Compile.run(["--no-optional-deps"]) == :ok
+             end) =~ "GitRepo.hello/0 is undefined"
+
+      refute Code.ensure_loaded?(GitRepo)
     end)
   end