Drop scala-xml as a dependency of compiler and REPL #6436

adriaanm · 2018-03-16T19:26:02Z

To simplify boostrapping, we're going NIH on scala-xml.
Next step: remove usage in diagram generation, or maybe
move that to a d3 / dot JS client-side solution.

The diff on scaladoc for the core is minimal, using
a textconv à la:

cat "$1" | perl -pe 's/\s+/ /gm' | perl -pe 's/ef26caa/60fce92/g' | \
  tidy -q --sort-attributes alpha --wrap 0 -i -utf8  -ashtml - 2> /dev/null

(shas are part of locally published version)

The html diff in its entirety:

diff --git i/library/scala/concurrent/ExecutionContext.html w/library/scala/concurrent/ExecutionContext.html
index 369ed46..061c49e 100644
--- i/library/scala/concurrent/ExecutionContext.html
+++ w/library/scala/concurrent/ExecutionContext.html
@@ -247,7 +247,7 @@
           </div>
           <dl class="attributes block">
             <dt>Annotations</dt>
-            <dd><span class="name">@<a class="extype" href="../annotation/implicitNotFound.html" id="scala.annotation.implicitNotFound" name="scala.annotation.implicitNotFound">implicitNotFound</a></span><span class="args">( <span>msg = <span class="defval" name="&quot;&quot;&quot;Cannot find an implicit ExecutionContext. You might pass&lt;br/&gt;an (implicit ec: ExecutionContext) parameter to your method&lt;br/&gt;or import scala.concurrent.ExecutionContext.Implicits.global.&quot;&quot;&quot;">...</span></span> )</span></dd>
+            <dd><span class="name">@<a class="extype" href="../annotation/implicitNotFound.html" id="scala.annotation.implicitNotFound" name="scala.annotation.implicitNotFound">implicitNotFound</a></span><span class="args">(<span>msg = <span class="defval" name="&quot;&quot;&quot;CannotfindanimplicitExecutionContext.Youmightpassan(implicitec:ExecutionContext)parametertoyourmethodorimportscala.concurrent.ExecutionContext.Implicits.global.&quot;&quot;&quot;">...</span></span>)</span></dd>
             <dt>Source</dt>
             <dd>
               <a href="https://github.com/scala/scala/tree/v2.13.0-local-60fce92/src/library/scala/concurrent/ExecutionContext.scala#L1" target="_blank">ExecutionContext.scala</a>
diff --git i/library/scala/concurrent/index.html w/library/scala/concurrent/index.html
index 8f1ca87..1d0e5bc 100644
--- i/library/scala/concurrent/index.html
+++ w/library/scala/concurrent/index.html
@@ -552,7 +552,7 @@
                     </div>
                     <dl class="attributes block">
                       <dt>Annotations</dt>
-                      <dd><span class="name">@<a class="extype" href="../annotation/implicitNotFound.html" id="scala.annotation.implicitNotFound" name="scala.annotation.implicitNotFound">implicitNotFound</a></span><span class="args">( <span>msg = <span class="defval" name="&quot;&quot;&quot;Cannot find an implicit ExecutionContext. You might pass&lt;br/&gt;an (implicit ec: ExecutionContext) parameter to your method&lt;br/&gt;or import scala.concurrent.ExecutionContext.Implicits.global.&quot;&quot;&quot;">...</span></span> )</span></dd>
+                      <dd><span class="name">@<a class="extype" href="../annotation/implicitNotFound.html" id="scala.annotation.implicitNotFound" name="scala.annotation.implicitNotFound">implicitNotFound</a></span><span class="args">(<span>msg = <span class="defval" name="&quot;&quot;&quot;CannotfindanimplicitExecutionContext.Youmightpassan(implicitec:ExecutionContext)parametertoyourmethodorimportscala.concurrent.ExecutionContext.Implicits.global.&quot;&quot;&quot;">...</span></span>)</span></dd>
                     </dl>
                   </div>
                 </li>

adriaanm · 2018-03-16T21:11:42Z

Needs some test suite updating, but already worth a review pass.

adriaanm · 2018-03-17T10:13:35Z

