Do not recommend shallow_since for git_repository

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 bazelbuild#10292
Fixes bazelbuild#12857

tools/build_defs/repo/git.bzl

@@ -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)
@@ -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 = "",
@@ -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.
 """,
 )
 

tools/build_defs/repo/git_worker.bzl

@@ -79,22 +79,27 @@ 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 "",
                   ctx.name,
                   ctx.attr.strip_prefix if ctx.attr.strip_prefix else "None",
               ))
-
     _update(ctx, git_repo)
     ctx.report_progress("Recording actual commit")
     actual_commit = _get_head_commit(ctx, git_repo)
     shallow_date = _get_head_date(ctx, git_repo)
 
     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)
 
@@ -133,6 +138,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,