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

Handle managedSources writing into unmanaged source directories #4099

Merged
merged 1 commit into from May 28, 2018

Conversation

Projects
None yet
3 participants
@eatkins
Copy link
Contributor

commented Apr 13, 2018

When source generators write into the unmanaged source directory, bad
things can happen. Continuous builds will loop indefinitely and
compiling will fail because the generated sources get added to the
source list twice, causing the incremental compiler to complain about
compiling classes it has already seen. My two-pronged solution is to
de-duplicate the sources task and to filter out managed source files in
watch sources. The drawback to the latter is that it causes the source
generation task to be executed twice per compile.

(See the guidelines for contributing, linked above)

@eed3si9n eed3si9n added the ready label Apr 13, 2018

@eatkins eatkins force-pushed the eatkins:redundant branch from 48d665e to a13e64a Apr 14, 2018

@@ -342,7 +356,7 @@ object Defaults extends BuildCommon {
sourceDirectories := Classpaths
.concatSettings(unmanagedSourceDirectories, managedSourceDirectories)
.value,
sources := Classpaths.concat(unmanagedSources, managedSources).value
sources := Classpaths.concat(unmanagedSources, managedSources).value.distinct

This comment has been minimized.

Copy link
@dwijnand

dwijnand Apr 23, 2018

Member

is there no way to avoid this?

This comment has been minimized.

Copy link
@dwijnand

dwijnand Apr 23, 2018

Member

reason I ask is distinct is like null-checks: you could fill your codebase with them. so I'd rather avoid it, and fix the root cause - if possible.

This comment has been minimized.

Copy link
@eatkins

eatkins Apr 23, 2018

Author Contributor

Looking more at this file, it could be sources := Classpaths.concatDistinct(unmanagedSources, managedSources).value (probably not what you mean though...). I can't really think of a way to guarantee that there is no overlap between managed sources and unmanaged sources since there is nothing that enforces that these files live in isolated directories. I guess you could do something like

sources := {
  val unmanaged = unmanagedSources.value
  val unmanagedDirectories = unmanaged.view.map(_.toPath.getParent).toSet
  unmanaged ++ managedSources.value.filterNot(f => unmanagedDirectories.contains(p.toPath.getParent))
}

This comment has been minimized.

Copy link
@dwijnand

dwijnand Apr 24, 2018

Member

OK.. Thanks for spending the time to look into this. Let's use Classpaths.concatDistinct I guess.

val baseSources =
if (sourcesInBase.value) Seq(new Source(baseDir, include, exclude, recursive = false))
if (sourcesInBase.value)
Seq(new Source(baseDir, include, exclude || new SimpleFileFilter({ f =>

This comment has been minimized.

Copy link
@eatkins

eatkins Apr 23, 2018

Author Contributor

I can remove this diff if this commit is brought in after #4096 (which makes the recursive source filter out files not in the base path).

This comment has been minimized.

Copy link
@dwijnand

dwijnand Apr 24, 2018

Member

OK, let's see how things land.

@dwijnand dwijnand added this to the 1.1.5 milestone Apr 27, 2018

Ethan Atkins
Remove watch loops
When source generators write into the unmanaged source directory, bad
things can happen. Continuous builds will loop indefinitely and
compiling will fail because the generated sources get added to the
source list twice, causing the incremental compiler to complain about
compiling classes it has already seen. My two-pronged solution is to
de-duplicate the sources task and to filter out managed source files in
watch sources. The drawback to the latter is that it causes the source
generation task to be executed twice per compile.

@eatkins eatkins force-pushed the eatkins:redundant branch from a13e64a to 0de8345 Apr 27, 2018

@dwijnand dwijnand modified the milestones: 1.1.5, 1.1.6 May 15, 2018

@eed3si9n eed3si9n merged commit 7e8e18b into sbt:1.1.x May 28, 2018

3 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@eatkins eatkins deleted the eatkins:redundant branch Oct 9, 2018

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.