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

Consider bumping scala-compiler's scala-xml dependency to 2.x in Scala 2.12 #12632

Closed
ckipp01 opened this issue Aug 9, 2022 · 14 comments
Closed
Assignees
Labels
Milestone

Comments

@ckipp01
Copy link
Member

ckipp01 commented Aug 9, 2022

Problem outline

I know this was a conversation a long time ago in scala/scala#9743 but recently this conversation has surfaced again. Recently in sbt-scoverage I had some reports of conflicts since I was still on 1.x, but some other libraries like scalatest bumped to 2.x making the problem hit people a bit harder. Due to that, I went ahead and updated to 2.x. Due to me updating, we got even more reports of conflicts with sbt-native-packager, which caused them to update to 2.x as well. Now we're in a position where people are getting hit with stuff like this:

[error]         * org.scala-lang.modules:scala-xml_2.12:2.1.0 (early-semver) is selected over {1.3.0, 1.0.6, 1.2.0}
[error]             +- org.scoverage:scalac-scoverage-reporter_2.12:2.0.2 (depends on 2.1.0)
[error]             +- com.github.sbt:sbt-native-packager:1.9.10 (sbtVersion=1.0, scalaVersion=2.12) (depends on 2.1.0)
[error]             +- org.scala-sbt:testing_2.12:1.7.1                   (depends on 1.3.0)
[error]             +- org.scala-sbt:sbinary_2.12:0.5.1                   (depends on 1.0.6)
[error]             +- org.scala-sbt:main_2.12:1.7.1                      (depends on 1.3.0)
[error]             +- org.scala-sbt:librarymanagement-core_2.12:1.7.0    (depends on 1.2.0)
[error]             +- org.scala-lang:scala-compiler:2.12.16              (depends on 1.0.6)
[error]             +- io.get-coursier:lm-coursier-shaded_2.12:2.0.10     (depends on 1.3.0)

Where common sbt plugins are conflicting with sbt and Scala 2.12 itself. Looking back at past conversations I understand the breaking changes between 1.x and 2.x of scala-xml are extremely small so doing the following is pretty safe:

in your project/plugins.sbt

ThisBuild / libraryDependencySchemes ++= Seq(
  "org.scala-lang.modules" %% "scala-xml" % VersionScheme.Always
)

But this isn't ideal to have this all over the place, and will cause confusions for users. Is it time to update scala-xml in 2.12? Is there something holding this back? We're in a weird position now where have the ecosystem is update for 2.12 and half isn't, causing issues.

@guizmaii
Copy link

guizmaii commented Aug 9, 2022

FYI, we're having this issue right now

@julienrf
Copy link

julienrf commented Aug 9, 2022

Bumping to 2.x will also create issues in projects that depend on 1.x in some way. However, I agree that we should initiate the migration at some point.

@ckipp01
Copy link
Member Author

ckipp01 commented Aug 9, 2022

Bumping to 2.x will also create issues in projects that depend on 1.x in some way.

For sure, but we have the ecosystem sort of split right now. So there already are problems. Also, from my understanding an update from scala-xml 1.x to 2.x is quite trivial. So if we're going to cause issues for 2.x users or 1.x users, then I think it's wise to lean in on everyone updating, especially since the train has left the station.

I'm happy to pr this change, and can even help out around the ecosystem with pr'ing bumps to 2.x in projects that are lagging behind.

@eed3si9n
Copy link
Member

eed3si9n commented Aug 9, 2022

Agreed on scala-xml 2.x. I am happy to update sbt dependency in the next feature release 1.8.x.

persistent versioning?

Related thought. In the age of scala-streward where people bump up versions the day a library gets released, we should be more mindful of any major version bumps for any org.scala-lang.modules that were formerly standard library, because effectively they remain more or less to be standard library.
One way to workaround the conundrum of progress vs stability is making the library persistent. https://eed3si9n.com/persistent-versioning/. A persistent library embeds the major version number in the organization name and package name, like org.scala-lang.modules.xml2 and scala.xml2. This will cause no version conflict since they will look like different libraries from library management perspective.

_2.13 vs _3

Related thought 2. Even if we convince all library authors to migrate to scala-xml 2.x, an interesting issue remains for Scala 2.13-3.x dependency, such as CrossVersion.for3Use2_13. I recently wanted to use 2.13 library from Scala 3, and again because scala-xml etc are effectively standard library, they popped up as:

[info] Fetched artifacts of
[error] Modules were resolved with conflicting cross-version suffixes in ProjectRef(...):
[error]    org.scala-lang.modules:scala-xml _2.13, _3

This got me thinking that maybe we should all consider not using the _3 variant for things like scala-xml, scala-collection-compat, etc. Or put another way, the price of using _3 variant for standard library is that CrossVersion.for3Use2_13 becomes somewhat useless. If we as ecosystem want to maximize the freedom for application developers, it might be worth standardizing standard modules to _2.13?

