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

Modified names for (class) is empty #292

Closed
stuhood opened this issue Apr 16, 2017 · 40 comments
Closed

Modified names for (class) is empty #292

stuhood opened this issue Apr 16, 2017 · 40 comments

Comments

@stuhood
Copy link

stuhood commented Apr 16, 2017

Pants 1.3.0.dev16 is using zinc 1.0.0-X7, and users have reported the following exception:

java.lang.AssertionError: assertion failed: Modified names for com.foo.bar.Baz is empty
                          at scala.Predef$.assert(Predef.scala:170)
                          at sbt.internal.inc.NamesChange.<init>(Changes.scala:31)
                          at sbt.internal.inc.IncrementalNameHashing.sameAPI(IncrementalNameHashing.scala:35)
                          at sbt.internal.inc.IncrementalCommon.sameClass(IncrementalCommon.scala:158)
                          at sbt.internal.inc.IncrementalCommon$$anonfun$7.apply(IncrementalCommon.scala:140)
                          at sbt.internal.inc.IncrementalCommon$$anonfun$7.apply(IncrementalCommon.scala:139)
                          at scala.collection.TraversableLike$$anonfun$flatMap$1.apply(TraversableLike.scala:241)
                          at scala.collection.TraversableLike$$anonfun$flatMap$1.apply(TraversableLike.scala:241)
                          at scala.collection.immutable.HashSet$HashSet1.foreach(HashSet.scala:316)
                          at scala.collection.immutable.HashSet$HashTrieSet.foreach(HashSet.scala:972)
                          at scala.collection.immutable.HashSet$HashTrieSet.foreach(HashSet.scala:972)
                          at scala.collection.TraversableLike$class.flatMap(TraversableLike.scala:241)
                          at scala.collection.AbstractTraversable.flatMap(Traversable.scala:104)
                          at sbt.internal.inc.IncrementalCommon.changedIncremental(IncrementalCommon.scala:139)
                          at sbt.internal.inc.IncrementalCommon.cycle(IncrementalCommon.scala:47)
                          at sbt.internal.inc.Incremental$$anonfun$1.apply(Incremental.scala:78)
                          at sbt.internal.inc.Incremental$$anonfun$1.apply(Incremental.scala:77)
                          at sbt.internal.inc.Incremental$.manageClassfiles(Incremental.scala:122)
                          at sbt.internal.inc.Incremental$.compile(Incremental.scala:77)
                          at sbt.internal.inc.IncrementalCompile$.apply(Compile.scala:60)
                          at sbt.internal.inc.IncrementalCompilerImpl.compileInternal(IncrementalCompilerImpl.scala:132)
                          at sbt.internal.inc.IncrementalCompilerImpl.incrementalCompile(IncrementalCompilerImpl.scala:87)
                          at org.pantsbuild.zinc.Compiler.compile(Compiler.scala:204)
                          at org.pantsbuild.zinc.Main$.run(Main.scala:97)
                          at org.pantsbuild.zinc.Main$.main(Main.scala:16)
                          at org.pantsbuild.zinc.Main.main(Main.scala)
                          at sun.reflect.GeneratedMethodAccessor1.invoke(Unknown Source)
                          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
                          at java.lang.reflect.Method.invoke(Method.java:498)
                          at com.martiansoftware.nailgun.NGSession.run(NGSession.java:280)

...while recompiling after switching branches. The affected class was changed in the branch, it's possible others were as well.

@jvican
Copy link
Member

jvican commented Apr 17, 2017

Hey @stuhood, thanks for reporting.

It seems that there are some Zinc invariants that are being violated. It's surprising that this change happened from last X6. Can you provide more information on this? Perhaps a way to reproduce? If you can reproduce, I can fix! 😄.

@stuhood
Copy link
Author

stuhood commented Apr 17, 2017

@jvican : The last version that was widely deployed with pants was 1.0.0-X5.

Will be on the lookout for a repro.

@jvican
Copy link
Member

jvican commented May 7, 2017

I remember now that this invariant existed to check that the name hashing algorithm worked well. I guess Gregorz didn't consider the possibility that branches could be switched.

The best thing would be to remove this assertion and make sure that it works correctly. Next version will have this change @stuhood.

@jvican jvican added this to the 1.0.0-RC1 milestone May 17, 2017
@jvican jvican modified the milestones: 1.0.0-RC1, 1.0.0-RC2 Jul 16, 2017
@dwijnand
Copy link
Member

In #118 (which I'm closing in favour of this more commented issue) it would appear Martin fixed this in 0.13.11 with sbt/sbt#2441? Or maybe not. It's got 36 comments. Have a little look.

@dwijnand
Copy link
Member

There's a duplicate report sbt/sbt#3579 with a very minimised test case.

@marekzebrowski
Copy link

I experienced the same problem in 1.0.2

java.lang.AssertionError: assertion failed: Modified names for X is empty
[error] 	at scala.Predef$.assert(Predef.scala:219)
[error] 	at sbt.internal.inc.NamesChange.<init>(Changes.scala:54)
[error] 	at sbt.internal.inc.IncrementalNameHashing.sameAPI(IncrementalNameHashing.scala:46)
[error] 

