Skip to content

fix: cache clear now deletes dep files from disk, not just JSON#2449

Open
Bhumika-SN wants to merge 2 commits into
jbangdev:mainfrom
Bhumika-SN:fix/deps-cache-clear-not-deleting-files
Open

fix: cache clear now deletes dep files from disk, not just JSON#2449
Bhumika-SN wants to merge 2 commits into
jbangdev:mainfrom
Bhumika-SN:fix/deps-cache-clear-not-deleting-files

Conversation

@Bhumika-SN
Copy link
Copy Markdown
Contributor

Problem

jbang cache clear was not actually clearing the dependency cache from disk.

The DependencyCache.clear() method only set the in-memory depCache to null,
but never deleted the actual cached files on disk. This means running
jbang cache clear would leave all downloaded JARs and the dependency_cache.json
file still present on disk.

Fix

Updated DependencyCache.clear() to also:

  • Delete the dependency_cache.json file from disk
  • Delete the depcache directory containing actual downloaded JARs

Related Issue

Fixes #475

@maxandersen
Copy link
Copy Markdown
Collaborator

this looks right - but would be nice with an actual test to verify the behavour

Copy link
Copy Markdown
Contributor

@quintesse quintesse left a comment

Choose a reason for hiding this comment

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

Nope! This is not the place to do this!
The clear() method is only used to clean the in-memory representation of the dependencies and is only used by tests.

This should be moved to Cache.clearCache()

@maxandersen
Copy link
Copy Markdown
Collaborator

Hmm. Not sure what that comment was partially coming from :)

Thanks for checking @quintesse !

@Bhumika-SN
Copy link
Copy Markdown
Contributor Author

Hi @quintesse and @maxandersen,

I've moved the fix to Cache.clearCache() where it belongs, and
reverted DependencyCache.clear() back to only clearing the
in-memory cache.

Could you please re-review? Thank you!

@Bhumika-SN
Copy link
Copy Markdown
Contributor Author

Hi @maxandersen and @quintesse,

I've moved the fix to Cache.clearCache() as suggested and reverted
DependencyCache.clear() back to only clearing in-memory cache.

The CI failures appear to be in Java 25 integration tests and smoke
tests which seem pre-existing and unrelated to this change. Could
you confirm?

Thank you!

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.

jbang cache clear does not nuke dependency cache

3 participants