Skip to content

Commit

Permalink
Do not recommend shallow_since for git_repository
Browse files Browse the repository at this point in the history
Git's `--shallow-since` feature can lead to broken checkouts and a fix isn't likely to happen:
https://public-inbox.org/git/CAPSG9dZV2EPpVKkOMcjv5z+NF7rUu=V-ZkZNx47rCv122HsiKg@mail.gmail.com/T/#u

This commit removes `shallow_since` from the canonical reproducible form of `git_repository` unless the attribute is provided explicitly. In cases where Bazel's attempt at fetching with `--depth=1` fail, e.g. if there is no server support for shallow fetches of arbitrary commits, the progress message is now used to inform the user about this potential for fetch time optimizations.

Fixes #10292
Fixes #12857

Closes #17356.

PiperOrigin-RevId: 508569071
Change-Id: I01e6662e38c236d1800d427f66d48a05df818800
  • Loading branch information
fmeum authored and copybara-github committed Feb 10, 2023
1 parent 79fc488 commit dde6d20
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 9 deletions.
23 changes: 16 additions & 7 deletions tools/build_defs/repo/git.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ def _clone_or_update_repo(ctx):
for item in ctx.path(dest_link).readdir():
ctx.symlink(item, root.get_child(item.basename))

return {"commit": git_.commit, "shallow_since": git_.shallow_since}
if ctx.attr.shallow_since:
return {"commit": git_.commit, "shallow_since": git_.shallow_since}
else:
return {"commit": git_.commit}

def _update_git_attrs(orig, keys, override):
result = update_attrs(orig, keys, override)
Expand All @@ -68,12 +71,14 @@ _common_attrs = {
"shallow_since": attr.string(
default = "",
doc =
"an optional date, not after the specified commit; the " +
"argument is not allowed if a tag is specified (which allows " +
"cloning with depth 1). Setting such a date close to the " +
"specified commit allows for a more shallow clone of the " +
"repository, saving bandwidth " +
"and wall-clock time.",
"an optional date, not after the specified commit; the argument " +
"is not allowed if a tag or branch is specified (which can " +
"always be cloned with --depth=1). Setting such a date close to " +
"the specified commit may allow for a shallow clone of the " +
"repository even if the server does not support shallow fetches " +
"of arbitary commits. Due to bugs in git's --shallow-since " +
"implementation, using this attribute is not recommended as it " +
"may result in fetch failures.",
),
"tag": attr.string(
default = "",
Expand Down Expand Up @@ -183,6 +188,10 @@ makes its targets available for binding. Also determine the id of the
commit actually checked out and its date, and return a dict with parameters
that provide a reproducible version of this rule (which a tag not necessarily
is).
Bazel will first try to perform a shallow fetch of only the specified commit.
If that fails (usually due to missing server support), it will fall back to a
full fetch of the repository.
""",
)

Expand Down
11 changes: 9 additions & 2 deletions tools/build_defs/repo/git_worker.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ def git_repo(ctx, directory):
recursive_init_submodules = ctx.attr.recursive_init_submodules,
)

ctx.report_progress("Cloning %s of %s" % (reset_ref, ctx.attr.remote))
if (ctx.attr.verbose):
_report_progress(ctx, git_repo)
if ctx.attr.verbose:
print("git.bzl: Cloning or updating %s repository %s using strip_prefix of [%s]" %
(
" (%s)" % shallow if shallow else "",
Expand All @@ -95,6 +95,12 @@ def git_repo(ctx, directory):

return struct(commit = actual_commit, shallow_since = shallow_date)

def _report_progress(ctx, git_repo, *, shallow_failed = False):
warning = ""
if shallow_failed:
warning = " (shallow fetch failed, fetching full history)"
ctx.report_progress("Cloning %s of %s%s" % (git_repo.reset_ref, git_repo.remote, warning))

def _update(ctx, git_repo):
ctx.delete(git_repo.directory)

Expand Down Expand Up @@ -133,6 +139,7 @@ def fetch(ctx, git_repo):
# "ignore what is specified and fetch all tags".
# The arguments below work correctly for both before 1.9 and after 1.9,
# as we directly specify the list of references to fetch.
_report_progress(ctx, git_repo, shallow_failed = True)
_git(
ctx,
git_repo,
Expand Down

0 comments on commit dde6d20

Please sign in to comment.