Note that that diff is irrelevant, as it shows the old version put the text that should show on the page into the name attribute. I have a separate commit to fix this.

SethTisue · 2018-03-17T14:03:36Z

Cc @ashawley

adriaanm · 2018-03-17T14:32:25Z

I'm now working on using https://github.com/dagrejs for diagrams. The integration is pretty easy, but I've no clue on how to arrange the proper CSS :-) It generates diagrams from DOT files in the browser, which right now we're doing by translating them to SVG while generating scaladoc (or not, since it's buggy).

ashawley · 2018-03-17T15:50:55Z

I was taking a similar NIH route. Looking good!

adriaanm · 2018-03-17T17:50:46Z

Thanks! The diagram work is ongoing here: https://github.com/adriaanm/scala/compare/xmlnininih...scaladoc-diags?expand=1

adriaanm · 2018-03-17T20:03:43Z

After playing around with dagre.js, I think this has a lot of potential, but I'm not sure I'm going to be able to figure out how to tweak the DOT generation and the CSS... Hopefully someone can pick this up.

Current output:

adriaanm · 2018-03-19T23:46:11Z

making some progress on diags front:

adriaanm · 2018-03-20T16:17:08Z

This needs the scalacheck fixes in #6440. Did a minimal fix already

ashawley · 2018-03-27T16:34:07Z

Dropping scala-xml from the compiler is no longer necessary in sbt 1.1.2 and later, FWIW, see sbt/sbt#3405.

Hopefully, the diagram work will continue on in some form.

SethTisue · 2018-03-27T16:40:32Z

Dropping scala-xml from the compiler is no longer necessary

(note that there is no shortage of other reasons to want to do this, it will simplify our bootstrapping and publishing)

ashawley · 2018-03-27T17:23:25Z

note that there is no shortage of other reasons to want to do this

ok, well that should be laid out in a new scala-dev issue?

SethTisue · 2018-03-27T17:28:30Z

ok, well that should be laid out in a new scala-dev issue?

we've still got scala/bug#9560

adriaanm · 2018-03-30T19:51:21Z

I've taken the diagram port as far as I could. Nodes link to the corresponding pages, but some other fancy JS/CSS stuff is no longer supported. I think it looks pretty good. Give it a whirl!

I think this undoes our xml knot. We do have some dependencies in the build, but it looks like most of them could be dropped. /cc @szeiger (for the obsolete ant compatibility parts re: pom files). There's also the intellij project gen -- could simple string replacement suffice there?

SethTisue · 2018-03-30T20:59:33Z

We do have some dependencies in the build, but it looks like most of them could be dropped

note that @ashawley has investigated this at #6480 (with encouraging results)

ashawley · 2018-03-31T12:20:51Z

(for the obsolete ant compatibility parts re: pom files). There's also the intellij project gen

Aren't these are in stb-land and not the build so sbt provides scala-xml bindings?

ashawley · 2018-04-25T19:08:05Z

I tried to research if the scala-xml dependency in partest is even necessary in scala/scala-partest#104

lrytz · 2018-04-25T19:29:31Z

Thanks @ashawley! In the interest of getting our build green, we should probably revert 5a582ce for now.

We might still have some test cases in the scala/scala repo that need the xml library to be around. Those should be moved to scala/scala-xml. Let's do this work first (I put it on my list), then remove xml from the build when it's all done.

SethTisue · 2018-04-25T20:02:54Z

We might still have some test cases in the scala/scala repo that need the xml library to be around. Those should be moved to scala/scala-xml. Let's do this work first (I put it on my list), then remove xml from the build when it's all done.

I think we (Aaron, Adriaan, and I) already agreed to just remove any such test cases here on the assumption that they'd be re-added to scala-xml, there's no need to hold things up here in scala/scala over it, as far as I can see. Can't we just rip the scala-xml dependency out of partest? M4 isn't imminent.

SethTisue · 2018-04-25T20:03:36Z

I'm available Thursday & Friday to work on it.

lrytz · 2018-04-25T20:52:38Z

That works for me, what ever causes the least friction. I personally think getting the build green first would be a good thing though, it removes stress.

SethTisue · 2018-04-26T09:29:06Z

