shallow_since appears to be a trap when used with commit, debug line should be removed

[djmarcin]

Description of the problem / feature request:

The shallow_since option to new_git_repository appears to be a trap, at least when used with the commit option. When shallow_since is provided, instead of using depth 1 to check out only the relevant commit, the entire repo state since some timestamp is cloned. On one of our repos, this causes a 25x difference in clone time (7s -> 175s) and presumably a large size disparity as well.

Now that we have removed shallow_since, bazel emits a debug line encouraging us to add it back.

DEBUG: Rule 'foo' indicated that a canonical reproducible form can be obtained by modifying arguments shallow_since = "1611092265 -0800"

This seems wrong -- as far as I understand, the repo at a specific commit hash is a canonical reproducible form, so it seems like bazel should not be suggesting this change.

Feature requests: what underlying problem are you trying to solve with this feature?

Bazel should not recommend adding shallow_since since it is dramatically slower in some cases, and seems generally unnecessary to obtain a snapshot of the repository.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Create a new_git_repository rule for a large git repository at a commit that is not close to HEAD. Providing shallow_since will slow down the clone substantially.

What operating system are you running Bazel on?

Linux, Ubuntu 20.04

What's the output of bazel info release?

INFO: Invocation ID: 6c8f5dce-cf89-4b6f-a335-93ff77612968
release 3.7.1

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

n/a

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

n/a (private repo)

Have you found anything relevant by searching the web?

No

Any other information, logs, or outputs that you want to share?

n/a

[thii]

+1

Also git's --shallow-since has a bug that can sometimes make git impossible to fetch the desired commit using its suggested timestamp: #10292.

[asuffield]

There is no way to persuade git to fetch a specific commit with --depth=1. If you try fetching a commit hash, most servers will just give you "server does not allow request for unadvertised object", and if you fetch a ref name then it can return anything and is not reproducible.

shallow_since is the only feature git offers to do this. Unfortunately, it's completely broken.

[asuffield]

Correction: --depth=1 on a single commit only works with protocol v2, see #12199

[djmarcin]

@asuffield I don't think we're on protocol v2 (although I'm not completely sure how to verify that) and we've been using the commit only since I posted the issue without any apparent problems or non-reproducibility. As far as I can tell, the actions bazel is taking are equivalent to the following commands, which appear to work for me.

mkdir ruby ; cd ruby
git init
git remote add origin git@github.com:ruby/ruby.git
# A random old commit
git fetch origin 5de38c41ae7bf17ae599fdfa9f8face87f16d8bb --depth 1
git checkout 5de38c41ae7bf17ae599fdfa9f8face87f16d8bb

Do you mean that it doesn't work reliably with all git servers, so maybe it works on github but not elsewhere?

$ git --version
git version 2.25.1
$ echo $GIT_PROTOCOL

[djmarcin]

It looks like maybe the server config option uploadpack.allowReachableSHA1InWant needs to be set to enable this, but the repository rules already seem to handle falling back when the server doesn't support this option anyway. shallow_since is perhaps better than the fallback option, but it's probably worse than fetching the commit directly when supported, so it shouldn't be recommended when fetching by commit is supported.

bazel/tools/build_defs/repo/git_worker.bzl

Line 129 in 8346ea4

# Perhaps uploadpack.allowReachableSHA1InWant or similar is not enabled on the server;

[asuffield]

It definitely fails with git 2.18 (ugh, upgrading that right now) against gitlab, I haven't tested exhaustively. But the main problem in the PR linked above is that one of bazel's target platforms has an old git version, and that's going to sabotage us here.

[CodingCanuck]

Are there ways to improve this situation, even given bazel's support for ubuntu 18.04 and its associated git version?

One option is simply to drop the warning from this rule (by making the rule output a canonical version that omits shallow_since if the caller did not specify it).

Another is to add something like an explicit depth arg to the git_repository rules: I don't like that from an API perspective, but it could allow callers to explicitly use a depth-based canonical form (rather than a shallow_since-based form) if that works for their environments.

I think that any change to the canonical form of this rule would involve changing what's returned from the git_repo function.

Are there other ideas that could remove warnings from a correct repository setup that's relying on depth=1 rather than a shallow_since date?