Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

prevent concurrent modifications of sources by limiting concurrency #153

Merged

Conversation

bjaglin
Copy link
Collaborator

@bjaglin bjaglin commented Jul 16, 2020

When rewrite rules are executed, scalafix and scalafixAll may conflict with other tasks transforming unmanagedSources. Since there is no way to identify such tasks given the sbt model, and to be defensive against this scenario, scalafix tasks may only run concurrently with other scalafix tasks, but not with any other tasks.

For explicit invocations, this should not damage throughput, but for complex builds with scalafixOnCompile := true, this can have a limited impact during the first compile; after that caching should make it negligible.

@bjaglin bjaglin force-pushed the prevent-concurrent-modifications branch from f429bd9 to fcfd608 Compare July 16, 2020 22:40
@@ -375,6 +378,9 @@ object ScalafixPlugin extends AutoPlugin {
}
}
}
task.tag(scalafixTag)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
task.tag(scalafixTag)
task

breaks the scripted

[info] [error] (Compile / check) java.lang.AssertionError: assertion failed: missing footer in
[info] [error] <<<// HEADER
[info] [error] object Example
[info] [error] >>>

When rewrite rules are executed, `scalafix` and `scalafixAll` may
conflict with other tasks transforming `unmanagedSources`. Since
there is no way to identify such tasks given the sbt model, and to
be defensive against this scenario, `scalafix` tasks may only run
concurrently with other `scalafix` tasks, but not with any other
tasks.

For explicit invocations, this should not damage throughput, but for
complex builds with `scalafixOnCompile := true`, this can have a
limited impact during the first compile; after that caching should
make it negligible.
@bjaglin bjaglin force-pushed the prevent-concurrent-modifications branch from fcfd608 to fc56a83 Compare July 16, 2020 23:00
@bjaglin
Copy link
Collaborator Author

bjaglin commented Jul 16, 2020

@sjrd I am proposing this to address the concerns you expressed in #140 (comment)

modifying files in a task (either input files or files produced by earlier tasks) is a very bad thing, and you should never do that, ever. The parallelism of sbt means that things can execute concurrently on already created files, causing very bad issues. If you want to modify things, use a command instead (and no, you can't execute a command from a task, not in a sound way anyway).

@bjaglin bjaglin marked this pull request as ready for review July 16, 2020 23:01
@bjaglin bjaglin requested a review from olafurpg July 16, 2020 23:07
Copy link
Contributor

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

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

LGTM 👍

I am constantly impressed with your ability to write integration tests that stress tricky to reproduce scenarios.

FYI I’ll be away for a few weeks and may not comment much on GitHub

@bjaglin
Copy link
Collaborator Author

bjaglin commented Jul 17, 2020

FYI I’ll be away for a few weeks and may not comment much on GitHub

Enjoy your break! I'll also be AFK some random weeks during the next 5 ones.

@bjaglin bjaglin merged commit dece553 into scalacenter:master Sep 4, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants