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

implement RemoteCache #5534

Merged
merged 2 commits into from Jun 10, 2020
Merged

implement RemoteCache #5534

merged 2 commits into from Jun 10, 2020

Conversation

eed3si9n
Copy link
Member

@eed3si9n eed3si9n commented May 6, 2020

See http://eed3si9n.com/cached-compilation-for-sbt for details.
Ref sbt/zinc#712

This adds pushRemoteCache, pushRemoteCacheTo, pullRemoteCache, etc to implement cached compilation facility.

In addition, the analysis file location is now made more clear.

@danicheg
Copy link

danicheg commented May 7, 2020

Do I understand correctly that the generated code located in 'src_managed' will not be cached?

final val cachedTestClasifier = "cached-test"

def gitCommitId: String =
scala.sys.process.Process("git rev-parse --short HEAD").!!.trim
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this line introduce a dependency from the git command line tool.
You can possibly do the same using jgit:

git.getRepository().exactRef("refs/heads/master").getObjectId().abbreviate(7).name()

from:
https://stackoverflow.com/questions/39356463/is-there-a-jgit-method-equivalent-to-git-rev-parse-short

It might be an additional dependency for Sbt itself, but I think it would be more "self-contained", what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm of double mind here. On one hand you're right that it would be good to be self-contained. But on the other, every layer of J-anything could lag behind and introduce problem on its own. See for example pgp vs BouncyCastle. If ppl want JGit support they could use sbt-git and write

ThisBuild / git.gitRunner := com.typesafe.sbt.git.JGitRunner
ThisBuild / remoteCacheId := git.gitHeadCommit.value

or something to the effect?

Copy link
Member Author

Choose a reason for hiding this comment

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

And I don't think it's a good idea to use git as a file cache.

The git commit id is the one for the source code, file cache will be a regular Maven repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand your concerns, still, for exactly one call I would prefer to opt for the self-contained solution.
PGP is a core functionality and it make sense to use the standard implementation.
Here we are using it as a tool for getting the job done.

Copy link
Member Author

Choose a reason for hiding this comment

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

Every dependency we bring into sbt is a long-term liability. here's the transitive dependencies for the current release:

$ cs resolve org.eclipse.jgit:org.eclipse.jgit:5.7.0.202003110725-r
com.googlecode.javaewah:JavaEWAH:1.1.7:default
com.jcraft:jsch:0.1.55:default
com.jcraft:jzlib:1.1.1:default
org.bouncycastle:bcpg-jdk15on:1.64:default
org.bouncycastle:bcpkix-jdk15on:1.64:default
org.bouncycastle:bcprov-jdk15on:1.64:default
org.eclipse.jgit:org.eclipse.jgit:5.7.0.202003110725-r:default
org.slf4j:slf4j-api:1.7.2:default

Note that (not really maintained) sbt-git uses a much older version of JGit. Keeping this in the plugin allows the dependencies to be updated without worrying about bincompat impact etc. Eventually when enough ppl start to use this and realize we need JGit, we can always add more later.

@He-Pin
Copy link

He-Pin commented May 7, 2020

There is one thing like this com-lihaoyi/mill#726

@eed3si9n
Copy link
Member Author

eed3si9n commented May 7, 2020

@danicheg

Do I understand correctly that the generated code located in 'src_managed' will not be cached?

It has not been cached. I am not sure if it's possible to cache that since the input for the code generation logic would be missing and it will be invalidated anyway. Also if the content hashing worked, it shouldn't matter as long as bit-by-bit identical code gets generated?

pushRemoteCache.in(Defaults.TaskZero) := (Def.task {
val s = streams.value
val config = pushRemoteCacheConfiguration.value
IvyActions.publish(ivyModule.value, config, s.log)
Copy link

Choose a reason for hiding this comment

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

can this override with any other filestore?

Copy link

@He-Pin He-Pin May 7, 2020

Choose a reason for hiding this comment

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

like maybe we can make use of something like https://www.seafile.com/home/ to keep things sync automatically.

Copy link
Member Author

Choose a reason for hiding this comment

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

can this override with any other filestore?

Yes. In the blog post I used:

ThisBuild / pushRemoteCacheTo := Some(MavenCache("local-cache", file("/tmp/remote-cache")))

@danicheg
Copy link

danicheg commented May 8, 2020

@eed3si9n

Also if the content hashing worked, it shouldn't matter as long as bit-by-bit identical code gets generated?

I wonder about the case of code generation from the static scheme that changes quite rarely (case of generation codecs for protocol buffers). The plugin tracks an earlier generated code (or, more accurately, it tracks the timestamp of changes the scheme) and doesn't generate/recompile code for every time. For now, we just copy the generation stuff from the previous job run at CI, but it looks like an ad-hoc workaround. Is it an edge case for the caching 'src_managed' stuff?

This adds `pushRemoteCache`, `pushRemoteCacheTo`, `pullRemoteCache`, etc to implement cached compilation facility.

In addition, the analysis file location is now made more clear.
In case there are a few local commits ahead of the remote cache, this would still grab the close point in the history to resume the build.
@eed3si9n eed3si9n merged commit 62c6cad into sbt:develop Jun 10, 2020
@eed3si9n eed3si9n deleted the wip/remote branch June 10, 2020 18:54
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.

None yet

4 participants