Skip to content

Commit 6878200

Browse files
christhekeelejosevalim
authored andcommitted
Produce better error messages for non-binary mix git deps refspecs. (#13088)
When using git dependencies, a branch/ref/tag specifier is passed verbatim to `System.cmd/3`. This can lead to intimidating error messages when they are not provided as a binary (for instance, an atom like `tag: :stable`): ``` ** (ArgumentError) all arguments for System.cmd/3 must be binaries (elixir 1.15.6) lib/system.ex:1083: System.cmd/3 (mix 1.15.6) lib/mix/scm/git.ex:287: Mix.SCM.Git.git!/2 ``` This PR adds a check during git opts verification time to provide better feedback.
1 parent 0a146d1 commit 6878200

File tree

2 files changed

+31
-11
lines changed

2 files changed

+31
-11
lines changed

lib/mix/lib/mix/scm/git.ex

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -201,18 +201,24 @@ defmodule Mix.SCM.Git do
201201
## Helpers
202202

203203
defp validate_git_options(opts) do
204-
err =
205-
"You should specify only one of branch, ref or tag, and only once. " <>
206-
"Error on Git dependency: #{redact_uri(opts[:git])}"
204+
case Keyword.take(opts, [:branch, :ref, :tag]) do
205+
[] ->
206+
opts
207207

208-
validate_single_uniq(opts, [:branch, :ref, :tag], err)
209-
end
208+
[{_refspec, value}] when is_binary(value) ->
209+
opts
210210

211-
defp validate_single_uniq(opts, take, error) do
212-
case Keyword.take(opts, take) do
213-
[] -> opts
214-
[_] -> opts
215-
_ -> Mix.raise(error)
211+
[{refspec, value}] ->
212+
Mix.raise(
213+
"A dependency's #{refspec} must be a string, got: #{inspect(value)}. " <>
214+
"Error on Git dependency: #{redact_uri(opts[:git])}"
215+
)
216+
217+
_ ->
218+
Mix.raise(
219+
"You should specify only one of branch, ref or tag, and only once. " <>
220+
"Error on Git dependency: #{redact_uri(opts[:git])}"
221+
)
216222
end
217223
end
218224

lib/mix/test/mix/scm/git_test.exs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ defmodule Mix.SCM.GitTest do
5050
end
5151
end
5252

53-
test "raises about conflicting Git checkout options" do
53+
test "raises about conflicting Git refspec options" do
5454
assert_raise Mix.Error, ~r/You should specify only one of branch, ref or tag/, fn ->
5555
Mix.SCM.Git.accepts_options(nil, git: "/repo", branch: "main", tag: "0.1.0")
5656
end
@@ -60,6 +60,20 @@ defmodule Mix.SCM.GitTest do
6060
end
6161
end
6262

63+
test "raises about non-binary Git refspec options" do
64+
assert_raise Mix.Error, ~r/A dependency's branch must be a string/, fn ->
65+
Mix.SCM.Git.accepts_options(nil, git: "/repo", branch: :main)
66+
end
67+
68+
assert_raise Mix.Error, ~r/A dependency's tag must be a string/, fn ->
69+
Mix.SCM.Git.accepts_options(nil, git: "/repo", tag: :stable)
70+
end
71+
72+
assert_raise Mix.Error, ~r/A dependency's ref must be a string/, fn ->
73+
Mix.SCM.Git.accepts_options(nil, git: "/repo", ref: :abcdef0123456789)
74+
end
75+
end
76+
6377
defp lock(opts \\ []) do
6478
[lock: {:git, "/repo", "abcdef0123456789", opts}]
6579
end

0 commit comments

Comments
 (0)