I understand now that I could have tested the bootstrap script changes first, before merging the PR, by pushing them to a branch here in in scala/scala, instead of my own fork, so that Travis-CI would notice them.

Anyway, regardless, I'm testing a bootstrap fix now, it seems like it shouldn't be that hard to fix without having to actually revert anything... knock on wood...

lrytz · 2018-04-26T09:32:26Z

👍 thanks!

SethTisue · 2018-04-26T10:29:03Z

bootstrap fix is #6563

SethTisue · 2018-04-26T11:31:01Z

I've added "and REPL" to the PR name since a not-obvious consequence of this is that XML literal syntax no longer works in the Scala REPL unless you explicitly put scala-xml on the classpath.

2.13.0-M3

Welcome to Scala 2.13.0-M3 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_172).

scala> <foo/>
res0: scala.xml.Elem = <foo/>

2.13.x now

% scala
Welcome to Scala 2.13.0-20180426-110853-91a7d72 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_172).

scala> <foo/>
       ^
       error: To compile XML syntax, the scala.xml package must be on the classpath.
       Please see https://github.com/scala/scala-xml for details.

% scala -classpath /Users/tisue/.ivy2/cache/org.scala-lang.modules/scala-xml_2.13.0-M4-pre-20d3c21/bundles/scala-xml_2.13.0-M4-pre-20d3c21-1.1.0-newCollectionsBootstrap.jar
Welcome to Scala 2.13.0-20180426-110853-91a7d72 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_172).

scala> <foo/>
res0: scala.xml.Elem = <foo/>

SethTisue · 2018-04-26T11:41:59Z

now on my short-term to-do list: pushing this change through the scala-dist and scala-dist-smoketest repos.

also, new scala-xml ticket on helping confused users wondering where scala-xml went: scala/scala-xml#212

SethTisue · 2018-04-26T13:37:30Z

followup PRs: scala/scala-dist#187, scala/scala-dist-smoketest#20, scala/community-build#713

SethTisue · 2018-04-27T16:47:14Z

meanwhile in the land of Dotty, scala/scala3#4394

som-snytt · 2018-10-10T01:04:37Z

How am I supposed to put scala-xml on the classpath? Or was this supposed to encourage a contribution of magic import to REPL?

$ ~/scala-2.13.0-M5/bin/scalac -Yrangepos -Yvalidate-pos:typer RangePosFail.scala
RangePosFail.scala:5: error: To compile XML syntax, the scala.xml package must be on the classpath.
Please see https://github.com/scala/scala-xml for details.
      <iframe/> // Positions$ValidateException: Enclosing tree [53] does not include tree [52]
      ^
one error found

adriaanm · 2018-10-10T08:01:16Z

Yes to the latter. I'm more of a strategic thinker than one might think.

scala-jenkins added this to the 2.13.0-M4 milestone Mar 16, 2018

adriaanm requested a review from SethTisue March 16, 2018 21:11

ashawley mentioned this pull request Mar 19, 2018

make scala-xml a provided dependency for scala-compiler, remove it from scaladoc? scala/scala-dev#291

Closed

adriaanm force-pushed the xmlnininih branch 2 times, most recently from df76664 to 320ea77 Compare March 20, 2018 16:13

ashawley mentioned this pull request Mar 29, 2018

Try purging scala-xml #6480

Closed

adriaanm force-pushed the xmlnininih branch from fb5bad2 to cb4fa76 Compare March 30, 2018 19:47

adriaanm changed the title ~~Drop scala-xml from scaladoc pt. 1~~ Drop scala-xml from scaladoc Mar 30, 2018

adriaanm force-pushed the xmlnininih branch 2 times, most recently from 15ac6e5 to 2531e33 Compare April 1, 2018 09:06

SethTisue mentioned this pull request Apr 25, 2018

port removed test cases from scala/scala repo scala/scala-xml#211

Closed

SethTisue force-pushed the xmlnininih branch 3 times, most recently from 9c965b1 to a5fb71e Compare April 25, 2018 13:01

ashawley mentioned this pull request Apr 25, 2018

Drop scala-xml dependency? scala/scala-partest#104

Closed

