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

[MRESOLVER-512] ScopeManager #445

Merged
merged 9 commits into from Mar 27, 2024
Merged

[MRESOLVER-512] ScopeManager #445

merged 9 commits into from Mar 27, 2024

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Mar 15, 2024

In Resolver 1.x times, resolver was unaware of "resolution scope", to get the wanted resolution scope caller had to tweak and set up various nits and bits, like selectors, filters, and so on. It was easy to miss. Similarly, resolver had no "first class" type for "dependency scope" either, they were just string labels (that everyone knew HOW should behave, but was never codified) and its meaning and notion was sprinkled across several classes. Introducing new scope in these conditions (or alter selector to something that would have new scopes, like Maven4 plans) was nearly impossible.

The ScopeManager aims to solve these issues: it defines "resolution scope" and "dependency scope", interprets them, and allows caller to simply make a call to "resolve me main-runtime" resolution scope. No hoops and loops. Moreover, it is FASTER than Resolver 1.x was, that used always same selector (to build dirty graph), so potentially huge graph even if you needed just a bit of it, that was later chopped down to clean up the graph. ScopeManager automates selector selection/setup, and goes directly for result, in most cases the resulting tree is done in first pass.

Finally, and most importantly, ScopeManager allows to be "configured": by supplying the recipe for dependency and resolution scopes, hence, makes Resolver 2.x versatile, in a sense, it is not "this or that" anymore, it can obey Maven3 and Maven4 dependency scopes at the same time.


https://issues.apache.org/jira/browse/MRESOLVER-512

@cstamas cstamas self-assigned this Mar 15, 2024
@gnodet
Copy link
Contributor

gnodet commented Mar 15, 2024

I'm really reluctant about this PR. This goes in the exact opposite direction than https://issues.apache.org/jira/browse/MRESOLVER-471, which aimed to make the resolver agnostic about scopes and leave these bits to be defined by Maven.

It basically replaces 250 lines of deprecated code (the JavaDependencyContextRefiner, JavaScopeDeriver and JavaScopeSelector which were deprecated a few weeks ago) with 3800 new lines of code, adds a strong tie between the resolver and maven, whereas the code could be easily moved into maven which already exposes those concepts.

@cstamas
Copy link
Member Author

cstamas commented Mar 15, 2024

Well, IMO this PR goes exactly in direction of MRESOLVER-471, as:

  • ScopeManager is optional, in a sense, if you want to use Resolver 2.x as you did Resolver 1.x, go for it
  • ScopeManager solves the problem of "Resolver should be oblivious about scopes" (MRESOLVER-471), as Resolver 1.x implemented scope related logic scattered across several (dozen?) classes that cemented their semantics, moreover modifications like "introducing new scope" was nearly impossible exactly due that (as Resolver was not oblivious, but had them cemented as String constants spread across dozen classes). Hence Resolver was strictly "this xor that" (exclusive or).

With ScopeManager Resolver IS really oblivious about scopes, in a sense, they are not only "string labels with some coded meaning scattered across codebase", but they are defined by client project, the app that is integrating Resolver 2.

@gnodet
Copy link
Contributor

gnodet commented Mar 20, 2024

@cstamas The whole SystemScopeHandler needs has to be removed/merged into ScopeManager imho. The isSystemScope really looks redundant with ScopeManager.getSystemScope().

Now, Resolver 2.x operates based on presence of ScopeManager:
* if not set (null), "legacy"/maven3 mode
* if set, is fully utilized

Hence, "system" scope may be:
* LEGACY when no scope manager set
* non-null or null value (no system scope) if scope manager set
@cstamas
Copy link
Member Author

cstamas commented Mar 23, 2024

@gnodet applied changes, IMHO it looks even better now 😄

@cstamas cstamas marked this pull request as ready for review March 27, 2024 10:53
@cstamas cstamas merged commit 39452f7 into apache:master Mar 27, 2024
4 checks passed
@cstamas cstamas deleted the MRESOLVER-512 branch March 27, 2024 10:53
cstamas added a commit to apache/maven that referenced this pull request Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants