Allow overriding specific dependencies in override (#15193)

Closes #14082. Closes #14156.

Author: josevalim

lib/mix/lib/mix/dep.ex

@@ -342,12 +342,14 @@ defmodule Mix.Dep do
     "the dependency compile environment is outdated, please run \"#{mix_env_var()}mix deps.compile\""
   end
 
-  def format_status(%Mix.Dep{app: app, status: {:divergedreq, vsn, other}} = dep) do
+  def format_status(%Mix.Dep{app: app, status: {:divergedreq, vsn, parent, other}} = dep) do
+    override = if parent, do: [parent], else: true
+
     "the dependency #{app} #{vsn}\n" <>
       dep_status(dep) <>
       "\n  does not match the requirement specified\n" <>
       dep_status(other) <>
-      "\n  Ensure they match or specify one of the above in your deps and set \"override: true\""
+      "\n  Ensure they match or specify one of the above in your deps and set \"override: #{inspect(override)}\""
   end
 
   def format_status(%Mix.Dep{app: app, status: {:divergedonly, other}} = dep) do
@@ -378,14 +380,16 @@ defmodule Mix.Dep do
       dep_status(other) <> "\n  #{recommendation}"
   end
 
-  def format_status(%Mix.Dep{app: app, status: {:diverged, other}} = dep) do
+  def format_status(%Mix.Dep{app: app, status: {:diverged, parent, other}} = dep) do
     "different specs were given for the #{app} app:\n" <>
-      "#{dep_status(dep)}#{dep_status(other)}\n  " <> override_diverge_recommendation(dep, other)
+      "#{dep_status(dep)}#{dep_status(other)}\n  " <>
+      override_diverge_recommendation(dep, parent, other)
   end
 
-  def format_status(%Mix.Dep{app: app, status: {:overridden, other}} = dep) do
+  def format_status(%Mix.Dep{app: app, status: {:overridden, parent, other}} = dep) do
     "the dependency #{app} in #{Path.relative_to_cwd(dep.from)} is overriding a child dependency:\n" <>
-      "#{dep_status(dep)}#{dep_status(other)}\n  " <> override_diverge_recommendation(dep, other)
+      "#{dep_status(dep)}#{dep_status(other)}\n  " <>
+      override_diverge_recommendation(dep, parent, other)
   end
 
   def format_status(%Mix.Dep{status: {:unavailable, _}, scm: scm}) do
@@ -404,11 +408,14 @@ defmodule Mix.Dep do
     "the dependency was built with another SCM, run \"#{mix_env_var()}mix deps.compile\""
   end
 
-  defp override_diverge_recommendation(dep, other) do
+  defp override_diverge_recommendation(dep, parent, other) do
     if dep.opts[:from_umbrella] || other.opts[:from_umbrella] do
       "Please remove the conflicting options from your definition"
     else
-      "Ensure they match or specify one of the above in your deps and set \"override: true\""
+      override = if parent, do: [parent], else: true
+
+      "Ensure they match or specify one of the above in your deps " <>
+        "and set \"override: #{inspect(override)}\""
     end
   end
 
@@ -499,9 +506,9 @@ defmodule Mix.Dep do
   @doc """
   Checks if a dependency has diverged.
   """
-  def diverged?(%Mix.Dep{status: {:overridden, _}}), do: true
-  def diverged?(%Mix.Dep{status: {:diverged, _}}), do: true
-  def diverged?(%Mix.Dep{status: {:divergedreq, _, _}}), do: true
+  def diverged?(%Mix.Dep{status: {:overridden, _, _}}), do: true
+  def diverged?(%Mix.Dep{status: {:diverged, _, _}}), do: true
+  def diverged?(%Mix.Dep{status: {:divergedreq, _, _, _}}), do: true
   def diverged?(%Mix.Dep{status: {:divergedonly, _}}), do: true
   def diverged?(%Mix.Dep{status: {:divergedtargets, _}}), do: true
   def diverged?(%Mix.Dep{}), do: false

lib/mix/lib/mix/dep/converger.ex

@@ -73,7 +73,31 @@ defmodule Mix.Dep.Converger do
   def converge(acc, lock, opts, callback) do
     {deps, acc, lock} = all(acc, lock, opts, callback)
     if remote = Mix.RemoteConverger.get(), do: remote.post_converge()
-    {topological_sort(deps), acc, lock}
+    sorted_deps = topological_sort(deps)
+    warn_on_unneeded_deps_overrides(sorted_deps)
+    {sorted_deps, acc, lock}
+  end
+
+  defp warn_on_unneeded_deps_overrides([%{app: app, opts: opts} = dep | rest]) do
+    override = Keyword.get(opts, :override, false)
+
+    if is_list(override) do
+      Enum.each(override, fn overridden_app ->
+        # Since we used a topological sort, if someone depends on dep, it will come after dep
+        with %{deps: deps} <- Enum.find(rest, &(&1.app == overridden_app)),
+             true <- Enum.any?(deps, &(&1.app == app)) do
+          :ok
+        else
+          _ -> warn_uneeded_override(dep, overridden_app)
+        end
+      end)
+    end
+
+    warn_on_unneeded_deps_overrides(rest)
+  end
+
+  defp warn_on_unneeded_deps_overrides([]) do
+    :ok
   end
 
   defp all(acc, lock, opts, callback) do
@@ -152,11 +176,10 @@ defmodule Mix.Dep.Converger do
 
   defp init_all(main, apps, rest, lock, callback, locked?, env_target, cache) do
     state = %{locked?: locked?, env_target: env_target, cache: cache, callback: callback}
-    {deps, _kept, _optional, rest, lock} = all(main, [], [], [], apps, [], rest, lock, state)
-    deps = Enum.reverse(deps)
+    {deps, _kept, _optional, rest, lock} = all(main, nil, [], [], [], apps, [], rest, lock, state)
     # When traversing dependencies, we keep skipped ones to
     # find conflicts. We remove them now after traversal.
-    {deps, _} = Mix.Dep.Loader.split_by_env_and_target(deps, env_target)
+    {deps, _} = deps |> Enum.reverse() |> Mix.Dep.Loader.split_by_env_and_target(env_target)
     {deps, rest, lock}
   end
 
@@ -199,19 +222,19 @@ defmodule Mix.Dep.Converger do
   # Now, since "d" was specified in a parent project, no
   # exception is going to be raised since d is considered
   # to be the authoritative source.
-  defp all([dep | t], acc, kept, upper, breadths, optional, rest, lock, state) do
-    case match_deps(acc, upper, dep, state.env_target) do
+  defp all([dep | t], parent, acc, kept, upper, breadths, optional, rest, lock, state) do
+    case match_deps(parent, acc, upper, dep, state.env_target) do
       {:replace, dep, acc} ->
-        all([dep | t], acc, kept, upper, breadths, optional, rest, lock, state)
+        all([dep | t], parent, acc, kept, upper, breadths, optional, rest, lock, state)
 
       {:match, acc} ->
-        all(t, acc, kept, upper, breadths, optional, rest, lock, state)
+        all(t, parent, acc, kept, upper, breadths, optional, rest, lock, state)
 
       :skip ->
         # We still keep skipped dependencies around to detect conflicts.
         # They must be rejected after every all iteration but they are not
         # included in the list of kept dependencies.
-        all(t, [dep | acc], kept, upper, breadths, optional, rest, lock, state)
+        all(t, parent, [dep | acc], kept, upper, breadths, optional, rest, lock, state)
 
       :nomatch ->
         {%{app: app, deps: deps, opts: opts} = dep, rest, lock} =
@@ -234,19 +257,21 @@ defmodule Mix.Dep.Converger do
         # no longer a dependency. Add it back for traversal.
         {no_longer_optional, optional} = Enum.split_with(optional, &(&1.app == app))
         t = no_longer_optional ++ t
+        acc = [dep | acc]
 
         {acc, kept, optional, rest, lock} =
-          all(t, [dep | acc], [dep.app | kept], upper, breadths, optional, rest, lock, state)
+          all(t, parent, acc, [dep.app | kept], upper, breadths, optional, rest, lock, state)
 
         # Now traverse all parent dependencies and see if we have any optional dependency.
         {discarded, deps} = split_non_fulfilled_optional(deps, kept, opts[:from_umbrella])
 
         new_breadths = Enum.map(deps, & &1.app) ++ breadths
-        all(deps, acc, kept, breadths, new_breadths, discarded ++ optional, rest, lock, state)
+        new_optional = discarded ++ optional
+        all(deps, app, acc, kept, breadths, new_breadths, new_optional, rest, lock, state)
     end
   end
 
-  defp all([], acc, kept, _upper, _current, optional, rest, lock, _state) do
+  defp all([], _parent, acc, kept, _upper, _current, optional, rest, lock, _state) do
     {acc, kept, optional, rest, lock}
   end
 
@@ -264,7 +289,7 @@ defmodule Mix.Dep.Converger do
   # diverges is in the upper breadth, in those cases we
   # also check for the override option and mark the dependency
   # as overridden instead of diverged.
-  defp match_deps(list, upper_breadths, %Mix.Dep{app: app} = dep, env_target) do
+  defp match_deps(parent, list, upper_breadths, %Mix.Dep{app: app} = dep, env_target) do
     case Enum.split_while(list, &(&1.app != app)) do
       {_, []} ->
         if Mix.Dep.Loader.skip?(dep, env_target) do
@@ -283,51 +308,84 @@ defmodule Mix.Dep.Converger do
           )
         end
 
