Skip to content

[2.x] fix: Invalidate update cache across commands when dependencies change#8501

Merged
eed3si9n merged 3 commits intosbt:developfrom
calm329:fix/8357-transitive-update-invalidation
Jan 12, 2026
Merged

[2.x] fix: Invalidate update cache across commands when dependencies change#8501
eed3si9n merged 3 commits intosbt:developfrom
calm329:fix/8357-transitive-update-invalidation

Conversation

@calm329
Copy link
Copy Markdown
Contributor

@calm329 calm329 commented Jan 12, 2026

Summary

Fix update cache invalidation across sbt command invocations when a dependency's dependencies change.

Problem

When project A's dependencies changed and A was compiled in one command, project B (depending on A) would not invalidate its update cache in a subsequent command. This caused stale classpaths.

The root cause was that depsUpdated only checked !stats.cached, which only detected fresh resolves within the same command. When a dependency was served from cache (even if resolved fresh in a previous command), cached was marked true, causing incorrect cache reuse.

Debug scenario from issue:
sbt:aaa> clean
sbt:aaa> a/compile
sbt:aaa> show itTests/depsUpdated
[info] * false <-- BUG: should be true

Solution

Added stamp: String field to UpdateStats that records when the update was resolved. This stamp persists across commands and enables accurate cross-command comparison:

  • If any dependency's stamp > our cached stamp, we re-resolve
  • Falls back to !cached check for backwards compatibility with old caches
  • Using String type allows future transition from timestamps to content hashes

Changes

  • Add stamp: String field to UpdateStats schema (default "" for compat)
  • Set stamp = System.currentTimeMillis().toString when creating UpdateStats
  • Modify cachedUpdate to compare stamps instead of just cached flag
  • Add backwards-compatible JSON deserialization for old cache files
  • Add scripted test i8357-transitive-update-invalidation

Test plan

  • Compiles successfully
  • lmCore/test passes
  • Run scripted test: scripted dependency-management/i8357-transitive-update-invalidation

Fixes #8357


Contribution by Gittensor, see my contribution statistics at https://gittensor.io/miners/details?githubId=148254234

Copy link
Copy Markdown
Member

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

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

Added some comment on the implementation, but overall this is a cool change!

@calm329
Copy link
Copy Markdown
Contributor Author

calm329 commented Jan 12, 2026

@eed3si9n Could you please review changes again?

Copy link
Copy Markdown
Member

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

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

Thanks!

@eed3si9n eed3si9n merged commit b4f73c9 into sbt:develop Jan 12, 2026
14 checks passed
> a/update

# Step 5: Now run itTests/update - should re-resolve with new dependencies
> show itTests/ourStamp
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i'd suggest adding an actual test, eg

dothetest := assert((Test / managedClasspath).value.toString.contains("cats-core_2.12-2.9.0")),

azdrojowa123 added a commit to azdrojowa123/sbt that referenced this pull request Jan 26, 2026
sbt#8600

- `depsUpdated` was removed in PR sbt#8501, but it seems it's still necessary (e.g., for files inside the global plugins dir) in addition to `UpdateStats#stamp` as inputs for the cache
azdrojowa123 added a commit to azdrojowa123/sbt that referenced this pull request Jan 26, 2026
sbt#8600

- `depsUpdated` was removed in PR sbt#8501, but it seems it's still necessary (e.g., for files inside the global plugins dir) in addition to `UpdateStats#stamp` as inputs for the cache
azdrojowa123 added a commit to azdrojowa123/sbt that referenced this pull request Jan 29, 2026
sbt#8600

- `depsUpdated` was removed in PR sbt#8501, but it seems it's still necessary (e.g., for files inside the global plugins dir) in addition to `UpdateStats#stamp` as inputs for the cache
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

itTest/update does not get invalidated, if a/compile is issued on its own

4 participants