@SethTisue SethTisue changed the title Consider bumping scala-xml to 2.x Consider bumping scala-xml to 2.x in Scala 2.12 Aug 10, 2022
@SethTisue
Copy link
Member

SethTisue commented Aug 10, 2022

I support including this upgrade in Scala 2.12.x (.17 or .18).

We might have to rethink if new information comes to light. For example, we'll want to see if scala/scala CI results contain any surprises, and the same for the Scala 2.12 community build. But based on what I know today, I'm in favor.

Just now I reviewed the various scala-xml 2.x release notes at https://github.com/scala/scala-xml/releases for potential compatibility concerns. The following PRs removed API. That bincompat breakage, even if largely rare/theoretical, was the forcing factor for the major version bump:

but it's all obscure deprecated stuff that's unlikely to have been in use by almost anybody. Well, scala.xml.pull is probably a bit likelier to have been in use, but I'm confident it was never in wide use.

A potential compatibility concern is:

This seems to me like the other most significant change (besides the scala.xml.pull removal) that some people might need to be aware of (most will not). We could link to it from the release notes.

We can't be sure a few people won't be bitten by other changes — there are other behavioral changes between 1.x and 2.x and any behavioral change might trip up somebody. Regardless, I think we should go forward with the upgrade.

If we as ecosystem want to maximize the freedom for application developers, it might be worth standardizing standard modules to _2.13?

That discussion, and the one about “persistent versioning”, is out of scope on this ticket, I think. Here, let's stick to the narrower question of what Scala 2.12.x's dependencies should be.

fyi @lrytz @ashawley

@ashawley
Copy link
Member

Releasing 2.0 last year was known to be both a binary compatibility change and a behavior change but overall it was intended to be as conservative a major release as possible. The version bump was necessary, but the deprecated removals were of "vestigial code that is either unused, of poor quality or both" and that no user should really notice their loss as I had documented in the release notes:

https://github.com/scala/scala-xml/blob/8008173/CHANGELOG.md

@SethTisue
Copy link
Member

Tentatively milestoned for 2.12.17, pending the outcome of this discussion. (2.12.18 is a possibility, too.)

@SethTisue SethTisue changed the title Consider bumping scala-xml to 2.x in Scala 2.12 Consider bumping scala-compiler's scala-xml dependency to 2.x in Scala 2.12 Aug 10, 2022
SethTisue added a commit to SethTisue/scala that referenced this issue Aug 10, 2022
SethTisue added a commit to SethTisue/scala that referenced this issue Aug 10, 2022
@SethTisue
Copy link
Member

SethTisue commented Aug 11, 2022

The (draft-for-now) upgrade in scala/scala went smoothly:

scala/scala#10108

though it did require one code change, since we were using a now-removed Elem constructor. It's difficult to guess whether others might hit the same issue.

And in the 2.12 community build:

• scala/community-build#1588

very smooth sailing — one repo had unused test-only code using scala.xml.pull, another repo had a test case that had an illegal XML comment that the new code rejects (correctly, afaict)

I've edited my long comment above so it now mentions the scala.xml.pull removal.

@SethTisue SethTisue self-assigned this Aug 15, 2022
@SethTisue
Copy link
Member

SethTisue commented Aug 15, 2022

it did require one code change, since we were using a now-removed Elem constructor. It's difficult to guess whether others might hit the same issue

The worst-case scenario I can imagine is that we discover after release that some popular library or popular sbt plugin (or a library used in a popular plugin) was using some removed bit of API (perhaps the Elem constructor, or scala.xml.pull), resulting in NoSuchMethodErrors when they get the upgrade because sbt 1.8.x gets the upgrade. If that library or plugin is still actively maintained, then it's not a big problem; the maintainers can put out a new version with the needed adjustments. If the library or plugin is abandoned, then doing something about it is harder, but not impossible (e.g. by forking).

Since sbt 1.x still has a long life ahead of it, I still think we should go ahead with this upgrade — I just want to lay out the nature of the risk we're taking.

@lrytz
Copy link
Member

lrytz commented Aug 16, 2022

I can't think of other concerns, but I'm also not very confident, it's difficult to predict the possible consequences. But reading through the notes here, I agree with the conclusion to go ahead.

@SethTisue
Copy link
Member

I did one further test, as follows, using the PR validation snapshot for the linked PR (commit e1a218e).

As expected, the POM at https://scala-ci.typesafe.com/native/scala-pr-validation-snapshots/org/scala-lang/scala-compiler/2.12.17-bin-e1a218e-SNAPSHOT/scala-compiler-2.12.17-bin-e1a218e-20220810.200117-1.pom has the scala-xml 2.1.0 dependency.

I grabbed the scala-compiler, scala-reflect, and scala-library JARs for e1a218e from scala-pr-validation-snapshots, and threw in Iterator.scala from the 2.12 standard library sources.