+        override = Keyword.get(other_opts, :override, false)
+
         cond do
-          in_upper? && other_opts[:override] ->
+          in_upper? && override == true ->
             {:match, list}
 
           not converge?(other, dep) ->
-            tag = if in_upper?, do: :overridden, else: :diverged
-            other = %{other | status: {tag, dep}}
-            {:match, pre ++ [other | pos]}
+            if parent_overriden?(override, parent) do
+              {:match, list}
+            else
+              tag = if in_upper?, do: :overridden, else: :diverged
+              other = %{other | status: {tag, parent, dep}}
+              {:match, pre ++ [other | pos]}
+            end
 
           vsn = req_mismatch(other, dep) ->
-            other = %{other | status: {:divergedreq, vsn, dep}}
-            {:match, pre ++ [other | pos]}
+            if parent_overriden?(override, parent) do
+              {:match, list}
+            else
+              other = %{other | status: {:divergedreq, vsn, parent, dep}}
+              {:match, pre ++ [other | pos]}
+            end
 
           not in_upper? and Mix.Dep.Loader.skip?(other, env_target) and
               not Mix.Dep.Loader.skip?(dep, env_target) ->
-            dep =
-              dep
-              |> with_matching_only_and_targets(other, in_upper?)
-              |> merge_manager(other, in_upper?)
-
-            {:replace, dep, pre ++ pos}
+            dep
+            |> merge_manager(other, in_upper?)
+            |> with_matching_only_and_targets(other, in_upper?, override, parent, list, fn dep ->
+              {:replace, dep, pre ++ pos}
+            end)
 
           true ->
