-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Do not recommend shallow_since
for git_repository
#17356
Conversation
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
@meteorcloudy Not sure who owns the @nelhage @asuffield Given that you contributed extensively to the linked issues, it would be highly appreciated if you could take a look at this proposed solution. |
It's certainly an improvement over the status quo. To fully unbreak it we should also retry without shallow when reset fails - that's when we discover that shallow has "succeeded" and fetched junk. |
I was thinking of proposing something more radical as a follow-up (but not cherry-pickable into Bazel 6): Deprecating @asuffield Do you think |
Yes, for some future bazel version we should drop it entirely and always pull commits by hash at depth 1, when an exact commit hash is given. This does require a relatively recent version of git, since depth 1 by hash was only added a couple of years ago, but that doesn't seem unreasonable. Of course that needs the usual deprecation dance, since it'll break any current users; the best cherry pick I can think of is to retry when reset fails and just pull a full clone, since that prevents any existing builds from being surprisingly broken in the future. |
Thanks for the improvement! +1 if we can do the follow up change in a backwards compatible way. |
@meteorcloudy Do you think the current PR should be cherry-picked? |
Sure, I'm fine with cherry-picking it. |
@bazel-io flag |
@bazel-io fork 6.1.0 |
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
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 Co-authored-by: Fabian Meumertzheim <[email protected]>
Using shallow_since is not recommended, and the attribute is expected to be removed in a future Bazel version: bazelbuild/bazel#17356, bazelbuild/bazel#12857 Change-Id: I3d6daeefad609fc34566b69de67eee2a49f7d5e4 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/193473 Presubmit-Verified: CQ Bot Account <[email protected]> Commit-Queue: Auto-Submit <[email protected]> Pigweed-Auto-Submit: Ted Pudlik <[email protected]> Reviewed-by: Armando Montanez <[email protected]>
Using shallow_since is not recommended, and the attribute is expected to be removed in a future Bazel version: bazelbuild/bazel#17356, bazelbuild/bazel#12857 Original-Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/193473 https://pigweed.googlesource.com/pigweed/pigweed third_party/pigweed Rolled-Commits: 371015e36501c36..9a3bb012f2c6dc4 Roller-URL: https://ci.chromium.org/b/8755460054149076049 GitWatcher: ignore CQ-Do-Not-Cancel-Tryjobs: true Change-Id: I8952a6aa062170a57fca6a2f0420e40d7242d403 Reviewed-on: https://pigweed-review.googlesource.com/c/example/echo/+/193492 Bot-Commit: Pigweed Roller <[email protected]> Commit-Queue: Pigweed Roller <[email protected]>
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 ofgit_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