scala / pickling

Fast, customizable, boilerplate-free pickling support for Scala
lampwww.epfl.ch/~hmiller/pickling
BSD 3-Clause "New" or "Revised" License
831 stars 79 forks source link

Apparent regression #52

Closed emchristiansen closed 11 years ago

emchristiansen commented 11 years ago

I think a bug was added within the last 11 days to 0.8.0-SNAPSHOT. I haven't been able to reproduce it as a test case in the scala-pickling project, but I do have a fairly simple external project in which the bug shows up.

The project is https://github.com/emchristiansen/PersistentMap. It passed its builds on 3 October, but failed when it was rebuilt today. Nothing of significance changed in that time (the logs show the Scala version changed, but I have also tested the previous version).

The build error comes from this file: https://github.com/emchristiansen/PersistentMap/blob/master/src/test/scala/st/sparse/persistentmap/testPersistentMap.scala

The error is

[error] /media/psf/Home/Dropbox/t/2013_q4/persistentmap/src/test/scala/st/sparse/persistentmap/testPersistentMap.scala:52: Cannot generate an unpickler for st.sparse.persistentmap.MyValue. Recompile with -Xlog-implicits for details
[error]     val map = PersistentMap.create[MyKey, MyValue]("test", database)

Enabling -Xlog-implicits, we have

[info] /media/psf/Home/Dropbox/t/2013_q4/persistentmap/src/test/scala/st/sparse/persistentmap/testPersistentMap.scala:93: pickling.this.Unpickler.genUnpickler is not a valid implicit value for scala.pickling.Unpickler[st.sparse.persistentmap.MyValue] because:
[info] hasMatchingSymbol reported error: value forcefulSet is not a member of reflect.runtime.universe.FieldMirror
[info]     val map2 = PersistentMap.connect[MyKey, MyValue]("test", database).get

I have also distilled the error into a test case here: https://github.com/emchristiansen/pickling/blob/picklerGenerationTest/core/src/test/scala/pickling/generic-spickler.scala. See the test labeled "possible regression".

phaller commented 11 years ago

This is very likely due to the fact that a number of implicit classes have been moved to a new internal package. Apparently, these are not in scope anymore at one of the required places, since forcefulSet cannot be found. It should be a simple fix.

emchristiansen commented 11 years ago

Indeed, internal does provide a forcefulSet pimp for any FieldMirror. Unfortunately, it is private, so I can't just import it to make my project work.

LBliss commented 11 years ago

Similar (duplicate?) with: https://github.com/scala/pickling/issues/50

emchristiansen commented 11 years ago

It's painful when a regression like this happens and there's no old package to fall back on, due to the transitory nature of SNAPSHOTs. Think we could get a 0.8.0-M1 or the like when this issue is resolved?

LBliss commented 11 years ago

I tried your tests with the latest version (of the source files, not the jar), and they seem to succeed:

[info] GenericSpickler:
[info] - stack-overflow-pickle-unpickle
[info] - possible regression
[info] Passed: Total 2, Failed 0, Errors 0, Passed 2
[success] Total time: 160 s, completed 16 oct. 2013 01:37:19

However, the one from #50 still fail.

test("Issue #50") {
  case class TestA (x: Option[Int])
  val a = TestA(Some(1))
  assert(a.pickle.unpickle[TestA] == a)

  case class TestB (x: Option[String])
  val b = TestB(Some("hello"))
  assert(b.pickle.unpickle[TestB] == b)
}
emchristiansen commented 11 years ago

Yeah, I know :/ For some reason, I'm not able to replicate it using pickling's internal tests (BTW, I said this in the original bug report, but I don't think I was very clear). I'm not sure what would be causing the discrepancy, unless the currently published SNAPSHOT doesn't reflect the current source.

On Tue, Oct 15, 2013 at 4:38 PM, LBliss notifications@github.com wrote:

I tried your tests with the latest version, and they seem to pass:

[info] GenericSpickler: [info] - stack-overflow-pickle-unpickle [info] - possible regression [info] Passed: Total 2, Failed 0, Errors 0, Passed 2 [success] Total time: 160 s, completed 16 oct. 2013 01:37:19

— Reply to this email directly or view it on GitHubhttps://github.com/scala/pickling/issues/52#issuecomment-26382029 .

phaller commented 11 years ago

I could reproduce it on PersistentMap using the latest SNAPSHOT artifact. I submitted a PR with a fix.

phaller commented 11 years ago

About releases: the plan is to do periodic releases (including milestones) as soon as a decision has been made on namespace management. Expect this decision within the next couple of days.

heathermiller commented 11 years ago

RE: @emchristiansen

It's painful when a regression like this happens and there's no old package to fall back on, due to the transitory nature of SNAPSHOTs. Think we could get a 0.8.0-M1 or the like when this issue is resolved?

We feel you. As @phaller indicated, the problem is actually namespace management and maven group IDs. We're waiting for a group of people to reach a consensus on this matter, which is unfortunately holding us back from publishing actual milestones which people can rely on. :-/

It's coming though, we're with you on this.

heathermiller commented 11 years ago

I believe all is OK for PersistentMap as well – it compiles fine, and all of the pickling tests pass (there are 3 failing SQL tests though, because I think they can't connect to some database somewhere)

emchristiansen commented 11 years ago

I'm still having issues with the version on Sonatype. As of now, the last SNAPSHOT was on Tuesday. Is this something that requires a few hours to refresh?

BTW, you were correct in that those MySQL tests expect a Travis CI environment.

heathermiller commented 11 years ago

Yep, you'll need to wait until the next snapshot is generated and uploaded to sonatype. Should be there by tomorrow. Alternatively, if you want to test pickling right away, you can checkout pickling, from sbt run package and copy the jar that sbt generates (sbt will tell you where it was generated) to a directory called lib in the root of PersistentMap. After that, go to your build file in PersistentMap and comment out the pickling dependency. Sbt will find pickling in that lib directory automatically.

emchristiansen commented 11 years ago

Okay, thanks for your help!

I'm curious, though, where the publishing lag is coming from. Are you using an automated system that builds and publishes your code each night? Or is that how long Sonatype takes to refresh?

heathermiller commented 11 years ago

Yep, we have a jenkins job that builds, tests, and publishes pickling on a nightly basis. Though @xeno-by just pointed out to me that I could manually trigger the job (duh). That's been done, so the sonatype jar should now be current :)