Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maven support disk cache fixes #9824

Merged
merged 8 commits into from
Jan 24, 2025
Merged

Conversation

sschuberth
Copy link
Member

Please have a look at the individual commit messages for the details.

It is not completely true that no version-specific features of the CLI
tools are used, e.g. the `info` sub-command needs to support the `--json`
option. As the comments were not using the required passive voice anyway,
simply remove them.

Signed-off-by: Sebastian Schuberth <[email protected]>
This just repeats the obvious implementation of
`getVersionRequirement()`. As the comment was not using the required
passive voice anyway, simply remove it.

Signed-off-by: Sebastian Schuberth <[email protected]>
Originally, these functions did not really add much value over calling
the code at the begin / end of `resolveDependencies()` for the
respective manager. Add some value to these functions by changing when
they are called:

Call `beforeResolution()` before `resolveDependencies()` is called for
any enabled manager, and `afterResolution()` after resolution has
finished for all enabled managers. This allows to use these functions for
cases where clean-ups, like the deletion of a `node_modules` directory,
have to happen after another manager (like Gradle) was run, to be able to
analyze complex React-Native applications with inter-dependencies between
managers.

Note that this requires changes to code paths that do not make use of
`analyzeInParallel()`, like `resolveSingleProject()` used in tests.

Signed-off-by: Sebastian Schuberth <[email protected]>
The `remoteArtifactCache` has never been properly closed before (until
process termination). Fix that to allow the same process (e.g. in a
server context) to use the cache multiple times in a row.

This requires each `MavenSupport` instance to have its own instance of
the disk cache.

Fixes #7978.

Signed-off-by: Sebastian Schuberth <[email protected]>
As `DiskLruCache` does not support concurrent edits by default [1] and
making it so is hard [2], use isolated disk cache directories for Gradle
and Maven to allow them to safely run in parallel.

[1]: https://github.com/JakeWharton/DiskLruCache?tab=readme-ov-file#disk-lru-cache
[2]: JakeWharton/DiskLruCache#19

Signed-off-by: Sebastian Schuberth <[email protected]>
Copy link

codecov bot commented Jan 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.08%. Comparing base (7c63104) to head (22573f9).
Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #9824   +/-   ##
=========================================
  Coverage     68.07%   68.08%           
- Complexity     1285     1286    +1     
=========================================
  Files           249      249           
  Lines          8828     8829    +1     
  Branches        918      918           
=========================================
+ Hits           6010     6011    +1     
  Misses         2432     2432           
  Partials        386      386           
Flag Coverage Δ
funTest-docker 65.01% <100.00%> (+0.01%) ⬆️
funTest-non-docker 33.35% <ø> (ø)
test-ubuntu-24.04 35.85% <0.00%> (-0.05%) ⬇️
test-windows-2022 35.83% <0.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@sschuberth sschuberth merged commit e77fadf into main Jan 24, 2025
26 checks passed
@sschuberth sschuberth deleted the maven-support-disk-cache-fixes branch January 24, 2025 09:57
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