Then as a sanity check I verified that java -classpath scala-compiler-2.12.17-bin-e1a218e-20220810.200117-1.jar:scala-library-2.12.17-bin-e1a218e-20220810.200117-1.jar:scala-reflect-2.12.17-bin-e1a218e-20220810.200117-1.jar scala.tools.nsc.ScalaDoc -usejavacp Iterator.scala fails with java.lang.NoClassDefFoundError: scala/xml/MetaData, since we have no version of scala-xml at all on the classpath.

Then I added /Users/tisue/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/modules/scala-xml_2.12/2.1.0/scala-xml_2.12-2.1.0.jar to the classpath and the same thing succeeded.

So then finally the real test is, suppose we put the scala-xml 1.0.6 JAR on the classpath, does Scaladoc generation still succeed?

And the answer is: yes, It does.

So that shows that we have an additional level of safety here: even if someone somehow ends up with the 2.12.17 scala-compiler.jar on their classpath along with scala-xml 1.0.6, they should be fine, even if they want to generate Scaladoc. (Recall that Scaladoc generation is the only part of the 2.12 compiler where scala-xml is even used.)

Yes, in theory, there could be some problematic code path in Scaladoc generation that this test didn't happen to exercise, so the test isn't completely ironclad. Regardless, it's good additional confidence to have.

@SethTisue
Copy link
Member

I asked the community about this on the 2.12.17 planning thread at https://contributors.scala-lang.org/t/scala-2-12-17-release-planning/5805/3

If nothing new comes to light from that this week, then I think we can go ahead.

wjglerum added a commit to lunatech-labs/lunatech-lunch-planner that referenced this issue Aug 23, 2022
guizmaii added a commit to guizmaii/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
guizmaii added a commit to guizmaii/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
guizmaii added a commit to guizmaii/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
guizmaii added a commit to guizmaii/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
guizmaii added a commit to guizmaii/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
guizmaii added a commit to guizmaii/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
guizmaii added a commit to guizmaii/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
guizmaii added a commit to guizmaii/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
guizmaii added a commit to guizmaii/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
guizmaii added a commit to guizmaii/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
guizmaii added a commit to guizmaii/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
guizmaii added a commit to guizmaii/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
guizmaii added a commit to guizmaii/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
ghostdogpr pushed a commit to ghostdogpr/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
ghostdogpr pushed a commit to ghostdogpr/caliban that referenced this issue Oct 14, 2022
Fixes issues with scala-xml bincompat issues
See scala/bug#12632
ckipp01 added a commit to ckipp01/coursier that referenced this issue Oct 14, 2022
This is a continuation or really a revert of what was done in coursier#2274.
Originally this was done because Scala 2.12 was on 1.x, but that is no
longer the case. You can see the discussion for this in
scala/bug#12632. I'm assuming that other
things like sbt that will use this will also hit on it as well if it's
still on 1.x. See sbt/sbt#6997 for more
details on that. This also then bumps Scala 2.12 to 2.12.17 to make sure
the scala-xml is aligned there too.
ckipp01 added a commit to ckipp01/coursier that referenced this issue Oct 14, 2022
This is a continuation or really a revert of what was done in coursier#2274.
Originally this was done because Scala 2.12 was on 1.x, but that is no
longer the case. You can see the discussion for this in
scala/bug#12632. I'm assuming that other
things like sbt that will use this will also hit on it as well if it's
still on 1.x. See sbt/sbt#6997 for more
details on that. This also then bumps Scala 2.12 to 2.12.17 to make sure
the scala-xml is aligned there too.
alexarchambault pushed a commit to coursier/coursier that referenced this issue Oct 18, 2022
This is a continuation or really a revert of what was done in #2274.
Originally this was done because Scala 2.12 was on 1.x, but that is no
longer the case. You can see the discussion for this in
scala/bug#12632. I'm assuming that other
things like sbt that will use this will also hit on it as well if it's
still on 1.x. See sbt/sbt#6997 for more
details on that. This also then bumps Scala 2.12 to 2.12.17 to make sure
the scala-xml is aligned there too.
bpholt added a commit to typelevel/catbird that referenced this issue Oct 20, 2022
magro added a commit to scala-steward/solrs that referenced this issue Dec 27, 2022
The build failed with
```
[error] java.lang.RuntimeException: found version conflict(s) in library dependencies; some are suspected to be binary incompatible:
[error]
[error]         * org.scala-lang.modules:scala-xml_2.12:2.1.0 (early-semver) is selected over {1.0.6}
[error]             +- org.scoverage:scalac-scoverage-reporter_2.12:2.0.7 (depends on 2.1.0)
```

See also scala/bug#12632
@silviobierman
Copy link

I would like to upgrade from Scala 2.12.15 to 2.12.17 but we do unfortunately have a dependency on scala.xml.pull so we are stuck. Can the scala.xml.pull classes be taken from and older scala-xml version or are there incompatibilities that will prevent this from working?

@SethTisue
Copy link
Member

Can the scala.xml.pull classes be taken from and older scala-xml version or are there incompatibilities that will prevent this from working?

Good question. Let's discuss at scala/scala-xml#638 rather than here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants