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

Add offline mode to `UpdateConfiguration` #92

Merged
merged 4 commits into from May 10, 2017

Conversation

Projects
None yet
3 participants
@jvican
Copy link
Member

commented May 5, 2017

The following commit tries to address the well-known issue that sbt
cannot be used in offline mode. In order to enable that use case, this
commit adds support for a flag in update configuration called offline
that users can change as they wish (and that sbt will expose via
settings).

It adds tests to check that the resolution uses the caches instead of
trying to resolve from the Internet. Unfortunately, ivy does not expose
a way to know whether a resolution was made from the cache or the
Internet, so the test case invents a metric to check that resolution
indeed happens from cache.

In order to benefit from this 100%, we need to update to ivy 2.4.0 or
cherry-pick a commit because a major issue in useCacheOnly has been
fixed: https://issues.apache.org/jira/browse/IVY-1515.

In short, this is good for the dependency lock file too. Since we can
make sure that once we have downloaded and resolved all the dependencies
locally, we do resolve from the cache.

@eed3si9n eed3si9n added the ready label May 5, 2017

@jvican

This comment has been minimized.

Copy link
Member Author

commented May 5, 2017

This fixes: sbt/sbt#2331 and sbt/sbt#290.
Partially addresses https://github.com/sbt/sbt/wiki/User-Stories:--Offline-mode-and-Dependency-Locking, awaiting changes to upstream sbt.

@jvican

This comment has been minimized.

Copy link
Member Author

commented May 5, 2017

Review by @eed3si9n.

P.S. The difference with the current offline is that the new implementation will not try to download from the Internet if the artifacts cannot be resolved in the cache. So we can actually remove the previous offline implementation.

jvican added a commit to jvican/ivy that referenced this pull request May 5, 2017

Apply IVY-1515 patch
Cherry picks https://issues.apache.org/jira/browse/IVY-1515 to unblock
sbt/librarymanagement#92.

This change is required for sbt offline to work in all the cases.

@jvican jvican referenced this pull request May 5, 2017

Merged

Apply IVY-1515 upstream patch #25

@jvican jvican force-pushed the scalacenter:offline-mode branch from a6fa33e to f28dc44 May 5, 2017

@jvican jvican force-pushed the scalacenter:offline-mode branch 16 times, most recently from cfc0bae to 2a74fd8 May 5, 2017

@jvican

This comment has been minimized.

Copy link
Member Author

commented May 8, 2017

Tests are now passing. This can be reviewed + merged. Ivy version updated too.

@dwijnand

This comment has been minimized.

Copy link
Member

commented May 9, 2017

[info] OfflineModeSpec:
[info] Offline update configuration
[info] - should reuse the caches when offline is enabled *** FAILED ***
[info]   offlineResolution.isRight was false (OfflineModeSpec.scala:39)
[info] - should reuse the caches when offline and cached resolution are enabled
[info] - should fail when artifacts are missing in the cache
[info] - should fail when artifacts are missing in the cache for cached resolution *** FAILED ***
[info]   failedOfflineResolution.isLeft was false (OfflineModeSpec.scala:61)
@jvican

This comment has been minimized.

Copy link
Member Author

commented May 9, 2017

This happened in a change I submitted after the comment. Thanks for the heads up @dwijnand.

@jvican jvican force-pushed the scalacenter:offline-mode branch 2 times, most recently from 88847c0 to 607a3dd May 9, 2017

jvican added some commits May 5, 2017

Add offline mode to `UpdateConfiguration`
The following commit tries to address the well-known issue that sbt
cannot be used in offline mode. In order to enable that use case, this
commit adds support for a flag in update configuration called `offline`
that users can change as they wish (and that sbt will expose via
settings).

It adds tests to check that the resolution uses the caches instead of
trying to resolve from the Internet. Unfortunately, ivy does not expose
a way to know whether a resolution was made from the cache or the
Internet, so the test case invents a metric to check that resolution
indeed happens from cache.

In order to benefit from this 100%, we need to update to ivy 2.4.0 or
cherry-pick a commit because a major issue in `useCacheOnly` has been
fixed: https://issues.apache.org/jira/browse/IVY-1515.

In short, this is good for the dependency lock file too. Since we can
make sure that once we have downloaded and resolved all the dependencies
locally, we do resolve from the cache.
Remove previous custom offline implementation
The previous custom offline implementation was not working on 100% of
the cases and relied on the TTL of ivy. As the previous commit
enabled the native offline implementation provided by ivy as of 2.3.0,
this functionality is not useful anymore.

The current place to specify offline is `UpdateConfiguration`, and not
`InlineIvyConfiguration` that is required to instantiate sbt. With the
current approach, we can be online or offline without having to
instantiate ivy sbt twice.

I will provide a Scalafix rewrite for this change.

jvican added some commits May 8, 2017

@jvican jvican force-pushed the scalacenter:offline-mode branch from c975710 to 89b722c May 9, 2017

@jvican

This comment has been minimized.

Copy link
Member Author

commented May 9, 2017

Tests were running in parallel... This option has been disabled, since this is not safe with ivy. Everything is green now and I'm done with the changes in this PR.

@jvican

This comment has been minimized.

Copy link
Member Author

commented May 10, 2017

ping @eed3si9n to merge / review

@eed3si9n eed3si9n merged commit 498367d into sbt:1.0 May 10, 2017

1 check passed

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

@eed3si9n eed3si9n removed the ready label May 10, 2017

@eed3si9n

This comment has been minimized.

Copy link
Member

commented May 10, 2017

Merged! Thanks for the contribution.

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.