I don't have minimised version yet

cunei pushed a commit to cunei/zinc that referenced this issue Oct 25, 2017
@marekzebrowski
Copy link

and in 1.0.3

java.lang.AssertionError: assertion failed: Modified names for X is empty
[error] 	at scala.Predef$.assert(Predef.scala:219)
[error] 	at sbt.internal.inc.NamesChange.<init>(Changes.scala:54)
[error] 	at sbt.internal.inc.IncrementalNameHashing.sameAPI(IncrementalNameHashing.scala:46)
[error]

@jvican
Copy link
Member

jvican commented Oct 27, 2017

Can you provide more information how that happened? @marekzebrowski A way to reproduce would be greatly appreciated.

@marekzebrowski
Copy link

Well, I just ran my build (multi-project, rather large). It works with 0.13.16, but fails with 1.0.[2,3].
Unfortunately code it is not open source, so I can't disclose it and I don't have any minified reproducer yet.
I can reproduce it locally perfectly, so if there is a way to get some better diagnostics, or run a different zinc version, I can help with debugging.

@jvican
Copy link
Member

jvican commented Oct 27, 2017

Can you please run debug in the sbt shell and copy paste the sbt shell output from a clean compile?

@insdami
Copy link

insdami commented Oct 27, 2017

I had the same issue, it got fixed after running rm -rf target/. Unfortunately, I couldn't reproduce it again

@marekzebrowski
Copy link

hmm, after removing all targets I can't reproduce it as well.

@jvican
Copy link
Member

jvican commented Nov 23, 2017

Sorry, I didn't realise this issue still exists in 1.0.3, must be another (related) bug.

@jvican jvican reopened this Nov 23, 2017
@jvican jvican self-assigned this Nov 23, 2017
@NeQuissimus
Copy link

Still an issue with 1.0.4

[error] ## Exception when compiling 2 sources to /home/nequi/dev/sbt_3579/target/scala-2.12/test-classes
[error] assertion failed: Modified names for SbtBugSpec is empty
[error] scala.Predef$.assert(Predef.scala:219)
[error] sbt.internal.inc.NamesChange.<init>(Changes.scala:54)
[error] sbt.internal.inc.IncrementalNameHashing.sameAPI(IncrementalNameHashing.scala:46)
[error] sbt.internal.inc.IncrementalCommon.sameClass(IncrementalCommon.scala:205)
[error] sbt.internal.inc.IncrementalCommon.$anonfun$changedIncremental$1(IncrementalCommon.scala:187)
[...]

@brunojppb
Copy link

I Had the same issue here and the only solution was to clean up all target folders and recompile everything again:

$ find . -regex ".*/target" -exec rm -r {} +
$ sbt compile

@jvican
Copy link
Member

jvican commented Nov 30, 2017

I'll try to look at this again next week.

@otonielortega
Copy link

otonielortega commented Dec 13, 2017

Ran into this after overriding hashcode and equals method on a class. Removing target folders as described above allowed compilation. For intellij I also had to invalidate and restart the IDE to get the application to run

@NeQuissimus
Copy link

git add . && git clean -xdf has become a good friend of mine because of this :D

@dwijnand dwijnand modified the milestones: 1.1.0, 1.something Dec 13, 2017
@dcecile
Copy link

dcecile commented Dec 20, 2017

It's not minimal, but I think I've got a version of it reproduced over here:

https://github.com/dcecile/scala-koans/tree/22a8d8242b6d959fcb47520799c0f4f76db3f1ce

Running sbt '~test' from a terminal, any change I make triggers the assertion, and clearing out target directories doesn't stop the assertion from getting triggered again on the next incremental build.

Maybe because I'm dealing with a ScalaTest subclass with no methods, here's how I clear out the error:

  1. Add a dummy method to the classes
  2. Incrementally compile
  3. Remove the dummy method
  4. Incrementally compile again
  5. (Repeat from 1. for other classes as needed)

@NeQuissimus
Copy link

@dcecile sbt/sbt#3579 has a minimal reproduction :)

@francisdb
Copy link

francisdb commented Dec 26, 2017

Also seeing these with 1.1.0-RC4 when moving methods around

Simpler reproducer below

trait T2{
}
trait T1 extends T2{
  def foo: String
}
class C1 extends T1{
  def foo = "test"
}
~compile

now move foo from T1 to T2

[error] ## Exception when compiling 14 sources to /Users/me/workspace/proj/target/classes
[error] assertion failed: Modified names for proj.T1 is empty
[error] scala.Predef$.assert(Predef.scala:219)
[error] sbt.internal.inc.NamesChange.<init>(Changes.scala:54)
[error] sbt.internal.inc.IncrementalNameHashing.sameAPI(IncrementalNameHashing.scala:46)

@ceilican
Copy link

I'm experiencing this issue too, but only when I try to compile from IntelliJ IDEA. From the command line, everything works fine.

@ashleymercer
Copy link

Just encountered the same issue, and like the example from @francisdb moved a method from a trait down to the concrete implementation.

