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

Allow to define concrete resolvers for dependencies #97

Merged
merged 1 commit into from May 13, 2017

Conversation

Projects
None yet
2 participants
@jvican
Copy link
Member

commented May 11, 2017

This depends on https://github.com/sbt/librarymanagement/pull/96/files.

Sometimes, for predictability and performance, we may be interested in
specifying the concrete resolver that a ModuleID should use.

This patch achieves this by adding a new field to UpdateOptions and
then getting this information from the SbtChainResolver, that will
select the concrete resolver for a given dependency descriptor.

Why is this useful? Well, two reasons:

  • Predictable behaviour. We have the guarantee that an artifact only
    comes from a concrete resolver.
  • Resolution speedup. Around 1/3 or 1/2 times faster than normal
    resolution in a moderate test case scenario. If there is a lot of
    latency or network connection is poor, speedups will be higher.

Some results of tests I have just ran:

NORMAL RESOLUTION TIME 1790
FASTER RESOLUTION TIME 1054
NORMAL RESOLUTION TIME 2078
FASTER RESOLUTION TIME 1055

Lots of projects can benefit from this option, as well as organizations
and companies. This will eventually integrate with the dependency lock
file, but can be used independently of it.

@eed3si9n eed3si9n added the ready label May 11, 2017

@jvican jvican force-pushed the scalacenter:unique-resolvers branch 2 times, most recently from c68f8ef to 3dcd9c3 May 11, 2017

@jvican

This comment has been minimized.

Copy link
Member Author

commented May 12, 2017

This has been rebased and is ready for review.

@jvican

This comment has been minimized.

Copy link
Member Author

commented May 12, 2017

Tests are green.

@@ -349,6 +349,8 @@ private[sbt] object IvySbt {
val mainChain = makeChain("Default", "sbt-chain", resolvers)
settings.setDefaultResolver(mainChain.getName)
}

// TODO: Expose the changing semantics to the caller so that users can specify a regex

This comment has been minimized.

Copy link
@jvican

jvican May 12, 2017

Author Member

I have followed up with: #101.

@eed3si9n
Copy link
Member

left a comment

Looks good overall.
Minor comments.

private[sbt] def copy(
circularDependencyLevel: CircularDependencyLevel = this.circularDependencyLevel,
interProjectFirst: Boolean = this.interProjectFirst,
latestSnapshots: Boolean = this.latestSnapshots,
consolidatedResolution: Boolean = this.consolidatedResolution,
cachedResolution: Boolean = this.cachedResolution,
resolverConverter: UpdateOptions.ResolverConverter = this.resolverConverter
resolverConverter: UpdateOptions.ResolverConverter = this.resolverConverter,
moduleResolvers: Map[ModuleID, Resolver] = this.moduleResolvers

This comment has been minimized.

Copy link
@eed3si9n

eed3si9n May 12, 2017

Member

moduleResolvers should be added to equals and hashCode.

This comment has been minimized.

Copy link
@jvican

jvican May 12, 2017

Author Member

This is a very good catch.

val resolverConverter: UpdateOptions.ResolverConverter
val resolverConverter: UpdateOptions.ResolverConverter,
// Map the unique resolver to be checked for the module ID
val moduleResolvers: Map[ModuleID, Resolver]

This comment has been minimized.

Copy link
@eed3si9n

eed3si9n May 12, 2017

Member

Should this be an immutable Map?

This comment has been minimized.

Copy link
@jvican

jvican May 12, 2017

Author Member

Yes, it should be immutable. We don't transform it internally and we don't want their contents to be exposed and modified from the outside. We just use it to represent some data that we use once inside sbt' chain resolver.

@jvican

This comment has been minimized.

Copy link
Member Author

commented May 12, 2017

Speedup is 3x in CI.

NORMAL RESOLUTION TIME 960

FASTER RESOLUTION TIME 361
Allow to define concrete resolvers for dependencies
Sometimes, for predictability and performance, we may be interested in
specifying the concrete resolver that a `ModuleID` should use.

This patch achieves this by adding a new field to `UpdateOptions` and
then getting this information from the `SbtChainResolver`, that will
select the concrete resolver for a given dependency descriptor.

Why is this useful? Well, two reasons:

* Predictable behaviour. We have the guarantee that an artifact only
  comes from a concrete resolver.
* Resolution speedup. Around 1/3 or 1/2 times faster than normal
  resolution in a moderate test case scenario. If there is a lot of
  latency or network connection is poor, speedups will be higher.

LOGS:

```
NORMAL RESOLUTION TIME 1790
FASTER RESOLUTION TIME 1054
```

```
NORMAL RESOLUTION TIME 2078
FASTER RESOLUTION TIME 1055
```

Lots of projects can benefit from this option, as well as organizations
and companies. This will eventually integrate with the dependency lock
file, but can be used independently of it.

@jvican jvican force-pushed the scalacenter:unique-resolvers branch from 3dcd9c3 to 8bb1676 May 12, 2017

@jvican

This comment has been minimized.

Copy link
Member Author

commented May 12, 2017

I just fixed the remaining bit. PR is green.

@eed3si9n eed3si9n merged commit 9e799a8 into sbt:1.0 May 13, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@eed3si9n eed3si9n removed the ready label May 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.