-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add :depth option to git deps #13128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a4530c4
3025d63
e58f568
849782c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,14 +125,18 @@ defmodule Mix.SCM.Git do | |
sparse_toggle(opts) | ||
update_origin(opts[:git]) | ||
|
||
rev = get_lock_rev(opts[:lock], opts) || get_opts_rev(opts) | ||
|
||
# Fetch external data | ||
["--git-dir=.git", "fetch", "--force", "--quiet"] | ||
|> Kernel.++(progress_switch(git_version())) | ||
|> Kernel.++(tags_switch(opts[:tag])) | ||
|> Kernel.++(depth_switch(opts[:depth])) | ||
|> Kernel.++(if rev, do: ["origin", rev], else: []) | ||
rhcarvalho marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|> git!() | ||
|
||
# Migrate the Git repo | ||
rev = get_lock_rev(opts[:lock], opts) || get_opts_rev(opts) || default_branch() | ||
rev = rev || default_branch() | ||
git!(["--git-dir=.git", "checkout", "--quiet", rev]) | ||
|
||
if opts[:submodules] do | ||
|
@@ -164,7 +168,7 @@ defmodule Mix.SCM.Git do | |
defp sparse_toggle(opts) do | ||
cond do | ||
sparse = opts[:sparse] -> | ||
sparse_check(git_version()) | ||
check_sparse_support(git_version()) | ||
git!(["--git-dir=.git", "config", "core.sparsecheckout", "true"]) | ||
File.mkdir_p!(".git/info") | ||
File.write!(".git/info/sparse-checkout", sparse) | ||
|
@@ -180,27 +184,50 @@ defmodule Mix.SCM.Git do | |
end | ||
end | ||
|
||
defp sparse_check(version) do | ||
unless {1, 7, 4} <= version do | ||
version = version |> Tuple.to_list() |> Enum.join(".") | ||
@min_git_version_sparse {1, 7, 4} | ||
@min_git_version_depth {1, 5, 0} | ||
@min_git_version_progress {1, 7, 1} | ||
|
||
defp check_sparse_support(version) do | ||
ensure_feature_compatibility(version, @min_git_version_sparse, "sparse checkout") | ||
end | ||
|
||
defp check_depth_support(version) do | ||
ensure_feature_compatibility(version, @min_git_version_depth, "depth (shallow clone)") | ||
end | ||
|
||
defp ensure_feature_compatibility(version, required_version, feature) do | ||
unless required_version <= version do | ||
Mix.raise( | ||
"Git >= 1.7.4 is required to use sparse checkout. " <> | ||
"You are running version #{version}" | ||
"Git >= #{format_version(required_version)} is required to use #{feature}. " <> | ||
"You are running version #{format_version(version)}" | ||
) | ||
end | ||
end | ||
|
||
defp progress_switch(version) do | ||
if {1, 7, 1} <= version, do: ["--progress"], else: [] | ||
if @min_git_version_progress <= version, do: ["--progress"], else: [] | ||
end | ||
|
||
defp tags_switch(nil), do: [] | ||
defp tags_switch(_), do: ["--tags"] | ||
|
||
defp depth_switch(nil), do: [] | ||
|
||
defp depth_switch(n) when is_integer(n) and n > 0 do | ||
check_depth_support(git_version()) | ||
["--depth=#{n}"] | ||
end | ||
|
||
## Helpers | ||
|
||
defp validate_git_options(opts) do | ||
opts | ||
|> validate_refspec() | ||
|> validate_depth() | ||
end | ||
|
||
defp validate_refspec(opts) do | ||
case Keyword.take(opts, [:branch, :ref, :tag]) do | ||
[] -> | ||
opts | ||
|
@@ -222,6 +249,22 @@ defmodule Mix.SCM.Git do | |
end | ||
end | ||
|
||
defp validate_depth(opts) do | ||
case Keyword.take(opts, [:depth]) do | ||
[] -> | ||
opts | ||
|
||
[{:depth, depth}] when is_integer(depth) and depth > 0 -> | ||
opts | ||
|
||
invalid_depth -> | ||
Mix.raise( | ||
"The depth must be a positive integer, and be specified only once, got: #{inspect(invalid_depth)}. " <> | ||
"Error on Git dependency: #{redact_uri(opts[:git])}" | ||
) | ||
end | ||
end | ||
|
||
defp get_lock(opts) do | ||
%{rev: rev} = get_rev_info() | ||
{:git, opts[:git], rev, get_lock_opts(opts)} | ||
|
@@ -238,7 +281,7 @@ defmodule Mix.SCM.Git do | |
defp get_lock_rev(_, _), do: nil | ||
|
||
defp get_lock_opts(opts) do | ||
lock_opts = Keyword.take(opts, [:branch, :ref, :tag, :sparse, :subdir]) | ||
lock_opts = Keyword.take(opts, [:branch, :ref, :tag, :sparse, :subdir, :depth]) | ||
|
||
if opts[:submodules] do | ||
lock_opts ++ [submodules: true] | ||
|
@@ -248,11 +291,7 @@ defmodule Mix.SCM.Git do | |
end | ||
|
||
defp get_opts_rev(opts) do | ||
if branch = opts[:branch] do | ||
"origin/#{branch}" | ||
else | ||
opts[:ref] || opts[:tag] | ||
end | ||
opts[:branch] || opts[:ref] || opts[:tag] | ||
rhcarvalho marked this conversation as resolved.
Show resolved
Hide resolved
|
||
end | ||
|
||
defp redact_uri(git) do | ||
|
@@ -282,6 +321,8 @@ defmodule Mix.SCM.Git do | |
end | ||
|
||
defp default_branch() do | ||
# Note: the `set-head -a` command requires the remote reference to be | ||
# fetched first. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added this note because the most obvious code would be to "compute" |
||
git!(["--git-dir=.git", "remote", "set-head", "origin", "-a"]) | ||
"origin/HEAD" | ||
end | ||
|
@@ -328,9 +369,17 @@ defmodule Mix.SCM.Git do | |
end | ||
end | ||
|
||
# Also invoked by lib/mix/test/test_helper.exs | ||
# Invoked by lib/mix/test/test_helper.exs | ||
@doc false | ||
def git_version do | ||
def unsupported_options do | ||
git_version = git_version() | ||
|
||
[] | ||
|> Kernel.++(if git_version < @min_git_version_sparse, do: [:sparse], else: []) | ||
|> Kernel.++(if git_version < @min_git_version_depth, do: [:depth], else: []) | ||
end | ||
|
||
defp git_version do | ||
case Mix.State.fetch(:git_version) do | ||
{:ok, version} -> | ||
version | ||
|
@@ -354,6 +403,10 @@ defmodule Mix.SCM.Git do | |
|> List.to_tuple() | ||
end | ||
|
||
defp format_version(version) do | ||
version |> Tuple.to_list() |> Enum.join(".") | ||
end | ||
|
||
defp to_integer(string) do | ||
{int, _} = Integer.parse(string) | ||
int | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -478,6 +478,207 @@ defmodule Mix.Tasks.DepsGitTest do | |
purge([GitRepo, GitRepo.MixProject]) | ||
end | ||
|
||
describe "Git depth option" do | ||
@describetag :git_depth | ||
|
||
test "gets and updates Git repos with depth option" do | ||
Process.put(:git_repo_opts, depth: 1) | ||
|
||
in_fixture("no_mixfile", fn -> | ||
Mix.Project.push(GitApp) | ||
|
||
Mix.Tasks.Deps.Get.run([]) | ||
message = "* Getting git_repo (#{fixture_path("git_repo")})" | ||
assert_received {:mix_shell, :info, [^message]} | ||
assert_shallow("deps/git_repo", 1) | ||
|
||
# Expand depth | ||
update_dep(depth: 2) | ||
Mix.Tasks.Deps.Get.run([]) | ||
assert_shallow("deps/git_repo", 2) | ||
|
||
# Reduce depth | ||
update_dep(depth: 1) | ||
Mix.Tasks.Deps.Get.run([]) | ||
assert_shallow("deps/git_repo", 1) | ||
end) | ||
end | ||
|
||
test "with tag" do | ||
Process.put(:git_repo_opts, depth: 1, tag: "with_module") | ||
|
||
in_fixture("no_mixfile", fn -> | ||
Mix.Project.push(GitApp) | ||
|
||
Mix.Tasks.Deps.Get.run([]) | ||
message = "* Getting git_repo (#{fixture_path("git_repo")} - with_module)" | ||
assert_received {:mix_shell, :info, [^message]} | ||
assert_shallow("deps/git_repo", 1) | ||
end) | ||
end | ||
|
||
test "with branch" do | ||
Process.put(:git_repo_opts, depth: 1, branch: "main") | ||
|
||
in_fixture("no_mixfile", fn -> | ||
Mix.Project.push(GitApp) | ||
|
||
Mix.Tasks.Deps.Get.run([]) | ||
message = "* Getting git_repo (#{fixture_path("git_repo")} - main)" | ||
assert_received {:mix_shell, :info, [^message]} | ||
assert_shallow("deps/git_repo", 1) | ||
end) | ||
end | ||
|
||
test "with ref" do | ||
[last, _ | _] = get_git_repo_revs("git_repo") | ||
|
||
Process.put(:git_repo_opts, depth: 1, ref: last) | ||
|
||
in_fixture("no_mixfile", fn -> | ||
Mix.Project.push(GitApp) | ||
|
||
Mix.Tasks.Deps.Get.run([]) | ||
message = "* Getting git_repo (#{fixture_path("git_repo")} - #{last})" | ||
assert_received {:mix_shell, :info, [^message]} | ||
assert_shallow("deps/git_repo", 1) | ||
end) | ||
end | ||
|
||
test "changing refspec updates retaining depth" do | ||
[last, first | _] = get_git_repo_revs("git_repo") | ||
|
||
Process.put(:git_repo_opts, ref: first, depth: 1) | ||
|
||
in_fixture("no_mixfile", fn -> | ||
Mix.Project.push(GitApp) | ||
|
||
Mix.Tasks.Deps.Get.run([]) | ||
message = "* Getting git_repo (#{fixture_path("git_repo")} - #{first})" | ||
assert_received {:mix_shell, :info, [^message]} | ||
assert_shallow("deps/git_repo", 1) | ||
assert File.read!("mix.lock") =~ first | ||
|
||
# Change refspec | ||
update_dep(ref: last, depth: 1) | ||
Mix.Tasks.Deps.Get.run([]) | ||
assert_shallow("deps/git_repo", 1) | ||
assert File.read!("mix.lock") =~ last | ||
end) | ||
end | ||
Comment on lines
+548
to
+568
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is intended to cover José's concern in the original conversation. The way to call |
||
|
||
test "removing depth retains shallow repository" do | ||
# For compatibility and simplicity, we follow Git's behavior and do not | ||
# attempt to unshallow an existing repository. This should not be a | ||
# problem, because all we guarantee is that the correct source code is | ||
# available whenever mix.exs or mix.lock change. If one wanted to have a | ||
# full clone, they can always run `deps.clean` and `deps.get` again. | ||
Process.put(:git_repo_opts, depth: 1) | ||
|
||
in_fixture("no_mixfile", fn -> | ||
Mix.Project.push(GitApp) | ||
|
||
Mix.Tasks.Deps.Get.run([]) | ||
message = "* Getting git_repo (#{fixture_path("git_repo")})" | ||
assert_received {:mix_shell, :info, [^message]} | ||
assert_shallow("deps/git_repo", 1) | ||
|
||
# Remove depth | ||
update_dep([]) | ||
Mix.Tasks.Deps.Get.run([]) | ||
refute File.read!("mix.lock") =~ "depth:" | ||
assert File.exists?("deps/git_repo/.git/shallow") | ||
|
||
assert System.cmd("git", ~w[--git-dir=deps/git_repo/.git rev-list --count HEAD]) == | ||
{"1\n", 0} | ||
end) | ||
end | ||
|
||
@tag :git_sparse | ||
test "with sparse checkout" do | ||
Process.put(:git_repo_opts, sparse: "sparse_dir", depth: 1) | ||
|
||
in_fixture("no_mixfile", fn -> | ||
Mix.Project.push(GitApp) | ||
|
||
Mix.Tasks.Deps.Get.run([]) | ||
message = "* Getting git_repo (#{fixture_path("git_repo")})" | ||
assert_received {:mix_shell, :info, [^message]} | ||
assert_shallow("deps/git_repo", 1) | ||
|
||
refute File.exists?("deps/git_repo/mix.exs") | ||
assert File.exists?("deps/git_repo/sparse_dir/mix.exs") | ||
assert File.read!("mix.lock") =~ "sparse: \"sparse_dir\"" | ||
end) | ||
end | ||
|
||
test "with subdir" do | ||
Process.put(:git_repo_opts, subdir: "sparse_dir", depth: 1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed that the |
||
|
||
in_fixture("no_mixfile", fn -> | ||
Mix.Project.push(GitApp) | ||
|
||
Mix.Tasks.Deps.Get.run([]) | ||
message = "* Getting git_repo (#{fixture_path("git_repo")})" | ||
assert_received {:mix_shell, :info, [^message]} | ||
assert_shallow("deps/git_repo", 1) | ||
|
||
assert File.exists?("deps/git_repo/mix.exs") | ||
assert File.exists?("deps/git_repo/sparse_dir/mix.exs") | ||
assert File.read!("mix.lock") =~ "subdir: \"sparse_dir\"" | ||
end) | ||
end | ||
|
||
test "does not affect submodules depth" do | ||
# The expectation is that we can add an explicit option in the future, | ||
# just like git-clone has `--shallow-submodules`. | ||
Process.put(:git_repo_opts, submodules: true, depth: 1) | ||
|
||
in_fixture("no_mixfile", fn -> | ||
Mix.Project.push(GitApp) | ||
|
||
Mix.Tasks.Deps.Get.run([]) | ||
message = "* Getting git_repo (#{fixture_path("git_repo")})" | ||
assert_received {:mix_shell, :info, [^message]} | ||
assert_shallow("deps/git_repo", 1) | ||
|
||
assert File.read!("mix.lock") =~ "submodules: true" | ||
# TODO: assert submodule is not shallow. This would likely require | ||
# changes to the fixtures. Apparently, not even the submodules-specific | ||
# tests check that the cloned repo contains submodules as expected. | ||
end) | ||
end | ||
|
||
defp update_dep(git_repo_opts) do | ||
# Flush the errors we got, move to a clean slate | ||
Mix.shell().flush() | ||
Mix.Task.clear() | ||
Process.put(:git_repo_opts, git_repo_opts) | ||
Mix.Project.pop() | ||
Mix.Project.push(GitApp) | ||
end | ||
|
||
defp assert_shallow(repo_path, depth) do | ||
assert File.read!("mix.lock") =~ "depth: #{depth}" | ||
|
||
# Check if the repository is a shallow clone | ||
assert File.exists?(repo_path <> "/.git/shallow") | ||
|
||
# Check the number of commits in the current branch. | ||
# | ||
# We could consider all branches with `git rev-list --count --all`, as in | ||
# practice there should be only a single branch. However, the test fixture | ||
# sets up two branches, and that brings us to an interesting situation: | ||
# instead of guaranteeing that the `:depth` option would keep the | ||
# repository lean even after refspec changes, we only guarantee the number | ||
# of commits in the current branch, perhaps leaving more objects around | ||
# than strictly necessary. This allows us to keep the implementation | ||
# simple, while still providing a reasonable guarantee. | ||
assert System.cmd("git", ~w[--git-dir=#{repo_path}/.git rev-list --count HEAD]) == | ||
{"#{depth}\n", 0} | ||
end | ||
end | ||
|
||
defp refresh(post_config) do | ||
%{name: name, file: file} = Mix.Project.pop() | ||
Mix.ProjectStack.post_config(post_config) | ||
|
Uh oh!
There was an error while loading. Please reload this page.