Skip to content

optimize nix build cache hits & local compile time with cargo-hakari#3163

Merged
insipx merged 1 commit intomainfrom
push-wztqyrwztzqs
Feb 11, 2026
Merged

optimize nix build cache hits & local compile time with cargo-hakari#3163
insipx merged 1 commit intomainfrom
push-wztqyrwztzqs

Conversation

@insipx
Copy link
Contributor

@insipx insipx commented Feb 11, 2026

crane docs highly recommend integrating cargo-hakari to reduce cache misses. cargo hakari should also optimize local building and reduce re-builds by optimizing the dependency graph of the cargo workspace.

read about how hakari works here

workspace-hack is used on some other large projects as well, like

rustc
firefox

this PR follows the instructions in cargo-hakari docs and is completely auto-generated. it adds a CI workflow to verify the config. you can run hakari locally with nix develop .# --command cargo hakari verify or nix develop .# --command cargo hakari generate

@insipx insipx requested a review from a team as a code owner February 11, 2026 15:17
@insipx
Copy link
Contributor Author

insipx commented Feb 11, 2026

This PR is part of a stack of 4 bookmarks:

  1. trunk()
  2. push-nwxxwzqwnxoq
  3. push-wztqyrwztzqs ← this PR
  4. push-lssnrpnpzvll
  5. push-mxrnnusmvnut

Created with jj-stack

@claude
Copy link

claude bot commented Feb 11, 2026

Claude finished @insipx's task —— View job


PR Review - cargo-hakari Integration

Review Date: 2026-02-11 20:12 UTC


Summary

This PR successfully integrates cargo-hakari to optimize workspace dependency unification, which should improve Nix cache hits and reduce local rebuild times. The implementation follows best practices and is entirely auto-generated as intended.

Key Findings

Strengths:

  • Proper CI integration: The workflow correctly validates both cargo hakari generate --diff and cargo hakari manage-deps --dry-run (.github/workflows/lint-workspace.yml:24-31)
  • Correct WASM handling: Properly excludes mio/tokio and bindings_wasm to avoid feature conflicts (.config/hakari.toml:28-33)
  • Platform coverage: Targets appropriate platforms for development (x86_64-linux, aarch64-darwin, Android) (.config/hakari.toml:17-22)
  • Tooling setup: cargo-hakari added to Nix dev environments (nix/libxmtp.nix:46, 97)
  • Format exclusion: Correctly excludes the hack crate from taplo formatting (.taplo.toml:4)

Potential Concerns

⚠️ Minor Issues:

  1. deny.toml cleanup may be too aggressive (deny.toml:1-39)

    • Removed 14 lines including comments about duplicate dependencies
    • While the workspace-hack should resolve many duplicates, consider preserving documentation about why certain duplicates might exist post-hakari
    • The previous [[bans.skip]] and [[bans.skip-tree]] sections were removed - verify these are truly unnecessary now
  2. Missing workspace.resolver check (.config/hakari.toml:15)

    • Config sets resolver = "3" but doesn't verify root Cargo.toml matches
    • Root Cargo.toml correctly has resolver = "3" (Cargo.toml:25), but hakari docs recommend verification
    • Not a bug, but worth noting

Security & Best Practices

No security concerns identified:

  • Generated code is minimal (stub lib.rs and build.rs)
  • No credentials or secrets
  • Uses well-established tool (hakari) used by rustc and firefox

Follows project conventions:

  • Matches existing Cargo.toml patterns
  • Integrates with existing Nix infrastructure
  • Uses workspace metadata appropriately

Performance Considerations

Expected benefits:

  • Unified feature resolution across workspace
  • Better Nix cache locality
  • Reduced local recompilation on dependency changes
  • crane docs cite this as high-impact optimization

Test Coverage

CI adequately validates:

  • Hack crate stays current with cargo hakari generate --diff
  • All crates depend on hack with cargo hakari manage-deps --dry-run
  • No additional test coverage needed for auto-generated code

Recommendations

  1. Optional: Consider adding a comment in deny.toml explaining that workspace-hack reduces duplicate dependencies
  2. Optional: Document the hakari workflow in CLAUDE.md for future contributors (e.g., when to run cargo hakari generate)
  3. Before merge: Verify CI passes with the new validation steps

Final Assessment

Approve with minor suggestions. This is a well-executed optimization following established patterns. The auto-generated nature and crane/hakari best practices make this low-risk. The only consideration is preserving institutional knowledge about dependency management that may have been in the removed deny.toml comments.


Todo List:

  • Review hakari configuration
  • Review CI integration
  • Review generated workspace-hack crate
  • Review workspace crate updates
  • Check for potential issues
  • Provide final feedback

@insipx insipx changed the title add cargo hakari workspace hack optimize nix build cache hits with cargo-hakari Feb 11, 2026
@insipx insipx changed the title optimize nix build cache hits with cargo-hakari optimize nix build cache hits & local compile time with cargo-hakari Feb 11, 2026
@macroscopeapp
Copy link
Contributor

