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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes remote caching not managing resource files #6554

Merged
merged 2 commits into from Jun 19, 2021

Conversation

Nirvikalpa108
Copy link

What is the problem? 馃様

When using remote caching, the resource files are not tracked so if they have changed, pullRemoteCache will deliver both the old resource file as well as the most recent version.

This is a problem, because it's not the behaviour that our users will expect and it's not in keeping with the contract of this feature.

Why is this happening?

Zinc, sbt's incremental compiler, keeps track of changes that have been made. It keeps this in what is called the Analysis file. However, resource files are not tracked in the Analysis file, so remote caching is not invalidating the unchanged resource in place of the new one.

What is the solution? 馃槂

PullRemoteCache deletes all of the resources files. After this, copyResources is called by PackageBin, which puts the latest
version of the resources back.

fixes #6394

What is the problem?
When using remote caching, the resource files are not tracked so if they
have changed, pullRemoteCache will deliver both the old resource file
as well as the changed one.

This is a problem, because it's not the behaviour that our users will
expect and it's not in keeping with the contract of this feature.

Why is this happening?
Zinc, sbt's incremental compiler, keeps track of changes that have
been made. It keeps this in what is called the Analysis file.
However, resource files are not tracked in the Analysis file, so
remote caching is not invalidating the unchanged resource file in
place of the latest version.

What is the solution?
PullRemoteCache deletes all of the resources files. After this,
copyResources is called by PackageBin, which puts the latest
version of the resources back.
Copy link
Member

@eed3si9n eed3si9n left a comment

Choose a reason for hiding this comment

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

Thanks @Nirvikalpa108!

@eed3si9n
Copy link
Member

I think this fix is good on its own, and is ready to merge.

I've actually thought of another cool way to fix this. When we saw copyResources:

val mappings: Seq[(File, File)] = resources.value.flatMap {
case r if !dirs(r) => transform(r).map(r -> _)
case _ => None
}
s.log.debug("Copy resource mappings: " + mappings.mkString("\n\t", "\n\t", ""))
Sync.sync(cacheStore)(mappings)
mappings

that is sort of doing something like incremental compilation, but it's just not virtualized like this PR does. So... if we take Sync.sync (https://github.com/sbt/sbt/blob/4926cb4c2b67d52a6ba0b646275c30735810f038/main-actions/src/main/scala/sbt/Sync.scala) and let it internally manage the list of VirtualFiles, we'd essentially have remote cache ready version of generic file syncing.

@eed3si9n eed3si9n added this to the 1.5.5 milestone Jun 19, 2021
@eed3si9n eed3si9n merged commit 0a33d91 into sbt:develop Jun 19, 2021
sebastian-alfers added a commit to sebastian-alfers/sbt that referenced this pull request Jun 20, 2021
Merge pull request sbt#6551 from samuelClarencTeads/bspIntegrationTestTag

Add the tag "integration-test" to the it configuration in BSP.
fixes remote caching not managing resource files

What is the problem?
When using remote caching, the resource files are not tracked so if they
have changed, pullRemoteCache will deliver both the old resource file
as well as the changed one.

This is a problem, because it's not the behaviour that our users will
expect and it's not in keeping with the contract of this feature.

Why is this happening?
Zinc, sbt's incremental compiler, keeps track of changes that have
been made. It keeps this in what is called the Analysis file.
However, resource files are not tracked in the Analysis file, so
remote caching is not invalidating the unchanged resource file in
place of the latest version.

What is the solution?
PullRemoteCache deletes all of the resources files. After this,
copyResources is called by PackageBin, which puts the latest
version of the resources back.

Merge branch 'develop' into remote-cache
Merge pull request sbt#6554 from Nirvikalpa108/remote-cache

fixes remote caching not managing resource files
Merge branch 'develop' into restore-charridge-return
cleanup spec;

Merge branch 'restore-charridge-return' of github.com:sebastian-alfers/sbt into restore-charridge-return
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Jul 31, 2021
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Jul 31, 2021
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Jul 31, 2021
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Jul 31, 2021
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Jul 31, 2021
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Jul 31, 2021
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Jul 31, 2021
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Jul 31, 2021
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Jul 31, 2021
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Jul 31, 2021
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Jul 31, 2021
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Jul 31, 2021
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Jul 31, 2021
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Oct 30, 2021
Nirvikalpa108 pushed a commit to Nirvikalpa108/sbt that referenced this pull request Oct 30, 2021
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.

Remote caching resurfaces resources with old filenames
2 participants