From 3210d1f705c2b9b3cc9ded0c29a8c5ed14c1ef51 Mon Sep 17 00:00:00 2001 From: Tyler Witt Date: Wed, 8 Jan 2025 21:46:00 +0700 Subject: [PATCH 1/3] Add override_for to dep options override_for gives more information about when deps should explicitly be overridden or not. If a dep no longer needs an override, a message will be written indicating so. Alternatively, if a dep needs an override when a dep is already using override_for, it will alert the dev. With override: true set, if a user were to add a new dep that depends on the overridden app, there would be no indication of a potential issue. --- lib/mix/lib/mix/dep.ex | 2 +- lib/mix/lib/mix/dep/converger.ex | 6 ++- lib/mix/lib/mix/dep/fetcher.ex | 39 +++++++++++++++++ .../custom/override_for_repo/mix.exs | 11 +++++ lib/mix/test/mix/dep_test.exs | 43 +++++++++++++++++++ 5 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 lib/mix/test/fixtures/deps_status/custom/override_for_repo/mix.exs diff --git a/lib/mix/lib/mix/dep.ex b/lib/mix/lib/mix/dep.ex index a19871aeaea..c70a38ec2cc 100644 --- a/lib/mix/lib/mix/dep.ex +++ b/lib/mix/lib/mix/dep.ex @@ -392,7 +392,7 @@ defmodule Mix.Dep 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\"" + "Ensure they match or specify one of the above in your deps and set \"override: true\" or \"override_for:\"" end end diff --git a/lib/mix/lib/mix/dep/converger.ex b/lib/mix/lib/mix/dep/converger.ex index 6a29828966b..32a0e28618d 100644 --- a/lib/mix/lib/mix/dep/converger.ex +++ b/lib/mix/lib/mix/dep/converger.ex @@ -299,7 +299,7 @@ defmodule Mix.Dep.Converger do end cond do - in_upper? && other_opts[:override] -> + in_upper? && (other_opts[:override] || override_for?(other_opts, app)) -> {:match, list} not converge?(other, dep) -> @@ -331,6 +331,10 @@ defmodule Mix.Dep.Converger do end end + defp override_for?(opts, app) do + !!opts[:override_for] && app in opts[:override_for] + end + defp with_matching_only_and_targets(other, dep, in_upper?) do %{opts: opts} = dep diff --git a/lib/mix/lib/mix/dep/fetcher.ex b/lib/mix/lib/mix/dep/fetcher.ex index b884cecea85..779be931aac 100644 --- a/lib/mix/lib/mix/dep/fetcher.ex +++ b/lib/mix/lib/mix/dep/fetcher.ex @@ -110,6 +110,8 @@ defmodule Mix.Dep.Fetcher do Mix.Dep.Lock.write(lock, opts) mark_as_fetched(parent_deps) + evaluate_overrides(Enum.filter(deps, & &1.opts[:override_for]), deps) + # See if any of the deps diverged and abort. show_diverged!(Enum.filter(all_deps, &Mix.Dep.diverged?/1)) @@ -156,6 +158,43 @@ defmodule Mix.Dep.Fetcher do end) end + defp evaluate_overrides([], _deps), do: :ok + + defp evaluate_overrides(override_deps, deps) do + for override_dep <- override_deps, + dep <- deps, + dep_dependency <- dep.deps, + override_dep.app == dep_dependency.app do + overridden? = dep.app in override_dep.opts[:override_for] + + {:ok, satisfied?} = + Mix.Dep.Loader.vsn_match(dep_dependency.requirement, override_dep.requirement, dep.app) + + cond do + overridden? && satisfied? -> + show_expired_override(override_dep, dep.app) + + not overridden? && not satisfied? -> + show_missing_override(override_dep, dep.app) + + true -> + :ok + end + end + + :ok + end + + defp show_expired_override(dep, app) do + Mix.shell().info( + "Dependency #{Mix.Dep.format_dep(dep)} no longer requires override_for on #{app}" + ) + end + + defp show_missing_override(dep, app) do + Mix.shell().info("Dependency #{Mix.Dep.format_dep(dep)} requires override_for on #{app}") + end + defp show_diverged!([]), do: :ok defp show_diverged!(deps) do diff --git a/lib/mix/test/fixtures/deps_status/custom/override_for_repo/mix.exs b/lib/mix/test/fixtures/deps_status/custom/override_for_repo/mix.exs new file mode 100644 index 00000000000..5e821be94c6 --- /dev/null +++ b/lib/mix/test/fixtures/deps_status/custom/override_for_repo/mix.exs @@ -0,0 +1,11 @@ +defmodule OverrideForRepo do + use Mix.Project + + def project do + [ + app: :override_for_repo, + version: "0.1.0", + deps: [{:git_repo, "0.3.0", [git: MixTest.Case.fixture_path("git_repo")]}] + ] + end +end diff --git a/lib/mix/test/mix/dep_test.exs b/lib/mix/test/mix/dep_test.exs index 04f5c13b1d8..835f7909e16 100644 --- a/lib/mix/test/mix/dep_test.exs +++ b/lib/mix/test/mix/dep_test.exs @@ -1240,6 +1240,49 @@ defmodule Mix.DepTest do end end + describe "override_for" do + test "warns when overrides are no longer needed for an app" do + deps = [ + {:deps_repo, "0.1.0", path: "custom/deps_repo"}, + {:git_repo, "0.1.0", + git: MixTest.Case.fixture_path("git_repo"), override_for: [:deps_repo]} + ] + + with_deps(deps, fn -> + in_fixture("deps_status", fn -> + Mix.Tasks.Deps.Get.run([]) + Mix.Tasks.Deps.Compile.run([]) + end) + end) + + message = + "Dependency git_repo (#{fixture_path("git_repo")}) no longer requires override_for on deps_repo" + + assert_received {:mix_shell, :info, [^message]} + end + + test "errors when more overrides are needed for an app" do + deps = [ + {:deps_repo, "0.1.0", path: "custom/deps_repo"}, + {:override_for_repo, "0.1.0", path: "custom/override_for_repo"}, + {:git_repo, "0.2.0", + git: MixTest.Case.fixture_path("git_repo"), override_for: [:deps_repo]} + ] + + with_deps(deps, fn -> + in_fixture("deps_status", fn -> + Mix.Tasks.Deps.Get.run([]) + Mix.Tasks.Deps.Compile.run([]) + end) + end) + + message = + "Dependency git_repo (#{fixture_path("git_repo")}) requires override_for on override_for_repo" + + assert_received {:mix_shell, :info, [^message]} + end + end + describe "app generation" do test "considers runtime from current app on nested deps" do Process.put(:custom_deps_git_repo_opts, runtime: false) From d3fffa420aa7405d1af4a8b7e079c6aedae8874c Mon Sep 17 00:00:00 2001 From: Tyler Witt Date: Wed, 8 Jan 2025 23:33:16 +0700 Subject: [PATCH 2/3] Remove added fixture --- .../deps_status/custom/override_for_repo/mix.exs | 11 ----------- lib/mix/test/mix/dep_test.exs | 6 ++---- 2 files changed, 2 insertions(+), 15 deletions(-) delete mode 100644 lib/mix/test/fixtures/deps_status/custom/override_for_repo/mix.exs diff --git a/lib/mix/test/fixtures/deps_status/custom/override_for_repo/mix.exs b/lib/mix/test/fixtures/deps_status/custom/override_for_repo/mix.exs deleted file mode 100644 index 5e821be94c6..00000000000 --- a/lib/mix/test/fixtures/deps_status/custom/override_for_repo/mix.exs +++ /dev/null @@ -1,11 +0,0 @@ -defmodule OverrideForRepo do - use Mix.Project - - def project do - [ - app: :override_for_repo, - version: "0.1.0", - deps: [{:git_repo, "0.3.0", [git: MixTest.Case.fixture_path("git_repo")]}] - ] - end -end diff --git a/lib/mix/test/mix/dep_test.exs b/lib/mix/test/mix/dep_test.exs index 835f7909e16..61413a3d734 100644 --- a/lib/mix/test/mix/dep_test.exs +++ b/lib/mix/test/mix/dep_test.exs @@ -1264,9 +1264,7 @@ defmodule Mix.DepTest do test "errors when more overrides are needed for an app" do deps = [ {:deps_repo, "0.1.0", path: "custom/deps_repo"}, - {:override_for_repo, "0.1.0", path: "custom/override_for_repo"}, - {:git_repo, "0.2.0", - git: MixTest.Case.fixture_path("git_repo"), override_for: [:deps_repo]} + {:git_repo, "0.2.0", git: MixTest.Case.fixture_path("git_repo"), override_for: []} ] with_deps(deps, fn -> @@ -1277,7 +1275,7 @@ defmodule Mix.DepTest do end) message = - "Dependency git_repo (#{fixture_path("git_repo")}) requires override_for on override_for_repo" + "Dependency git_repo (#{fixture_path("git_repo")}) requires override_for on deps_repo" assert_received {:mix_shell, :info, [^message]} end From a6481a34081d5c5d10d1c4c5b17281d8f54423d6 Mon Sep 17 00:00:00 2001 From: Tyler Witt Date: Fri, 10 Jan 2025 14:09:40 +0700 Subject: [PATCH 3/3] Add documentation --- lib/mix/lib/mix/tasks/deps.ex | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/mix/lib/mix/tasks/deps.ex b/lib/mix/lib/mix/tasks/deps.ex index 8fe0572c56c..77fe792d669 100644 --- a/lib/mix/lib/mix/tasks/deps.ex +++ b/lib/mix/lib/mix/tasks/deps.ex @@ -97,6 +97,13 @@ defmodule Mix.Tasks.Deps do * `:override` - if set to `true` the dependency will override any other definitions of itself by other dependencies + * `:override_for` - if set to a list of applications requiring the + override (see the `override` opt), warnings will be emitted when other + apps are added that also require an override, and when the override is no + longer required. This setting behaves similarly to `override: true` for + dependency resolution. It does not fail resolution when more apps require + an override. + * `:manager` - Mix can also compile Rebar3 and makefile projects and can fetch sub dependencies of Rebar3 projects. Mix will try to infer the type of project but it can be overridden with this