-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Allow repo_contents_cache in WORKSPACE if bazelignored #26522
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
base: master
Are you sure you want to change the base?
Allow repo_contents_cache in WORKSPACE if bazelignored #26522
Conversation
258d469 to
f473eed
Compare
|
Could you add a test for this? / Have you verified this works? One way to make sure is to remove this line: https://cs.opensource.google/bazel/bazel/+/master:scripts/bootstrap/bootstrap.sh;drc=e6979d3f878ddd663f15a10b09407ecbb9380fee;l=42 (or set it to some other value), create a .bazelignore file around here to exclude the repo contents cache dir, and make sure |
bef7328 to
0ee146d
Compare
|
I have given a shot at implementing dedicated test for this behavior (allowing I did not follow Your particular suggestion of testing, because I felt like it would not be as clean and direct as dedicated check. Of course, if there is something I missed / not considered, I will be very happy to fix / change that. I have also tested the changes by running the binary built from this branch, on my WORKSPACES that use the pattern of |
0ee146d to
08e6c15
Compare
This change allows the `repo_contents_cache` to be set to a path within the current WORKSPACE, as long as said path is excluded from Bazel evaluation via `.bazelignore` entry.
This changeset aims to validate the expected behavior of `--repo_contents_cache` when set to path within current WORKSPACE; namely to fail if it is not .bazelignored and to work if it is.
08e6c15 to
e7aacc7
Compare
|
Hey @Wyverald , |
|
Sorry about the delay! Thanks for adding that test; however, what I'm worried about is not the code you added not functioning correctly, but rather whether |
|
Thanks for explanation - I will move the PR into draft mode and ensure things work as they should before re-opening. |
### What does this PR do? Now `bazelisk` is available on all our CI executors ([macOS](DataDog/ci-platform-machine-images#395) runners, [Linux](DataDog/datadog-agent-buildimages#951) & [Windows](DataDog/datadog-agent-buildimages#953) containers), this change secures an initial part of our `bazel` setup while providing reusable job templates to ease caching in CI. Practically, it adds a handful of jobs focusing on **building `bazel` dependencies** in the corresponding GitLab `deps_build` stage. Overall, it consists in: 1. verifying **`bazelisk` properly bootstraps `bazel` across all platforms**, (primary scope of the PR) 2. extracting initial `bazel:`-prefixed CI job templates and dogfooding them, (i.e. by building deps) 3. binding `bazelisk`/`bazel` caches to GitLab caching capabilities (runner-based as of now): installed binaries, "repository cache", "repo contents cache", and "disk cache" are all saved/restored correctly to/from GitLab while honoring OS/architecture boundaries, 4. marking in-workspace cache in both `.bazelignore` and `.gitignore`, 5. adjusting code/job ownership accordingly. ### Motivation Securing some initial `bazel` configuration in CI : - ensures all platforms are kept in the radar over next iterations, - prevents regressions over next iterations, - establishes foundations for future jobs. ### Possible Drawbacks / Trade-offs - on Windows, compilation errors made be descope dependencies being built to **only `bzip2`**. This will be of course addressed in a subsequent PR, - GitLab caching is still runner-based for the time being, which can be revisited later. (like everything) ### Additional Notes Main addressed challenges: - cache location conflicts[^1]: externalized "repo contents cache" through `tools/bazel*` wrappers: - bazelbuild/bazel#26384 - bazelbuild/bazel#26522 - bazelbuild/bazel#26773 - bazelbuild/bazel#26802 - macOS runners: - `bzip2` dependency: fetch from a reachable source: - [x] #40219 - Windows: - `bazel` spawn strategy: fallback to a permissive strategy since `sandboxed` is unsupported - [x] #40328 - `tools/bazel` wrapper: fallback to batch (`tools/bazel.bat`), since `bash` is discouraged by `bazel` in this case and `tools/bazel.ps1` poses detection problems, - long paths were supported in containers but not on runners: - [x] DataDog/ci-platform-machine-images#429 - recursive symlinks: use `robocopy` instead of `copy`/`move`/`xcopy`. [^1]: > ERROR: The repo contents cache [/path/to/datadog-agent/.cache/repo_contents] is inside the workspace [/path/to/datadog-agent]. This can cause spurious failures. Disable the repo contents cache with `--repo_contents_cache=`, or specify `--repo_contents_cache=<path outside the workspace>`.
This change allows the
repo_contents_cacheto be set to a path within the current WORKSPACE, as long as said path is excluded from Bazel evaluation via.bazelignoreentry.Closes #26384