feat: add ability to cache json responses in-repo

[brentleyjones]

Besides a performance improvement, it allows cross-platform builds to work (e.g. Linux host building Apple code).

[brentleyjones]

@cgrindel @luispadron @jpsim I have to clean up some stuff, maybe add some tests, etc. but I want to get people's thoughts on this before I spend the time on that. We've been using this for a couple months in our Linux -> macOS RBE, and it's been nice.

[jpsim]

nit:

Suggested change

	cached_json_directory = cached_json_directory, init_submodules = init_submodules,
	cached_json_directory = cached_json_directory,
	init_submodules = init_submodules,

[jpsim]

Out of curiosity, can you please share more about the performance implications? What is improved and by how much?

[brentleyjones]

That's actually the least important part. Whatever time is taken up by swift package dump-package and swift package description is now cached. But soon I expect we can use the repo cache that was introduced in Bazel 8.3.0 to save on the common case time there.

The important part is being able to force packages to be treated the same regardless of host OS. If you cache the packages on macOS, but build on Linux, they will now use the macOS resolutions. Without this you can run into build issues, like resource bundles missing (as they aren't supported on Linux).

This doesn't make things perfect, since you might want to resolve things for Linux as well on Linux. But I think #1547 and possibly extending this caching itself to support multiple OSes would be the solution there.

[jpsim]

Thanks for the details! I don't use RBE for any of my projects, so I'm not directly impacted by the cross-platform caching aspect, so take this with a grain of salt, but wouldn't it be possible for some projects to actually prefer having the platform-specific implementations of swift package dump-package and swift package description? I can imagine some cross-platform Swift projects may want to have different dependencies or package manifest settings across macOS and Linux, and for those projects they actively wouldn't want to cache the results across macOS/Linux. It's probably the minority case though, so if there's a way to opt out of these files being cached across host OSes, that might be ok.

I'm also happy if this is something that we just handle if the need arises for it in the future.

So I'm ok with this change landing and will gladly take the perf improvements to swift package commands 👍

[brentleyjones]

If you don't set cached_json_directory, it behaves as today, so people have to opt into this behavior. It can be an pain to upgrade deps as well with the caching, we have something like this in an update script: rm -f tools/third_party/swiftpkg/*/{dump,desc}.json tools/third_party/swiftpkg/{dump,desc}.json.

[cgrindel]

Cool idea. Should we add a utility to help a client cache their JSON?

[cgrindel]

We should add a doc string.

[cgrindel]

Line length.

[cgrindel]

Can this be path.removeprefix("./")?

[claude]

Claude finished @brentleyjones's task —— View job

---

PR Review: JSON Caching Feature

Reviewing this pull request that adds JSON response caching for Swift package commands. This feature aims to improve performance and enable cross-platform builds.

Todo List

• Read and analyze all changed files
• Review Bazel rule implementation and best practices
• Assess Swift package handling correctness
• Evaluate build file generation logic
• Consider performance implications
• Check for security concerns
• Analyze test coverage needs
• Review documentation requirements
• Address existing reviewer feedback
• Provide comprehensive review feedback

Analysis Complete - Review Feedback

✅ Overall Assessment

This is a well-implemented feature that addresses a real need for cross-platform builds and performance optimization. The caching mechanism is opt-in, properly integrated across all repository rules, and follows Bazel best practices.

🔧 Bazel Rule Implementation & Best Practices

Strengths:

• ✅ Consistent parameter propagation across all repository rules (swift_package, local_swift_package, registry_swift_package)
• ✅ Proper use of repository_ctx.watch() for cache invalidation in repository_utils.bzl:112
• ✅ Opt-in design prevents breaking existing workflows
• ✅ Follows Bazel's repository rule patterns with proper attribute definitions

Areas for Improvement:

• Line 272 in swift_deps.bzl: Missing line break after cached_json_directory = cached_json_directory, as noted by @jpsim's review comment. This affects code readability.
• Missing documentation: The cached_json_directory attribute in several files lacks comprehensive documentation strings explaining the caching behavior and cross-platform implications.

🚀 Swift Package Handling Correctness

Implementation Analysis:

• ✅ Correctly caches both dump-package and describe --type json commands
• ✅ Proper path normalization using paths.join() and relative path handling
• ✅ Cache files are named consistently (dump.json, desc.json)
• ✅ Registries and SCM replacement flags are properly preserved in cached commands

Path Handling in pkginfos.bzl:1667-1668:

if path.startswith("./"):
    path = path[2:]

@cgrindel's suggestion to use path.removeprefix("./") is valid and would be more idiomatic. However, removeprefix() was added in Python 3.9, so we need to verify Bazel's Starlark supports it.

🏗️ Build File Generation Logic

Strengths:

• ✅ Cache integration is transparent to build file generation
• ✅ JSON parsing occurs before build file generation, maintaining separation of concerns
• ✅ Debug files are still generated alongside cached files for troubleshooting

Potential Concerns:

• Cache invalidation: The implementation relies on file watching, but doesn't validate cache content integrity or handle partial writes during command execution.

⚡ Performance Considerations

Benefits:

• ✅ Eliminates redundant swift package command execution
• ✅ Particularly beneficial for large dependency graphs and RBE scenarios
• ✅ Cross-platform compatibility allows Linux hosts to use macOS-resolved dependencies

Implementation Efficiency:

• ✅ Cache check occurs before expensive Swift command execution
• ✅ File operations are minimal (single file reads/writes)
• Minor concern: Multiple repository_ctx.execute() calls for directory creation could be batched

🔒 Security Considerations

File System Access:

• ✅ Cache paths are properly validated and normalized
• ✅ No arbitrary file system access outside designated cache directories
• ⚠️ Medium concern: Cache files are trusted without integrity checks. A corrupted cache file could cause build failures, but no security vulnerabilities.

Cross-Platform Implications:

• ⚠️ Design consideration: Using macOS-resolved packages on Linux could potentially miss platform-specific dependencies, but this is documented as intentional behavior for RBE scenarios.

🧪 Test Coverage Analysis

Current State:

• ❌ Major gap: No visible test coverage for the caching functionality
• ❌ No tests for cache hit/miss scenarios
• ❌ No tests for cross-platform cache usage
• ❌ No tests for cache invalidation behavior

Recommended Test Coverage:

1. Unit tests for cache file creation and retrieval
2. Integration tests for cross-platform cache sharing
3. Tests for cache invalidation when Package.swift changes
4. Tests for handling corrupted cache files
5. Performance regression tests

📚 Documentation Needs

Missing Documentation:

• Critical: cached_json_directory attribute needs comprehensive docstrings explaining:
  • Cache file format and location
  • Cross-platform implications
  • Cache invalidation behavior
  • Performance benefits and trade-offs
• Important: README or documentation updates explaining the feature and use cases

🔍 Code Quality Issues

High Priority:

1. swift_deps.bzl:272: Fix line formatting as noted in review
2. pkginfos.bzl:1668: Consider using removeprefix() if supported by Starlark version

Medium Priority:

1. Missing docstring for cached_json_directory attribute throughout multiple files
2. repository_utils.bzl:95: Line length appears to exceed style guidelines per @cgrindel's comment

🎯 Specific Recommendations

1. Add comprehensive documentation for the cached_json_directory parameter across all repository rules
2. Implement test coverage for the caching functionality
3. Consider cache integrity checks to handle corrupted files gracefully
4. Add validation for cache directory paths to prevent misuse
5. Update user documentation explaining the cross-platform build capabilities

🏁 Conclusion

This is a valuable feature that addresses real cross-platform build challenges. The implementation is sound and follows Bazel best practices. While there are opportunities for improvement in documentation and testing, the core functionality is well-designed and should provide significant benefits for users requiring cross-platform Swift package builds.

Recommendation: Approve with minor revisions - primarily documentation improvements and test coverage additions.

---