Possibly the change detection isn't accounting for exactly which class / trait a method is defined on? So after a move, the complete set of method names and signatures is identical, but the declaration sites have changed.

jvican added a commit to scalacenter/zinc that referenced this issue Jan 17, 2018
This assertion dates from the time where the whole Zinc 1.x was under
development. As I understand it, it was put in place to make sure that
the names invalidation was correct.

However, the assertion itself is not correct. It violates some legit
scenarios that are valid, where there are changes in APIs but there are
no changes in the names.

One of these examples is the following one, provided by @francisdb,
where the original code is:

```
trait T2
trait T1 extends T2 { def foo: String }
class C1 extends T1 { def foo = "test" }
```

And moving `foo` from `T1` to `T2` causes the assertion to fail. In this
concrete scenario, we can see how the API has changed (the hashes are
not the same than the previous scenario) but the names are identical.

There are more examples, some of them I have occassionally run into.

Therefore, this commit should fix sbt#292 once and for all, and end the
pain of those who have kindly complained about it in the ticket. :)
@jvican
Copy link
Member

jvican commented Jan 17, 2018

PR is up #484. Thank you for your patience.

jvican added a commit to scalacenter/zinc that referenced this issue Jan 21, 2018
This assertion dates from the time where the whole Zinc 1.x was under
development. As I understand it, it was put in place to make sure that
the names invalidation was correct.

However, the assertion itself is not correct. It violates some legit
scenarios that are valid, where there are changes in APIs but there are
no changes in the names.

One of these examples is the following one, provided by @francisdb,
where the original code is:

```
trait T2
trait T1 extends T2 { def foo: String }
class C1 extends T1 { def foo = "test" }
```

And moving `foo` from `T1` to `T2` causes the assertion to fail. In this
concrete scenario, we can see how the API has changed (the hashes are
not the same than the previous scenario) but the names are identical.

There are more examples, some of them I have occassionally run into.

Therefore, this commit should fix sbt#292 once and for all, and end the
pain of those who have kindly complained about it in the ticket. :)
jvican added a commit to scalacenter/zinc that referenced this issue Jan 22, 2018
This assertion dates from the time where the whole Zinc 1.x was under
development. As I understand it, it was put in place to make sure that
the names invalidation was correct.

However, the assertion itself is not correct. It violates some legit
scenarios that are valid, where there are changes in APIs but there are
no changes in the names.

One of these examples is the following one, provided by @francisdb,
where the original code is:

```
trait T2
trait T1 extends T2 { def foo: String }
class C1 extends T1 { def foo = "test" }
```

And moving `foo` from `T1` to `T2` causes the assertion to fail. In this
concrete scenario, we can see how the API has changed (the hashes are
not the same than the previous scenario) but the names are identical.

There are more examples, some of them I have occassionally run into.

Therefore, this commit should fix sbt#292 once and for all, and end the
pain of those who have kindly complained about it in the ticket. :)
@dwijnand dwijnand modified the milestones: 1.something, 1.1.1 Jan 22, 2018
@dwijnand
Copy link
Member

Fixed in #484.

@y2k-shubham
Copy link

y2k-shubham commented Feb 10, 2018

I'm on SBT v1.0.3 (Scala v2.11.11) and have remained on the same version since starting the project (over 3 months back). I've never faced this issue, until now that is; but I've encountered it in a rather strange fashion.

I'm able to compile the code and build fat-JAR (sbt assembly) on my Mac. But after having pushed the code to GitHub, Jenkins fails to build fat-JAR with this error message (Modified names..)

@y2k-shubham
Copy link

Okay so it turns out that the culprit is conflict(s) between one of the following (still not sure who the real culprit is)

  1. ScalikeJdbc's SQL-template (for SQL-interpolation)
  2. A user-defined method called 'sql' (in the same file where I'm using SQL-interpolation; although renaming that method didn't have any impact)
  3. A user-defined method called 'sql' defined in an implicit class inside a package object
  4. A user-defined package called 'sql'

In short, it's a mess right now and the word 'sql' can refer to too many things in my code.
I'll update with the root cause and actual solution, once I find them.

@eed3si9n
Copy link
Member

Please try sbt 1.1.1.

@y2k-shubham
Copy link

Sorry for all the chaos; turns out that the issue was completely unrelated with this thread or even zinc.

While the problem that I described above (identifier "sql" messing up the namespace) sounded like obvious failure-point, it wasn't really the culprit here [package-names don't affect the code (my bad judgement) and conflict between implicit 'sql' method(s) was easily solved by moving corresponding imports inside individual methods where they were being invoked rather than having them globally].

Rather strangely, my fault was that I had accidently invoked an abstract super.method() inside an overridden method from a trait. What's disturbing is this error message did not make the slightest indication of the actual problem (and leading me to think towards completely unrelated directions)

Please correct me if I'm wrong.
Otherwise I plan to add this as QA on StackOverflow for reference to others who face this issue.

@eed3si9n
Copy link
Member

@y2k-shubham Feel free to open a new ticket describing the reproduction steps and the error message you see.

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