-            other =
-              other
-              |> with_matching_only_and_targets(dep, in_upper?)
-              |> merge_manager(dep, in_upper?)
-
-            {:match, pre ++ [other | pos]}
+            other
+            |> merge_manager(dep, in_upper?)
+            |> with_matching_only_and_targets(dep, in_upper?, override, parent, list, fn other ->
+              {:match, pre ++ [other | pos]}
+            end)
         end
     end
   end
 
-  defp with_matching_only_and_targets(other, dep, in_upper?) do
+  defp parent_overriden?(list, app) when is_list(list), do: app in list
+  defp parent_overriden?(_list, _app), do: false
+
+  defp with_matching_only_and_targets(other, dep, in_upper?, override, parent, list, callback) do
     %{opts: opts} = dep
 
     if opts[:optional] do
-      other
+      maybe_warn_uneeded_override(dep, override, parent)
+      callback.(other)
     else
-      other
-      |> with_matching(:only, dep, opts, in_upper?)
-      |> with_matching(:targets, dep, opts, in_upper?)
+      with {:ok, other} <- with_matching(other, :only, dep, opts, in_upper?),
+           {:ok, other} <- with_matching(other, :targets, dep, opts, in_upper?) do
+        maybe_warn_uneeded_override(dep, override, parent)
+        callback.(other)
+      else
+        {:error, other} ->
+          if parent_overriden?(override, parent) do
+            {:match, list}
+          else
+            callback.(other)
+          end
+      end
+    end
+  end
+
+  defp maybe_warn_uneeded_override(dep, override, parent) do
+    if parent_overriden?(override, parent) do
+      warn_uneeded_override(dep, parent)
     end
   end
 
+  defp warn_uneeded_override(dep, parent) do
+    Mix.shell().error(
+      "Dependency #{Mix.Dep.format_dep(dep)} no longer requires :override on #{inspect(parent)}"
+    )
+  end
+
   # When in_upper is true
   #
   # When a parent dependency specifies :only/:targets that is a
@@ -345,16 +403,16 @@ defmodule Mix.Dep.Converger do
         case Keyword.fetch(opts, key) do
           {:ok, value} ->
             case List.wrap(value) -- List.wrap(other_value) do
-              [] -> other
-              _ -> %{other | status: {:"diverged#{key}", dep}}
+              [] -> {:ok, other}
+              _ -> {:error, %{other | status: {:"diverged#{key}", dep}}}
             end
 
           :error ->
-            %{other | status: {:"diverged#{key}", dep}}
+            {:error, %{other | status: {:"diverged#{key}", dep}}}
         end
 
       :error ->
-        other
+        {:ok, other}
     end
   end
 
@@ -369,9 +427,9 @@ defmodule Mix.Dep.Converger do
     value = Keyword.get(opts, key)
 
     if other_value && value do
-      put_in(other.opts[key], Enum.uniq(List.wrap(other_value) ++ List.wrap(value)))
+      {:ok, put_in(other.opts[key], Enum.uniq(List.wrap(other_value) ++ List.wrap(value)))}
     else
-      %{other | opts: Keyword.delete(other_opts, key)}
+      {:ok, %{other | opts: Keyword.delete(other_opts, key)}}
     end
   end
 

lib/mix/lib/mix/tasks/deps.ex

@@ -99,7 +99,11 @@ defmodule Mix.Tasks.Deps do
       targets (like `[:host, :rpi3]`)
 
     * `:override` - if set to `true` the dependency will override any other
-      definitions of itself by other dependencies
+      definitions of itself by other dependencies. From Elixir v1.20.0,
+      this option can also be a list of dependency names, which allows you to
+      override the definition of specific dependencies. If a dependency is not
+      included in the list and an override is required, it will still fail.
+      If a dependency is in the list and no longer necessary, then an error is emitted
 
     * `:manager` - Mix can also compile Rebar3 and makefile projects
       and can fetch sub dependencies of Rebar3 projects. Mix will