macroscopeapp bot commented Feb 11, 2026

Add cargo-hakari workspace-hack crate and CI checks to improve Nix build cache hits and local compile time across Rust workspace

Introduce the generated xmtp-workspace-hack crate, add cargo-hakari config, wire CI lint to verify hack generation and dependency presence, and update Nix shells to include cargo-hakari and library paths.

📍Where to Start

Start with the cargo-hakari configuration in .config/hakari.toml, then review the CI integration in .github/workflows/lint-workspace.yml and the crate manifest in crates/xmtp-workspace-hack/Cargo.toml.


Macroscope summarized 24449cb.

macroscopeapp[bot]
macroscopeapp bot previously approved these changes Feb 11, 2026
Copy link
Contributor

@macroscopeapp macroscopeapp bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-evaluating approvability for a370386

@codecov
Copy link

codecov bot commented Feb 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.25%. Comparing base (8d5ce50) to head (24449cb).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3163      +/-   ##
==========================================
- Coverage   74.28%   74.25%   -0.03%     
==========================================
  Files         448      448              
  Lines       55750    55750              
==========================================
- Hits        41413    41398      -15     
- Misses      14337    14352      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@macroscopeapp macroscopeapp bot dismissed their stale review February 11, 2026 15:35

Dismissing my prior approval and re-evaluating approvability for 126ad37

@insipx insipx force-pushed the push-wztqyrwztzqs branch 2 times, most recently from 0dd6b7b to a370386 Compare February 11, 2026 15:41
macroscopeapp[bot]
macroscopeapp bot previously approved these changes Feb 11, 2026
Copy link
Contributor

@macroscopeapp macroscopeapp bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-evaluating approvability for 796f015

@macroscopeapp macroscopeapp bot dismissed their stale review February 11, 2026 16:02

Dismissing my prior approval and re-evaluating approvability for 19bb54f

macroscopeapp[bot]
macroscopeapp bot previously approved these changes Feb 11, 2026
Copy link
Contributor

@macroscopeapp macroscopeapp bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-evaluating approvability for b9154cd

@macroscopeapp macroscopeapp bot dismissed their stale review February 11, 2026 18:09

Dismissing my prior approval and re-evaluating approvability for b9154cd

@insipx insipx force-pushed the push-nwxxwzqwnxoq branch 2 times, most recently from c934c2f to f678186 Compare February 11, 2026 18:14
macroscopeapp[bot]
macroscopeapp bot previously approved these changes Feb 11, 2026
Copy link
Contributor

@macroscopeapp macroscopeapp bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-evaluating approvability for d01a6e0

@macroscopeapp macroscopeapp bot dismissed their stale review February 11, 2026 18:25

Dismissing my prior approval and re-evaluating approvability for 2d8a460

macroscopeapp[bot]
macroscopeapp bot previously approved these changes Feb 11, 2026
Copy link
Contributor

@macroscopeapp macroscopeapp bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-evaluating approvability for ae2f637

Base automatically changed from push-nwxxwzqwnxoq to main February 11, 2026 19:09
@macroscopeapp macroscopeapp bot dismissed their stale review February 11, 2026 19:09

Dismissing my prior approval and re-evaluating approvability for 3debeee

@insipx insipx force-pushed the push-wztqyrwztzqs branch 2 times, most recently from bab2023 to ae2f637 Compare February 11, 2026 19:15
macroscopeapp[bot]
macroscopeapp bot previously approved these changes Feb 11, 2026
Copy link
Contributor

@macroscopeapp macroscopeapp bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-evaluating approvability for 9926a9a

@macroscopeapp macroscopeapp bot dismissed their stale review February 11, 2026 19:22

Dismissing my prior approval and re-evaluating approvability for bab2023

macroscopeapp[bot]
macroscopeapp bot previously approved these changes Feb 11, 2026
Copy link
Contributor

@macroscopeapp macroscopeapp bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-evaluating approvability for 24449cb

@macroscopeapp macroscopeapp bot dismissed their stale review February 11, 2026 19:50

Dismissing my prior approval and re-evaluating approvability for a07dd3c

@insipx insipx mentioned this pull request Feb 11, 2026
@insipx insipx enabled auto-merge (squash) February 11, 2026 20:10
ANDROID_NDK_ROOT = androidPaths.ndkHome;
NDK_HOME = androidPaths.ndkHome;
EMULATOR = "${androidEmulator}";
LD_LIBRARY_PATH = lib.makeLibraryPath [ openssl zlib ];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be helpful to know when this list of inputs would need to change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was getting build errors in android, I think some buildscripts need the LD_LIBRARY_PATH to be set in order to work correctly. I've found its mostly an issue with the zstd/openssl dependencies

@insipx insipx merged commit d6cbc1b into main Feb 11, 2026
66 of 70 checks passed
@insipx insipx deleted the push-wztqyrwztzqs branch February 11, 2026 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants