Skip to content
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

Consider "bazel sync" instead of "bazel fetch" #6140

Closed
coryan opened this issue Mar 30, 2021 · 3 comments
Closed

Consider "bazel sync" instead of "bazel fetch" #6140

coryan opened this issue Mar 30, 2021 · 3 comments
Labels
type: cleanup An internal cleanup or hygiene concern.

Comments

@coryan
Copy link
Contributor

coryan commented Mar 30, 2021

We use bazel fetch in our (Bazel) CI builds to prevent flakes caused by failed downloads. It seems like we could simplify this by using bazel sync instead. fetch requires listing the rules that will be executed, and some indirect dependencies. sync seems to download things we do not need, e.g., the SDKs for Windows when compiling on Linux.

@coryan coryan added the type: cleanup An internal cleanup or hygiene concern. label Mar 30, 2021
@devjgm
Copy link
Contributor

devjgm commented Aug 13, 2021

I think we want to keep bazel fetch not bazel sync. The later downloads more deps than are necessary. However, I think we can simplify our bazel fetch invocation from

"ci/retry-command.sh" 3 120 bazel fetch ... \
@local_config_platform//... \
@local_config_cc_toolchains//... \
@local_config_sh//... \
@go_sdk//... \
@remotejdk11_linux//:jdk

To simply bazel fetch ....

I think the simpler invocation is supposed to download all the deps that are necessary. And in an experiment on my local machine, it appeared to do just that. I did a clean bazel build ..., and looked at all the files downloaded in ~/.cache/bazel/_bazel_jgm/cache/repos/. Then I expunged everything and did a bazel fetch ..., and it fetched everything that the bazel build fetched.

devjgm added a commit to devjgm/google-cloud-cpp that referenced this issue Aug 13, 2021
Fixes: googleapis#6140

We only need to `bazel fetch ...`, which should get everything we need
for a `bazel build ...`. The extra packages that we previously had
listed were either redundant, or unnecessary, for example, a `bazel
build ...` never downloaded (or needed) `@go_sdk//...` in my local
experiments.
@devjgm
Copy link
Contributor

devjgm commented Aug 13, 2021

Well, I just found #5346, which seems to suggest that I'm wrong... let me do a bit more investigation.

@devjgm
Copy link
Contributor

devjgm commented Aug 13, 2021

I think we should be able to only specify bazel fetch ... and have it get everything, but the PR above is correct that we need to list more. I think that's a bazel bug and filed: bazelbuild/bazel#13847.

Regardless, for the purpose of this issue, we want to keep bazel fetch. bazel sync is not what we want because it fetches way too much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

No branches or pull requests

2 participants