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

Scala.js 0.6.30 does not acknowledge IR version 0.6.29 as supported. #3865

Closed
sjrd opened this issue Nov 19, 2019 · 7 comments
Closed

Scala.js 0.6.30 does not acknowledge IR version 0.6.29 as supported. #3865

sjrd opened this issue Nov 19, 2019 · 7 comments
Assignees
Labels
bug
Milestone

Comments

@sjrd
Copy link
Member

@sjrd sjrd commented Nov 19, 2019

We forgot to reset binaryEmitted to "0.6.29", and add "0.6.29" to the list of binarySupported.

That means that when using a library built with 0.6.29 from a 0.6.30, the deserializer complains that it does not support version 0.6.29 (although in fact it does).

This is a critical issue that makes 0.6.30 completely unusable. Unfortunately it's already on Maven Central, so we'll have to release 0.6.31 to fix it.

@sjrd sjrd added the bug label Nov 19, 2019
@sjrd sjrd self-assigned this Nov 19, 2019
@sjrd sjrd added this to the v0.6.31 milestone Nov 19, 2019
@gzm0

This comment has been minimized.

Copy link
Contributor

@gzm0 gzm0 commented Nov 19, 2019

Oh dear :-/

fthomas added a commit to fthomas/scala-steward that referenced this issue Nov 19, 2019
fthomas added a commit to fthomas/scala-steward that referenced this issue Nov 19, 2019
* Mark Scala.js 0.6.30 as bad

See scala-js/scala-js#3865.

* Also add scalajs-library
sjrd added a commit to sjrd/scala-js that referenced this issue Nov 20, 2019
…0.6.{29-30}.

We also change the "rules" for what to put in `binaryEmitted` so
that hopefully we can avoid this issue in the future.
sjrd added a commit that referenced this issue Nov 20, 2019
Fix #3865: Set the emitted IR version to 0.6.29, and support 0.6.{29-30}.
@sjrd

This comment has been minimized.

Copy link
Member Author

@sjrd sjrd commented Nov 20, 2019

Fixed in 6281863.

@sjrd sjrd closed this Nov 20, 2019
@gzm0

This comment has been minimized.

Copy link
Contributor

@gzm0 gzm0 commented Nov 20, 2019

It seems that this was high impact enough for us to write a quick post mortem summary / lessons learned:

  • What happened & why (who was impacted if possible)?
  • What went well?
  • What went bad?
  • Where did we get lucky?
  • How are we preventing this in the future?
@sjrd

This comment has been minimized.

Copy link
Member Author

@sjrd sjrd commented Nov 20, 2019

Draft:

What are the symptoms?

When trying to (transitively) use a Scala.js library built with Scala.js 0.6.29 from a Scala.js project using 0.6.30, linking will report an error saying that it cannot read the .sjsir files with the version 0.6.29, and that you should "upgrade" to 0.6.29.

What happened & why?

Scala.js uses .sjsir files as an intermediate representation for compiled---but not yet linked---Scala.js code. These files are very similar to .class files for the JVM, and in particular they are the ones published in .jars on Maven Central. .sjsir files have a binary format, which sometimes evolves in backward binary compatible ways, and sometimes does not change at all, between two patch versions of 0.6.x. Any version of Scala.js 0.6.x can read the IR files produced by all 0.6.y for y <= x, and sometimes some 0.6.z for z > x, as long as no version until z included changed the IR binary format since x. For example, Scala.js 0.6.20 can read .sjsir files produced until 0.6.28 included, because the IR binary format did not change between 0.6.17 and 0.6.28.

In order to provide good error messages when trying to use an .sjsir file that cannot be read, we store the IR binary format version in .sjsir files. An .sjsir file produced by Scala.js 0.6.28 contains the IR version 0.6.17. If we try to read it with an older version, such as 0.6.16, the sbt plugin will check whether "0.6.16" is in the list of versions it supports, and since it isn't, emit an error.

What went bad?

We maintain an explicit list of supported versions as a constant in a .scala file.

