Skip to content

Enable RJE_UNSAFE_CACHE by default - #1584

Merged
shs96c merged 10 commits into
bazel-contrib:masterfrom
shs96c:enable-unsafe-cache
Jul 3, 2026
Merged

Enable RJE_UNSAFE_CACHE by default#1584
shs96c merged 10 commits into
bazel-contrib:masterfrom
shs96c:enable-unsafe-cache

Conversation

@shs96c

@shs96c shs96c commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

The shared cache is now used unless RJE_UNSAFE_CACHE is set to "0" or "false". The CLI flag --use_unsafe_shared_cache has been replaced by --use_unique_cache, which opts out of the shared cache.

Maven and Gradle docs updated accordingly. Coursier behaviour is unchanged for now.

@shs96c
Enable RJE_UNSAFE_CACHE by default
1c14d20 
The shared cache is now used unless RJE_UNSAFE_CACHE is set to "0" or
"false". The CLI flag --use_unsafe_shared_cache has been replaced by
--use_unique_cache, which opts out of the shared cache.

Maven and Gradle docs updated accordingly. Coursier behaviour is
unchanged for now.
@shs96c
shs96c requested review from cheister and jin as code owners June 9, 2026 15:40
shs96c added 8 commits June 9, 2026 17:25
@shs96c
Isolate HOME for unpinned m2local test
d680efa 
Earlier resolver pin steps now write to ~/.m2 (because RJE_UNSAFE_CACHE
is enabled by default) without .sha1/.md5 sidecars, which trips up
Coursier's m2local checksum check during this test's unpinned repin.
Temporarily override HOME for the test so the m2local scan sees only
the artifacts the test itself publishes.

