Skip to content

Scaladoc has unnecessary dependencies on scala-parser-combinators and scala-xml #9560

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

Closed
scabug opened this issue Nov 17, 2015 · 13 comments
Closed
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented Nov 17, 2015

  • Scaladoc doesn't need to parse JSON, only generate it, and it's silly to depend on scala.util.parsing for that, especially since it only happens in one small place so the dependency would be easy to cut
  • similarly for XML, except cutting the dependency is harder because we're generating HTML throughout the code. but but there's no truly fundamental difficulty

in both cases we could avoid replacing one Scala dependency with another Scala dependency by just using plain Scala and/or by adding a Java dependency.

main benefit is reduced complexity in the build.

see discussion at scala/slip#24 (comment) and https://gitter.im/scala/slip?at=564a3a2e8b242470793e3035 (for HTML generation, Haoyi thought "vendoring a small 1000loc subset of scalatags" was plausible)

also I discussed this with retronym just now and:

  • he doesn't recall there being any special reason that the parser-combinators dependency was left in place
  • if they'd realized in advance how much work setting up the necessary "build gymnastics" for the circular dependencies would be, they might have considered other approaches, and just because those gymnastics now exist isn't a good reason to keep using them unless really needed
  • he says there might be a place in a non-Scaladoc portion of the compiler where there's another not-really-needed dependency on scala-xml; if so, that could be taken care of at the same time
@scabug
Copy link
Author

scabug commented Nov 17, 2015

Imported From: https://issues.scala-lang.org/browse/SI-9560?orig=1
Reporter: @SethTisue
See #8358

@scabug
Copy link
Author

scabug commented Nov 17, 2015

@retronym said:
Looks like @adriaanm already removed the use of scala-xml in parsing compiler plugin descriptors as part of scala/scala#2667

@scabug
Copy link
Author

scabug commented Nov 18, 2015

@SethTisue said:
related: #8358

@scabug
Copy link
Author

scabug commented Nov 19, 2015

@soc said:
Eliminated parser-combinators: scala/scala#4851

@scabug
Copy link
Author

scabug commented Nov 23, 2015

@lrytz said:
once library/reflect/compiler have no more dependencies, we should get everything related to modules out of the scala/scala repo. the current build still resolves all modules, probably for building scala-library-all?

@scabug
Copy link
Author

scabug commented Jul 25, 2016

@szeiger said:
scala-library-all resolves the dependencies to add them to the POM / ivy.xml but it should be possible to fake that and generate the correct descriptor without resolving the artifacts.

@scabug
Copy link
Author

scabug commented Feb 23, 2017

@dwijnand said:
/r/therewasanattempt at scala/scala#5689

@scabug
Copy link
Author

scabug commented Feb 23, 2017

@SethTisue said:
5689 is about mitigating the effect of the dependency's existence on downstream users of scala-xml, addressing scala/scala-dev#291, but doesn't remove the dependency entirely. if we can remove the dependency entirely, then 291 goes away and 5689 won't be needed.

@ashawley
Copy link
Member

similarly for XML, except cutting the dependency is harder because we're generating HTML throughout the code.

So presumably the HTML bit is because of Scaladoc.

there might be a place in a non-Scaladoc portion of the compiler where there's another not-really-needed dependency on scala-xml; if so, that could be taken care of at the same time

Is this talking about XML literals or is there a third situation?

@SethTisue
Copy link
Member

So presumably the HTML bit is because of Scaladoc

yup

Is this talking about XML literals or is there a third situation?

I'm not sure what Jason was thinking when he said that. We can cross that bridge when we come to it, if there's even a river left to cross.

@SethTisue SethTisue modified the milestones: Backlog, 2.13.0-RC1 Mar 27, 2018
@SethTisue
Copy link
Member

scala-library-all resolves the dependencies to add them to the POM / ivy.xml

scala-library-all no longer exists in 2.13, so that's one less thing we need to worry about here

@ashawley
Copy link
Member

It would seem the compiler could still be built without a scala-xml dependency and preserve XML literal support, but I suppose the issue is being able to rip out the dependency from the compiler test suite, which see scala/scala@5b532e9 is a commit from Adriaan from Nov-14 2013 that put back the tests for scala-xml back when the community module work was done.

It looks like the scala compiler junit test suite doesn't seem to need scala-xml as a dependency. Presumably, this is because those junit tests were able to be moved to the scala-xml repo.

@SethTisue
Copy link
Member

scala/scala#6436 has finished this off.

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

5 participants