SethTisue mentioned this pull request Apr 26, 2018

changes for 2.13 compat scala/scala-partest#106

Merged

SethTisue changed the title ~~Drop scala-xml as a dependency of scala-compiler~~ Drop scala-xml as a dependency of scala-compiler and REPL Apr 26, 2018

SethTisue changed the title ~~Drop scala-xml as a dependency of scala-compiler and REPL~~ Drop scala-xml as a dependency of compiler and REPL Apr 26, 2018

SethTisue mentioned this pull request Apr 26, 2018

in the readme, provide 2.13 users with appropriate help adding the dependency? scala/scala-xml#212

Closed

This was referenced Apr 26, 2018

drop support for "assemble" build system? lightbend-labs/dbuild#208

Closed

scala-xml is no longer bundled scala/scala-dist#187

Merged

scala-xml is no longer bundled scala/scala-dist-smoketest#20

Merged

ashawley mentioned this pull request Apr 27, 2018

Add XML tests to partest #6569

Merged

SethTisue mentioned this pull request May 12, 2018

GraphViz errors in release-main scala/scala-jenkins-infra#190

Closed

Philippus mentioned this pull request Aug 26, 2018

Scaladoc generation can create invalid HTML. scala/bug#8073

Closed

ashawley mentioned this pull request Aug 30, 2018

Serialization in 2.13.0-M5 scala/scala-xml#254

Closed

som-snytt mentioned this pull request Oct 10, 2018

Avoid invalid range positions for XML sequences with CRLF line endings #7327

Closed

SethTisue mentioned this pull request Dec 2, 2018

Ant and XML utilities are missing in 2.13.0-M5 scala/bug#11286

Closed

ashawley mentioned this pull request Aug 22, 2019

remove unused JavaScript dependencies and licenses #8360

Merged

nosrednd23 mentioned this pull request Oct 22, 2023

To simplify boostrapping, we're going NIH on scala-xml. nosrednd23/LaDriere-fraud-malicious-intent-#1

Open

Drop scala-xml as a dependency of compiler and REPL #6436

Drop scala-xml as a dependency of compiler and REPL #6436

Uh oh!

Conversation

adriaanm commented Mar 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adriaanm commented Mar 16, 2018

Uh oh!

adriaanm commented Mar 17, 2018

Uh oh!

SethTisue commented Mar 17, 2018

Uh oh!

adriaanm commented Mar 17, 2018

Uh oh!

ashawley commented Mar 17, 2018

Uh oh!

adriaanm commented Mar 17, 2018

Uh oh!

adriaanm commented Mar 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adriaanm commented Mar 19, 2018

Uh oh!

adriaanm commented Mar 20, 2018

Uh oh!

ashawley commented Mar 27, 2018

Uh oh!

SethTisue commented Mar 27, 2018

Uh oh!

ashawley commented Mar 27, 2018

Uh oh!

SethTisue commented Mar 27, 2018

Uh oh!

adriaanm commented Mar 30, 2018

Uh oh!

SethTisue commented Mar 30, 2018

Uh oh!

ashawley commented Mar 31, 2018

Uh oh!

ashawley commented Apr 25, 2018

Uh oh!

lrytz commented Apr 25, 2018

Uh oh!

SethTisue commented Apr 25, 2018

Uh oh!

SethTisue commented Apr 25, 2018

Uh oh!

lrytz commented Apr 25, 2018

Uh oh!

SethTisue commented Apr 26, 2018

Uh oh!

lrytz commented Apr 26, 2018

Uh oh!

SethTisue commented Apr 26, 2018

Uh oh!

SethTisue commented Apr 26, 2018

2.13.0-M3

2.13.x now

Uh oh!

SethTisue commented Apr 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SethTisue commented Apr 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SethTisue commented Apr 27, 2018

Uh oh!

som-snytt commented Oct 10, 2018

Uh oh!

adriaanm commented Oct 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

adriaanm commented Mar 16, 2018 •

edited

Loading

adriaanm commented Mar 17, 2018 •

edited

Loading

SethTisue commented Apr 26, 2018 •

edited

Loading

SethTisue commented Apr 26, 2018 •

edited

Loading