Long-term fix is to have the Downloader fetch checksum sidecars when
caching to the shared local repository.
@shs96c
Put temp HOME under real HOME so linux-sandbox is happy
ca0c53a 
mktemp -d defaults to /tmp, and CI's linux-sandbox refuses an
output_base under /tmp (it's mounted as tmpfs). Anchor the temp HOME
under the original HOME instead.
@shs96c
Forward HOME into repo-rule subprocesses via --repo_env
2e2cb14 
Without this, coursier.bzl invokes Coursier with environment={} which
clears the shell env entirely; Java's user.home then falls back to the
buildkite-agent user's /etc/passwd entry and Coursier scans the real
~/.m2 instead of the test's isolated one.
@shs96c
Use surgical artifact cleanup in m2local test
033c989 
Revert the HOME-redirect and --repo_env experiments, and instead
follow the same convention every other m2local test uses: rm -rf
just the published artifact's directory before and after the run.

Also fix a latent bug where jar_dir pointed at com/example/kt/1.0.0,
even though this test publishes com.example:no-docs:1.0.0.
@shs96c
Work around caching issues in test without nuking the user ~/.m2 dir
51c9b83
@shs96c
Better targetting of HOME
36366d5
@shs96c
Make sure we track the env vars we use for repo rules
9aade52
@shs96c
Once more
0ef7819
@cheister

cheister commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Why are we changing the default to use something that was previously deemed unsafe?

@shs96c

shs96c commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

TL;DR: we're making the gradle and maven resolvers act like coursier does.

The coursier resolver hasn't been paying attention to this for a long time. On macOS, coursier caches files in ~/Library/Caches/Coursier/v1, and because pinning happens via bazel run the user's env variables are used. If you've ever needed to repin a repo with a lot of dependencies more than once on the same machine, you'll find it happens suspiciously quickly. That's why.

The gradle resolver delegates down to Gradle, which will correctly populate the gradle cache, so it's a safe operation to do there. Similarly, the maven resolver uses Maven's own classes for this: it downloads the POMs to the right places.

We recently landed changes that attempt to use a stable gradle home between invocations (#1473) Because that directory was stable, even though we weren't using the user's cache, we were using the same cache repeatedly.

@shs96c

shs96c commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

The reason we viewed the repository cache as "unsafe" is because other tools could (and would) write to and modify it. I don't think that's the right call any more, otherwise a lot of maven and gradle developers would be having a very bad time of it.

cheister
cheister reviewed Jun 15, 2026
View reviewed changes
Comment thread README.md Outdated
cheister
cheister reviewed Jun 15, 2026
View reviewed changes
Comment thread private/rules/coursier.bzl
@shs96c @claude
Rename resolver flag to --use_isolated_cache and clarify cache docs
b3e9a7a 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@shs96c
shs96c merged commit 06db7d7 into bazel-contrib:master Jul 3, 2026
10 checks passed
@shs96c
shs96c deleted the enable-unsafe-cache branch July 3, 2026 14:26
vinnybod added a commit to confluentinc/rules_jvm_external that referenced this pull request Oct 13, 2026
@vinnybod @Andrius-B
@titusfortner @shs96c @claude @styurin @collinfunk @tharakadesilva @dzbarsky @Bencodes @Dolfik1 Nikolay @JonathanPerry651 @mikhail-0330 @keith @timothyklim Ted Kaplan @aNOOBisTheGod @vadikmironov @fmeum @pettermahlen
Merge upstream rules_jvm_external 7.2 (#87)
f00ccf0 
* Prefer getenv to env.get (bazel-contrib#1568)

* update environment variable handling for pinning (bazel-contrib#1573)

* Fix MavenPublisher to support POM-only publications (bazel-contrib#1574)

* Bump the deps in the Spring Boot examples (bazel-contrib#1580)

* Update remaining examples (bazel-contrib#1581)

This involves rewriting the Android Kotlin app to avoid AppCompat as otherwise
the `install` tag became a mess, and that's not what we're trying to demonstrate.

* Update Gradle to 9.5.0 (bazel-contrib#1569)

Gradle 9.x requires JVM 17 to run, so this also bumps the project's
default Java toolchain in .bazelrc from 11 to 17. Downstream projects
can continue targeting Java 11 with their own .bazelrc, though using
the Gradle resolver will require JVM 17+.

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

* Fix link to docs/bzlmod.md in README.md (bazel-contrib#1564)

* Fix GitHub search query in README (bazel-contrib#1550)

The `filename:` query was a part of a legacy implementation of code
search. The current equivalent is `path:` [1].

[1] github/docs#24666

* fix: resolve concurrent NPE in ErrorReportingListener (bazel-contrib#1152) (bazel-contrib#1546)

Use Collections.synchronizedList and synchronized block in
getExceptions() to prevent NullPointerException when multiple
threads add exceptions concurrently during Maven resolution.

Extract common addExceptionIfPresent method with null-safe event check.
Add ErrorReportingListenerTest to verify thread safety.

Closes bazel-contrib#1152

* Run jvm_import jar tools with exec Java (bazel-contrib#1583)

Invoke AddJarManifestEntry_deploy.jar with current_java_runtime in the exec configuration instead of executing the AddJarManifestEntry java_binary launcher. This preserves --tool_java_runtime_version and selects Java for the execution platform when the target and execution platforms differ.

Treat AddJarManifestEntry_deploy.jar and JavaRuntimeInfo.files as action tools.

Add jvm_import_uses_execution_platform_java_runtime_test with an Android s390x target platform. Android s390x has no Java runtime toolchain, so the test fails if StampJarManifest or CreateCompileJar resolves Java against the target platform. The test also verifies that both actions pass AddJarManifestEntry_deploy.jar to Java.

* Refactor: carry a `ResolvedArtifact` for every resolution node when resolving (bazel-contrib#1585)

Replace the `Map<Coordinates, Path>` side-channel on `ResolutionResult`
with a `Map<Coordinates, ResolvedArtifact>` that has an entry for every
node in the resolved graph. Each `ResolvedArtifact` holds its coordinates
and an optional path, so "this node has no known binary" is represented
explicitly rather than inferred from an absent map entry.

This is a pure refactoring: the Maven and Gradle resolvers populate the
new shape from exactly the information they already had, the downloader
still receives the same set of known paths, and the lock file, its
format, and the resolved-artifact hashes are all unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* Make `amend_artifact` also match BOMs (bazel-contrib#1586)

`amend_artifact` previously searched only the `artifacts` list, so a BOM
(declared via `install(boms = ...)` or split out of a `from_toml`
`libs.versions.toml` via `bom_modules`) could not be amended. The
amendment failed with "No artifact found matching coordinates" because
BOMs are tracked in a separate list that the amendment never searched.

`amend_artifact` now searches both the artifacts and boms lists, so a
BOM's version and other properties can be adjusted through the same
mechanism as a regular artifact (for example, force-pinning a BOM
version so transitive deps can't silently upgrade it).

Add unit tests covering amendment of artifacts, BOMs, and the
no-match case.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

* Allow boms to be defined in the gradle version catalogues (bazel-contrib#1543)

* Apply Maven exclusions to generated deps (bazel-contrib#1594)

* Fix BOM-only pinning (bazel-contrib#1592)

* Fix null shasum artifacts in v3 locks (bazel-contrib#1591)

* Preserve compound archive extensions when publishing (bazel-contrib#1590)

* Add `publish-to-bcr` automation (bazel-contrib#1587)

* Fix resolver NPE for no-binary artifacts (bazel-contrib#1595)

* Fix TOML AAR overrides using original artifact copy targets (bazel-contrib#1600)

* fix: stop Gradle daemons after dependency resolution (bazel-contrib#1598)

Closing the ProjectConnection alone leaves the Tooling API's daemon
running for reuse. Call disconnect() on the connector so the daemon
is shut down once pinning completes.

* Index top-level kotlin functions, properties and type aliases properly. (bazel-contrib#1599)

Top-level Kotlin functions, properties, and type aliases compile into a synthetic <File>Kt facade, so they were invisible to the class-name index and could not be attributed to an artifact in split packages.

Read them from the @kotlin.Metadata annotation via kotlin-metadata-jvm and fold the public ones into the class index under their Kotlin source names.

* Skip package-info.class when indexing jars (bazel-contrib#1597)

`package-info.class` files only hold package-level annotations and
cannot be referenced from production code, so they should never
appear in the indexed class set.

* Don't deduplicate additional_coursier_options (bazel-contrib#1596)

Co-authored-by: Nikolay <n.matyushin@tawasal.ae>

* Enable RJE_UNSAFE_CACHE by default (bazel-contrib#1584)

The shared cache is now used unless RJE_UNSAFE_CACHE is set to "0" or
"false". The CLI flag --use_unsafe_shared_cache has been replaced by
--use_unique_cache, which opts out of the shared cache.

Maven and Gradle docs updated accordingly. Coursier behaviour is
unchanged for now.

---------

Co-authored-by: Claude <noreply@anthropic.com>

* fix: make :outdated work when runfiles trees are disabled (bazel-contrib#1602)

`bazel run @maven//:outdated` failed with "Unable to access jarfile"
under --noenable_runfiles (the default on Windows) because the
generated sh_binary passed cwd-relative rootpaths, which only resolve
inside a materialised runfiles tree. Resolve the arguments with the
Bash runfiles library instead, as :pin already does.

Co-authored-by: Claude <noreply@anthropic.com>

* Preserve Gradle variant identity in the lock graph (bazel-contrib#1576)

The Gradle resolver was flattening multiple selected variants of the same
module into a single dependency node. When a module was resolved both as its
main jar and as test fixtures, we merged their outgoing dependencies and then
stamped that merged child set onto both output coordinates. That can create
self loops and main-to-test-fixtures back-edges in the generated lockfile,
which Bazel cannot repair downstream.

Fix this by keeping variant identity through resolution so each selected
variant keeps its own dependency set. Variants are classified and artifacts
are attached to them using Gradle capabilities, the canonical identity for
feature variants, so any capability-based variant is handled, not just test
fixtures. Requested test-fixtures classifiers resolve through Gradle's
testFixtures(...) notation instead of being treated like an ad hoc classifier
fetch.

* feat: Support pluggable dependency download and metadata caching SPIs (bazel-contrib#1589)

Provides pluggable SPIs for rules_jvm_external's dependency resolution, allowing custom download/caching and authentication implementations (such as integrating with Bazel's remote fetcher/downloader APIs) to be registered via classpath plugins.

Key Changes:
1. Public API Targets & SPIs:
   - Created public interfaces for MetadataService, DependencyMetadata, and DownloadService.
   - Added a generic SpiLoader utility to load classpath SPI service implementations (ensuring at most one implementation is loaded per SPI).
   - Exposed custom classpath library loading via the new resolver_extra_dependencies attribute.
   - Decomposed the API targets into fine-grained libraries: :api_core, :download_service, :metadata_service, and :spi_loader.
   - Relocated the build definition closer to the sources in //resolver/src/main/java/com/github/bazelbuild/rules_jvm_external/resolver/BUILD.
   - Flattened public SPI classes into a single package (com.github.bazelbuild.rules_jvm_external.resolver) to prevent split-package issues and avoid spanning multiple packages.

2. Aether & Downloader SPI Integration:
   - Replaced Aether's default HTTP transport with a custom HttpDownloaderTransporterFactory, routing all POM, BOM, and metadata lookups through the pluggable DownloadService SPI.
   - Extracted HttpDownloaderTransporter into its own top-level package-private class.
   - Renamed DownloadService.download to get to match HTTP verb naming conventions and align with head.
   - Implemented null-safe defensive copying of collections in DependencyMetadata constructor using Set.copyOf/Map.copyOf.

3. Inline URL Credentials Handling:
   - Aether downloads are now routed through the pluggable DownloadService SPI backed by HttpDownloader without inline credentials support (making both resolution and downloading fail consistently). Users must configure credentials via .netrc instead.
   - Deleted the shouldDownloadOverHttpWithAuthenticationPassedInOnRepoUrl test from ResolverTestBase.java since that flow does not work and is explicitly unsupported.

4. Cleanup & Optimization:
   - Removed final modifier from SpiLoader class and refactored joining logic to use Guava Joiner.
   - Removed redundant metadata caches and unused resolver binaries.

* fix: honour pinned versions in external resolvers (bazel-contrib#1606)

* BOM/POM coordinates condition marked as `skipped` must not be turned into `http_file downloads (bazel-contrib#1604)

* Use a UTF-8 locale for jvm_import jar actions (bazel-contrib#1610)

* fix: force a single version per Gradle module when pinning (bazel-contrib#1608)

Gradle selects one version per group:artifact and ignores classifiers,
so forcing each classified root separately (e.g. a main jar and its
test-fixtures jar at different versions) made resolution unsatisfiable.
Group roots by module and force only the defining version: the
unclassified root, or the highest classified request.

Also stop the resolver plugin's detached-configuration retry from
undermining pins: skip it when the main configuration fully resolved,
and re-apply the forced modules when it runs, since detached
configurations never see `configurations.all`.

Co-Authored-By: Claude <noreply@anthropic.com>

* Prepare for 7.1 release (bazel-contrib#1593)

Making the example build under Bazel 9 needed protobuf pinned back to
33.4 (35.0 requires Bazel >= 8, which was breaking the 7.x cell), an
explicit `rules_java` bazel_dep, and `load` statements for `java_library`
and `java_proto_library` since Bazel 9 no longer autoloads them.

* Return repo_metadata from repository rules (bazel-contrib#1616)

* Remove -noverify (bazel-contrib#1620)

-noverify was removed in JDK 27 (deprecated since JDK 13) and now causes the launcher to fail with an unrecognized option error. It was only needed for proguarded coursier JARs from very old releases: coursier/coursier#1245

* perf: speed up java export exclusions (bazel-contrib#1624)

Signed-off-by: Simon Mavi Stewart <simon.m.stewart@gmail.com>

* fix: Improve Gradle resolver console output (bazel-contrib#1623)

* build: update rules_kotlin to 2.4.0 (bazel-contrib#1621)

* feat: expose kt_defs.bzl via public bzl_library for stardoc (bazel-contrib#1626)

* fix: make Gradle resolver cache writes race-tolerant (bazel-contrib#1615)

When pinning a large dependency graph with resolver = "gradle", the pin
aborted non-deterministically with FileAlreadyExistsException while
copying a freshly-downloaded artifact into the local cache. The failing
artifact differed from run to run: whichever one lost a race between
parallel downloader threads.

For the Gradle resolver, localRepository is the shared Gradle module
cache, which the Gradle daemon and other worker threads write to
concurrently. Downloader.performDownload cached downloads with
Files.copy(REPLACE_EXISTING), which is not atomic: REPLACE_EXISTING only
removes a target present at check time, so a target created in the TOCTOU
window makes the copy fail. A plain copy also lets another worker observe
a half-written cache file through the Files.exists check at the top of
performDownload.

Cache to a temp file in the destination directory, then atomically move
it into place. These artifacts are content-addressed, so a destination
another writer already produced is success, and FileAlreadyExistsException
on the move is ignored.

* Handle overrides for fileless artifacts (bazel-contrib#1625)

* Pass COURSIER_CACHE to coursier when fetching unpinned repositories (bazel-contrib#1601)

Coursier derives its default cache location from the JVM's `user.home`,
which on Linux comes from the passwd database and ignores $HOME, while
these rules derive the shared cache location from $HOME. When the two
disagree (e.g. $HOME remapped to a network mount), repinning bypasses
the shared cache and fails with "Error while trying to parse the path
of file in the coursier cache".

Pass the computed location to the spawned coursier process via
COURSIER_CACHE, as the pinned flow already does. If no variable a cache
location can be derived from is set, leave the environment untouched.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* fix: don't fail resolution when an artifact has no sources jar (bazel-contrib#1614)

When fetch_sources = True and an artifact publishes a main jar but no
sources jar (for example the empty com.google.guava:listenablefuture jar
that Guava depends on), the Maven and Gradle resolvers' shared download
phase threw UriNotFoundException instead of treating the sources jar as
absent, failing the whole resolution.

Downloader.download now returns null for a missing sources/javadoc jar so
the caller can skip it, matching the `if (sdm != null)` guard already in
AbstractMain.getDependencyInfos.

Adds a DownloaderTest unit reproducer plus end-to-end guava installs
exercised via both the Maven and Gradle resolvers.

* Use rules_pkg in the java-export example (bazel-contrib#1628)

The example loaded `pkg_tar` from `@bazel_tools//tools/build_defs/pkg:pkg.bzl`, which has been deprecated in favor of rules_pkg for years and is slated for removal from `@bazel_tools`.

* Encapsulate bzlmod dependency layering (bazel-contrib#1618)

* Add support for AAR files to IndexJar (bazel-contrib#1490)

Fixes bazel-contrib#1489

* fix: apply remaining bzlmod layering review feedback from bazel-contrib#1618 (bazel-contrib#1629)

Filter testonly non-root artifacts before the conflicting-force check so
that declarations layering drops cannot fail the build. Report equal-version
force displacement with an RJE_VERBOSE-gated INFO diagnostic; the non-root
declaration previously replaced the root's silently. Rename the
addtional_artifact_message variable.

* fix: honour pinned versions in layered coursier installs (bazel-contrib#1631)

* fix(publish): Apparently we need a specific version of the publish yaml (bazel-contrib#1612)

* chore: prep for the 7.2 release (bazel-contrib#1633)

---------

Signed-off-by: Simon Mavi Stewart <simon.m.stewart@gmail.com>
Co-authored-by: Andrius Bankauskas <and.bankauskas@gmail.com>
Co-authored-by: Titus Fortner <titusfortner@users.noreply.github.com>
Co-authored-by: Simon Mavi Stewart <simon.m.stewart@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Sergey Tyurin <sergey.v.tyurin@gmail.com>
Co-authored-by: Collin Funk <collin.funk1@gmail.com>
Co-authored-by: Tharaka De Silva <tharaka.uo@gmail.com>
Co-authored-by: David Zbarsky <dzbarsky@gmail.com>
Co-authored-by: Ben Lee <ben@ben.cm>
Co-authored-by: Nikolay M <dolphinn@bk.ru>
Co-authored-by: Nikolay <n.matyushin@tawasal.ae>
Co-authored-by: JonathanPerry651 <jonathan.perry@gs.com>
Co-authored-by: mikhail-0330 <145472039+mikhail-0330@users.noreply.github.com>
Co-authored-by: Keith Smiley <keithbsmiley@gmail.com>
Co-authored-by: timothy <387270+timothyklim@users.noreply.github.com>
Co-authored-by: Ted Kaplan <tkaplan@roku.com>
Co-authored-by: Vasilii Novozhilov <69043738+aNOOBisTheGod@users.noreply.github.com>
Co-authored-by: vadikmironov <77026200+vadikmironov@users.noreply.github.com>
Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
Co-authored-by: Petter Måhlén <pettermahlen@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cheister cheister cheister approved these changes
@jin jin Awaiting requested review from jin jin is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@shs96c @cheister