shallow_since appears to be broken

[mehrdadn]

Description of the problem / feature request:

shallow_since appears to be broken.

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

Try to build //:BUILD.boost from nelhage/rules_boost@df90835. You'll receive a recommendation for shallow_since. Insert that recommendation, and suddenly Bazel fails to find that commit at all.

What operating system are you running Bazel on?

Windows

What's the output of bazel info release?

release 1.1.0

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

N/A

Have you found anything relevant by searching the web?

No

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

Create an empty BUILD.bazel alongside the following WORKSPACE:

load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
git_repository(name="rules_boost", commit="df908358c605a7d5b8bbacde07afbaede5ac12cf", remote="https://github.com/nelhage/rules_boost")

Now run bazel --ignore_all_rc_files query "deps(@rules_boost//:BUILD.boost)":

bazel --ignore_all_rc_files query "deps(@rules_boost//:BUILD.boost)"
@bazel --ignore_all_rc_files clean --expunge

It outputs something like the following (after formatting/redactions):

INFO: Writing tracer profile to 'command.profile.gz'
DEBUG: Rule 'rules_boost' indicated that a canonical reproducible form can be obtained by modifying arguments shallow_since = "1569511515 +0200"
DEBUG: Call stack for the definition of repository 'rules_boost' which is a git_repository (rule definition at external/bazel_tools/tools/build_defs/repo/git.bzl:195:18):
 - WORKSPACE:2:1
ERROR: no such target '@rules_boost//:BUILD.boost': target 'BUILD.boost' not declared in package ''; however, a source file of this name exists.  (Perhaps add 'exports_files(["BUILD.boost"])' to /BUILD?) defined by external/rules_boost/BUILD
Loading: 1 packages loaded

Now add shallow_since="1569511515 +0200" as recommended and re-run the query:

INFO: Writing tracer profile to 'command.profile.gz'
INFO: Call stack for the definition of repository 'rules_boost' which is a git_repository (rule definition at external/bazel_tools/tools/build_defs/repo/git.bzl:195:18):
 - WORKSPACE:2:1
ERROR: An error occurred during the fetch of repository 'rules_boost':
error running 'git reset --hard df908358c605a7d5b8bbacde07afbaede5ac12cf' while working with @rules_boost:
fatal: Could not parse object 'df908358c605a7d5b8bbacde07afbaede5ac12cf'.
ERROR: no such package '@rules_boost//'
Loading: 0 packages loaded

Suddenly Bazel seems to be unable to find the commit altogether.

[thii]

We got the same issue on macOS, but it seems to be a problem with git (tried with git clone --shallow-since).

[mehrdadn]

Yeah, seems like it. Though I'd say Bazel is still responsible for avoiding recommending shallow_since unless/until this is fixed (a version check on git might make sense). Otherwise it's really confusing for the user to be recommended an option that's broken.

[laurentlb]

@aehlig Do you know what causes it?

[nelhage]

I did some digging here, and reported an upstream bug to git

The upshot is that --shallow-since is subtly incorrect when the provided timestamp would cause us to include some parents of a merge, but not others. In the case provided here, nelhage/rules_boost@df90835 has a commit timestamp of 1569511515. That commit is the second part of a merge, nelhage/rules_boost@76f3064; that merge's first parent is nelhage/rules_boost@a83197e, with an older commit timestamp of 1569487118

The bug I reported results in git discarding all parents of the merge (nelhage/rules_boost@76f3064), including the desired commit.

For now, the workaround for bazel users -- where available -- is to switch your commit= line to point to a commit on the first-parent spine, e.g. nelhage/rules_boost@76f3064. If you add the suggested shallow_since line for that commit, things work fine.

For Bazel itself, it might be nice to recognize this situation and adjust the shallow_since recommendation, at least until the upstream bug can be sorted out. I'm not certain offhand what the right/easiest heuristic to detect it is, though.

[asuffield]

It turns out that the workaround proposed here doesn't work either.

The shallow_since bug is even worse than I expected. It stops at the first commit which has any parents before the cutoff time. This means we have the following horrible scenario:

1. Latest commit on main branch is aaaa, with parents baaa and bbbb. aaaa is at 8am, baaa is at 6am and bbbb is at 4am
2. I create a git_repository rule with commit = "aaaa", shallow_since = "4am"
3. This works fine when I build it
4. Somebody adds a new commit caaa to main branch, with parents aaaa and cbbb. caaa is at 9am, cbbb is at 2am
5. git clone --shallow_since=4am will now fetch only caaa because it has a parent before 4am, and aaaa will not be fetched

This is completely unusable and I don't see a workaround.

[nelhage]

Yeah, I agree with @asuffield's analysis -- I ran into that situation since posting #10292 (comment). In light of this and of #12857 it really seems like the right path forward is to stop suggesting --shallow-since. It might be appropriate in cases where the developers know neither of these problems will apply -- or are able to debug their way out if necessary -- but it's clearly too broken to suggest as a default.

[asuffield]

I'm piecing together an alternative solution - for cases where a tag or commit hash is given, we can use point fetch with --depth=1 as long as we verify the results and correctly fallback to deep fetch if we don't get what we needed. It doesn't work on all git versions, but I don't believe there is a solution other than deep fetch for older git.

[mehrdadn]

If you auto-detect GitHub URLs you can instead just suggest HTTPS cloning. It's probably a good suggestion in any case; we switched to that as it's faster too.

[asuffield]

If you auto-detect GitHub URLs you can instead just suggest HTTPS cloning. It's probably a good suggestion in any case; we switched to that as it's faster too.

This is an interesting idea which would likely be best implemented in git itself - the protocol's rich enough to pull it off.

[mehrdadn]

git archive already exists in git. GitHub has refused to support it.

[limdor]

I would like to get some progress on this issue. Would it be fine if I create a PR to remove the debug log saying:

indicated that a canonical reproducible form can be obtained by modifying arguments shallow_since =

The issue with this log is that one a user see it the first thing will do is to set up the shallow_since, but because of this bug it is something that should consider twice.