Recent change in semantics of version conflict resolution

[ghostbuster91]

Hi,

When updating to a newer version of smithy-language-server that uses maven-resolver(aether) instead of coursier for dependency resolution (introduced in #113) we noticed a difference in behavior that was not expected.

I am skipping the details of that difference for brevity because it was quite complex. The end result was that the language server was seeing some old version of a dependency in the project's classpath - it was smithy-model version 1.27 instead of version 1.36 (artifacts/versions aren't important, I just use them to make it more concrete)

After closer inspection of the classpath we noticed that smithy-model is being included twice transitively by two different artifacts in two different versions - 1.27.0 and 1.36.0.

Previously, coursier in such cases was always picking the latest version. However, the behavior of maven-resolver is different. If you take a look into MavenRepositorySystemUtils the class that smithy-cli uses to instantiate DefaultRepositorySystemSession you will see that it defines ConflictResolver as follows:

DependencyGraphTransformer transformer = new ConflictResolver(
                new NearestVersionSelector(), new JavaScopeSelector(),
                new SimpleOptionalitySelector(), new JavaScopeDeriver());

where "nearest" means:

Nearest is defined as the declaration that has the least transitive steps away from the project being built.

In our cases the newer version was indeed one step deeper than the older one, but no one was paying attention to how many nesting steps of transitive dependencies there are.

This seems indeed to be the way of how maven works by default as at the time of writing there is no other implementation of VersionSelector besides NearestVersionSelector.

The solution in our case was simply - just add the smithy-model with the correct version as an explicit dependency in smithy-build.json.

Was this change intentional? Would you be interested in providing an option to restore old behavior, so that users have a smoother migration path?

[srchase]

The change to use maven-resolver in place of coursier was made to keep the Language Server and the core Smithy packages in sync in how they do dependency resolution.

Do you have a repro for this issue? We wouldn't want to re-introduce the dependency on coursier at this point, but it sounds like the Language Server pulling in two versions of a dependency is not correct.

[ghostbuster91]

but it sounds like the Language Server pulling in two versions of a dependency is not correct.

Let me be clear about this. In the end on the classpath there is only one version of each library (smithy-model in our case). The surprising part is how this particular version is chosen in the presence of version conflict due to transitive dependencies.

Do you have a repro for this issue?

I don't have it atm. I can prepare one, but I am very low on time recently so it might take a while.

In general it should be something like this:

1. Have four libraries A, B, C, D.
2. Have A depend on B in version 0.1, D depend on B in version 0.2, and C depend on D.
3. Add A and C into the language server dependencies via buildConfig file.

{
  "version": "1.0",
  "maven": {
    "dependencies": [
      "A",
      "C",
    ]
  },
  "imports": [
    "./smithy"
  ],
  "languageServer": "com.disneystreaming.smithy:smithy-language-server:0.0.28"
}

The dependency graph will look as follows:

-A
 \-B(0.1)
-C
 \-D
   \-B(0.2)

maven-resolver will chose B(0.1) because it is nearer according to the definition, while coursier would chose B(0.2) because it is newer.