When we published 0.6.29, we changed the IR binary format, and so we bumped the version that is stored in .sjsir files. However, when doing so, we forgot to add it to the list of supported versions! Since a version always considers that it supports itself, we did not notice this mistake when publishing 0.6.29. In fact, 0.6.29 itself was perfectly fine, but leaving the code in this mistaken state was setting us up for failure later in 0.6.30.

When we reached 0.6.30, we still had not put 0.6.29 in the list of supported versions. Scala.js 0.6.30 therefore thinks it cannot read .sjsir files produced by 0.6.29, and fails with an early error (although the actual deserializer code is actually capable of reading the files). Scala.js 0.6.30 will also emit .sjsir files with the version 0.6.30, although in fact it produces files that are compatible with 0.6.29.

In order to fix this in 0.6.31, we added both 0.6.29 and 0.6.30 in the list of supported versions, and we told 0.6.31 that it produces .sjsir files with the IR version 0.6.29.

What went well?

When it discovered the 0.6.30 artifacts on Maven Central, Scala Steward started sending PRs to upgrade to Scala.js 0.6.30 to repos that have opted in to its services. Of course the CI failed, and an early user, namely @cquiroz, quickly notified us of the issue, even before we announced 0.6.30 to the world. This allowed us to quickly fix the issue and release a new version.

In addition, since Scala.js 0.6.30 did not, in fact, break binary compatibility, and does store a valid IR version number in the .sjsir files it produces (namely "0.6.30"), the state of the ecosystem of published .jars is not corrupted. Even if some libraries correctly build with 0.6.30 (because they do not depend on any library built with 0.6.29) and are published on Maven Central, they do not make up a distinct, incompatible sub-ecosystem. We are able to use those libraries from Scala.js 0.6.31.

Where did we get lucky?

Not sure what to write here.

How are we preventing this in the future?

In 0.6.x, we have now changed the rules of how we maintain the IR version number for produced .sjsir files. It used to be too easy to miss that it was necessary to fix it to the release version when a new version was released. This is precisely what happened in 0.6.29. Now, during the development period leading to a release with a changed IR format, we will make it clear that the IR version is a SNAPSHOT version, rather than "whatever the Scala.js version is". Hopefully this will make us notice that we need to change it when releasing a version.

In Scala.js 1.x, we are taking even better steps to prevent this kind of issue. In #3874 we switched from an explicit list of supported versions to a SemVer-based algorithm to automatically decide whether a version is supported or not, and whether the IR version produced by a given Scala.js version is compatible with that Scala.js version.

@gzm0

This comment has been minimized.

Copy link
Contributor

@gzm0 gzm0 commented Nov 20, 2019

Thank you for writing this so fast. Mostly LGTM only minor stuff.

Where did we get lucky?: Not sure what to write here.

I think that we do not have botched 0.6.30 jars out there goes in here IMO.

What went well?

You can also put that you / the Scala.js team reacted quickly.

Another action item might be to investigate if we can actively try an link with old versions. scripted might make this not too hard (in combination with the mima previous version setting).

@sjrd

This comment has been minimized.

Copy link
Member Author

@sjrd sjrd commented Nov 20, 2019

Where did we get lucky?: Not sure what to write here.

I think that we do not have botched 0.6.30 jars out there goes in here IMO.

Ah yes, that makes sense.

Another action item might be to investigate if we can actively try an link with old versions. scripted might make this not too hard (in combination with the mima previous version setting).

I thought about that, but I don't know how that would prevent this issue. If we forgot to fix the IR version in 0.6.29, we would also have forgotten to include/update the test making sure we can link against 0.6.29 ;)

@gzm0

This comment has been minimized.

Copy link
Contributor

@gzm0 gzm0 commented Nov 21, 2019

I thought about that, but I don't know how that would prevent this issue.

That's actually a very good point :-/

The only way I think we could prevent this is by keeping a allKnownVersions list in our build. We assert that the current version is in this list if it is not pre-release version. From this list, we then determine mima targets and potential IR compatibility test targets (filtering out the current version).

So in essence, our build would fail to load if we change to a release version and do not include it in the list of all known versions. This would ensure we keep the list up-to-date. (of course it needs a big comment saying only literals may be here).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.