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

Restart of sbt is required after changing the value of the name hashing flag #1081

Closed
gkossakowski opened this issue Jan 14, 2014 · 0 comments

Comments

@gkossakowski
Copy link
Contributor

If you change the value of the name hashing flag then simply running clean followed by compile is not enough to switch to different implementation of incremental compilation algorithm. It turns out, a full restart of sbt is required because Analysis objects are cached in memory by incremental compiler.

The caching was introduced in 135609e. The commit message said it was temporary hack but it never got reverted.

We'll need to investigate whether the hack is needed at all and if so, we'll need to clarify caching strategy.

gkossakowski added a commit to gkossakowski/sbt that referenced this issue Feb 17, 2014
The CompileSetup class is being used to detect changes to arguments of
incremental compiler that affect result of compilation and trigger
recompilation. Examples of such arguments include, the target (output)
directory, Scala compiler options, Scala compiler version, etc.

By adding `nameHashing` to CompileSetup we have a chance to handle change
to that flag smoothly by throwing away old Analysis object and starting
with an empty one. That's implemented in AggressiveComile by extending
the logic that was responsible for detection of changes to CompileSetup
values. Thanks to this change we fix sbt#1081.

Analysis formats has been updated to support persisting of newly added
value in CompileSetup. We used to not store the value of `nameHashing`
flag in persisted Analysis file and infer it from contents of relations
but that leads to issue sbt#1071 when empty relations are involved. Given
the fact that CompileSetup stores `nameHashing` value now, we can just
use it when reading relations and fix sbt#1071. This requires reading/writing
compile setup before reading relations. I decided to make that change even
if there's a comment saying that reading/writing relations first was done
intentionally.
@gkossakowski gkossakowski self-assigned this Mar 2, 2014
eed3si9n pushed a commit to eed3si9n/sbt that referenced this issue Mar 21, 2014
The CompileSetup class is being used to detect changes to arguments of
incremental compiler that affect result of compilation and trigger
recompilation. Examples of such arguments include, the target (output)
directory, Scala compiler options, Scala compiler version, etc.

By adding `nameHashing` to CompileSetup we have a chance to handle change
to that flag smoothly by throwing away old Analysis object and starting
with an empty one. That's implemented in AggressiveComile by extending
the logic that was responsible for detection of changes to CompileSetup
values. Thanks to this change we fix sbt#1081.

Analysis formats has been updated to support persisting of newly added
value in CompileSetup. We used to not store the value of `nameHashing`
flag in persisted Analysis file and infer it from contents of relations
but that leads to issue sbt#1071 when empty relations are involved. Given
the fact that CompileSetup stores `nameHashing` value now, we can just
use it when reading relations and fix sbt#1071. This requires reading/writing
compile setup before reading relations. I decided to make that change even
if there's a comment saying that reading/writing relations first was done
intentionally.
gkossakowski added a commit to gkossakowski/sbt that referenced this issue Sep 23, 2014
The 135609e introduced "temporary hack"
that was meant to help with not reading Analysis object from disk on every
compile. It succeeded so the hack stayed for 3 years. We move the logic
introduced in 135609e to a separate object
called `AnalysisStoreCache`. This way we have a place to stick
documentation onto and the cache becomes a first-class concept.

Also, documented the fact that AnaysisStoreCache has no eviction policy
which leads to problem described in sbt#1081.

This commit changes API of AggressiveCompile class but as far as I know
it's not part of public API used by zinc or IDE so we should be ok. I've
marked AggressiveCompiler as `private[sbt]` to clarify that.
gkossakowski added a commit to gkossakowski/sbt that referenced this issue Sep 23, 2014
The CompileSetup class is being used to detect changes to arguments of
incremental compiler that affect result of compilation and trigger
recompilation. Examples of such arguments include, the target (output)
directory, Scala compiler options, Scala compiler version, etc.

By adding `nameHashing` to CompileSetup we have a chance to handle change
to that flag smoothly by throwing away old Analysis object and starting
with an empty one. That's implemented in AggressiveComile by extending
the logic that was responsible for detection of changes to CompileSetup
values. Thanks to this change we fix sbt#1081.

Analysis formats has been updated to support persisting of newly added
value in CompileSetup. We used to not store the value of `nameHashing`
flag in persisted Analysis file and infer it from contents of relations
but that leads to issue sbt#1071 when empty relations are involved. Given
the fact that CompileSetup stores `nameHashing` value now, we can just
use it when reading relations and fix sbt#1071. This requires reading/writing
compile setup before reading relations. I decided to make that change even
if there's a comment saying that reading/writing relations first was done
intentionally.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant