Skip to content

Rewrite the Versioned Dependency Resolver for determinism, simplicity, and performance#1014

Merged
ras0219-msft merged 15 commits intomicrosoft:mainfrom
ras0219-msft:dev/roschuma/depres
Apr 26, 2023
Merged

Rewrite the Versioned Dependency Resolver for determinism, simplicity, and performance#1014
ras0219-msft merged 15 commits intomicrosoft:mainfrom
ras0219-msft:dev/roschuma/depres

Conversation

@ras0219-msft
Copy link
Collaborator

@ras0219-msft ras0219-msft commented Apr 9, 2023

Primary Problem

Today, dependency resolution depends on evaluation order of dependencies due to "dynamic occlusion":

A -> B -> D>=1
A -> C -> D>=2
A -> E
D@1 -> E>=2
D@2 -> E>=1
All packages have baseline @0.

Eval Order: A@0, B@0, D@1, E@2, C@0, D@2
Result: A=0, B=0, D=2, E=2, C=0

Eval Order: A@0, C@0, D@2, E@1, B@0
Result: A=0, B=0, D=2, E=1, C=0

This fundamentally occurs due to "occluding" versions earlier than the current "best" version along a schema. For the example above, if D>=2 has been considered, we don't consider D@1 at all.

Solution

The decision to occlude a port must be known based on "static" information. There are two obvious sets of static, up-front information: the baseline and the top-level manifest's core dependencies. While the core dependencies of the top-level manifest are static, treating them differently than feature dependencies would result in a much more confusing user experience.

Therefore, we use the baseline values as the sole occlusion. All versions referenced that compare >= the baseline are considered. Any versions less than the baseline or incomparable[1] are not considered. For every required feature, we consider the dependencies of that feature for all considered versions.

  • Both phases (resolve constraints, serialize plan) are now iterative instead of recursive
  • Substantially enhanced batching of dep_info calculations -- for a simple vcpkg.json with boost as a dependency this results in 24 fewer calls to CMake, each of which costs ~50ms on Windows (1.2 seconds of wall-clock-time savings!)
  • Split algorithm into a separate file from dependencies.cpp since it's unrelated to the classic-mode algorithm.
  • Substantially reduced the source lines by reducing duplicate logic. For example, deciding between overlays, overrides, or baselines is now handled in a single place.
  • Default dependencies are now handled even more clearly. The rules for defaults are now simply:
    • All dependencies not referenced in the top-level "dependencies" list have defaults applied.
    • Any dependency object without "default-features": false applies defaults.

Drive-by Improvements

  • Fix VS 2022 Intellisense understanding of ExpectedT::then
  • Fix Util::filter()
  • Added more error message testing
  • Added more tests for created InstallPlanActions
  • Improved version conflict message
    Before:
    error: version conflict on a:x86-windows: baseline required 1.0.2 but vcpkg could not compare it to 1.0.1.
    The two versions used incomparable schemes:
        "1.0.1" was of scheme string
        "1.0.2" was of scheme semver
    This can be resolved by adding an explicit override to the preferred version, for example:
        "overrides": [
            { "name": "a", "version": "1.0.1" }
        ]
    See `vcpkg help versioning` for more information.
    
    After:
    error: version conflict on a:x86-windows: toplevel-spec required 1.0.1, which cannot be compared with the baseline version 1.0.2.
    
    The versions have incomparable schemes:
        a@1.0.2 has scheme semver
        a@1.0.1 has scheme string
    
    This can be resolved by adding an explicit override to the preferred version. For example:
    
        "overrides": [
            { "name": "a", "version": "1.0.2" }
        ]
    
    See `vcpkg help versioning` or https://learn.microsoft.com/vcpkg/users/versioning for more information.
    
    Before:
    error: version conflict on a:x86-windows: baseline required 1.0.2 but vcpkg could not compare it to 1.0.1.
    The two versions used incomparable schemes:
        "1.0.1" was of scheme string
        "1.0.2" was of scheme string
    This can be resolved by adding an explicit override to the preferred version, for example:
        "overrides": [
            { "name": "a", "version": "1.0.1" }
        ]
    See `vcpkg help versioning` for more information.
    
    After:
    error: version conflict on a:x86-windows: toplevel-spec required 1.0.1, which cannot be compared with the baseline version 1.0.2.
    
    Both versions have scheme string but different primary text.
    
    This can be resolved by adding an explicit override to the preferred version. For example:
    
        "overrides": [
            { "name": "a", "version": "1.0.2" }
        ]
    
    See `vcpkg help versioning` or https://learn.microsoft.com/vcpkg/users/versioning for more information.
    
  • Improved missing feature message
    Before:
    error: b:x64-windows@1 does not have required feature y
    
    After:
    error: b@1 does not have required feature y needed by a:x86-windows@1
    
  • Improved cycle detected message
    Before:
    error: cycle detected during a:x86-windows:
    a:x86-windows
    b:x86-windows
    
    After:
    error: cycle detected during a:x86-windows:
    a:x86-windows@1
    b:x86-windows@1
    

(Resolves internal bug https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1791323)

[1]: We can exclude incomparable versions because any valid plan action MUST satisfy the baseline -- and so must be comparable.

@ras0219-msft ras0219-msft marked this pull request as ready for review April 12, 2023 03:27
@ras0219-msft ras0219-msft requested a review from vicroms April 14, 2023 17:46
Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

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

Still not a fan of CHECK_LINES/require losing the context in terms of where the failure happens

Copy link
Member

@vicroms vicroms left a comment

Choose a reason for hiding this comment

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

LGTM

@ras0219-msft ras0219-msft merged commit 6035212 into microsoft:main Apr 26, 2023
@ras0219-msft ras0219-msft deleted the dev/roschuma/depres branch April 26, 2023 03:06
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.

4 participants