Separate jars for XML, Parser Combinators. Use released JLine. #2704

Merged
merged 7 commits into from Jul 9, 2013

Conversation

Projects
None yet
7 participants
Owner

adriaanm commented Jul 4, 2013

The actual refactoring had already been done.
This changes the source directory layout and the produced jars.

Review by @gkossakowski

/cc @huitseeker, @harrah, @jsuereth

Member

soc commented Jul 4, 2013

Anyway, LVGTM!

gkossakowski was assigned Jul 4, 2013

@dragos dragos and 1 other commented on an outdated diff Jul 4, 2013

src/eclipse/reflect/.classpath
@@ -1,8 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<classpath>
<classpathentry kind="src" path="reflect"/>
- <classpathentry combineaccessrules="false" kind="src" path="/scala-library"/>
@dragos

dragos Jul 4, 2013

Contributor

In this way the classpath won't contain the library that was just built. Instead it will use the Eclipse built-in one, and maybe you'll need changes in both at the same time (reflect and library are still tightly coupled). Just something to keep in mind.

@adriaanm

adriaanm Jul 4, 2013

Owner

Good point, this was a mistake.

@adriaanm

adriaanm Jul 4, 2013

Owner

Reflect should be built against the library. The projects currently built during quick but not during locker should be built using the Scala version in the plugin.

Member

soc commented Jul 4, 2013

I have no clue about the Eclipse stuff, but the rest LGTM.

Contributor

harrah commented Jul 4, 2013

This LGTM. We'd need coordination for nightly builds against 2.11 if we aren't ok with transient breakage in projects that use xml or parser combinators until builds are updated.

Owner

adriaanm commented Jul 4, 2013

@harrah, thanks. Yes, I'd like to minimize that breakage, but need to merge this into M4, which we're releasing next week. Do you have some time next week to update your build? /cc @dragos, @huitseeker for IDE

Owner

adriaanm commented Jul 5, 2013

/cc @cunei for caa3989

Member

soc commented Jul 5, 2013

Weird. Looks like it can't find Profiler.java, although it's supposed to be in the same package.

Contributor

harrah commented Jul 5, 2013

@adriaanm yes. The changes should be straightforward, but require switching on the Scala version. So, once they are added, the build won't work on older 2.11 versions. I'm just going to look at >=2.11.x vs <= 2.10.x and not try to get the exact nightly. I assume that is all ok.

Owner

adriaanm commented Jul 5, 2013

@harrah, that's fine with me -- I'm not hearing anything from the IDE team about his PR, so I'll double check with them.

Owner

adriaanm commented Jul 5, 2013

Weird. Looks like it can't find Profiler.java, although it's supposed to be in the same package.

It's because java and scala are compiled separately and partest only saw it's own class files through its dependency on scalap. Fixed now.

Member

gkossakowski commented Jul 5, 2013

I reviewed all commits pushed so far. Great work!

I think there are two outstanding (small) tasks:

  • improved commit message in fa31d3c that explains why it's ok to get rid of migration code before M4 is released
  • determine whether $ is missing in 2391c20

adriaanm added some commits Jul 4, 2013

@adriaanm adriaanm No more duplication in maven-deploy.xml.
I just couldn't stand the incredible mess in there anymore. More cleanup to come.
For now, suffice it to say you need only add one line per new module.
b672009
@adriaanm adriaanm Spin off src/library/scala/xml to src/xml/scala/xml.
Summary:
  - Remove the last vestiges of xml from Predef and Contexts.
  - Change build to compile scala.xml to scala-xml.jar.
  - Deploy scala-xml module to maven.
  - Update partest accordingly.

Note:
An older compiler cannot use the new standard library to
compile projects that use XML. Thus, skipping locker will
break the build until we use 2.11.0-M4 for STARR.

In the future build process, where we drop locker,
we would have to release a milestone that supports the old
and the new approach to xml. As soon as we'd be using that
new milestone for starr, we could drop support for the old approach.
4340f79
@adriaanm adriaanm Spin off parser combinators to scala-parser-combinators.jar. 46a4635
@adriaanm adriaanm Unfork jline: use vanilla jline 2.11 as a dependency.
Notes:
  - no longer specifying terminal by class name in scripts (using 'unix')
  - jline doesn't need a separate jansi dependency;
    it includes its own version according to:
    http://mvnrepository.com/artifact/jline/jline/2.11
1b0fa91
@adriaanm adriaanm Add meta-information for dbuild.
The next version of [dbuild](http://typesafehub.github.io/distributed-build/0.5.3/index.html)
will parse `dbuild-meta.json` to determine which jars are produced by a Scala build.

This way we can modularize without changing dbuild itself.

Yes, I know `dbuild-meta.json` should be generated during the build.
However, given the state of our build.xml, I think this is pointless.

My goal is to generate build.xml, dbuild-meta.json and Eclipse projects
from a higher-level description of our build. Baby steps...

Including improvements by @cunei:
  - Removing outdated field "uri" from ExtractedBuildMeta
  - Changed "partest" to "scala-partest" (the actual jar name)
a0a60e7
@adriaanm adriaanm Updated eclipse project files.
Set everything up so that soon-to-be independent modules are
compiled with the Eclipse plugin's compiler & library (2.11.0-M3 currently).

Most projects still compile with 2.11.0-M3, but
partest will need a nightly build of 2.11.
57534f9
Owner

adriaanm commented Jul 5, 2013

Rebased. Thanks for the reviews everyone!

Member

gkossakowski commented Jul 7, 2013

LGTM.

Great work! We are celebrating tomorrow with 🍕 and 🍸 🍸. :-)

Owner

adriaanm commented Jul 7, 2013

Yay! Note that this breaks the eclipse build as SBinary apparently uses xml, so it'll need scala-xml.jar on its classpath

Contributor

harrah commented Jul 8, 2013

It is a Format instance for XML that can be dropped for sbinary 0.5.0 to keep its dependencies to just scala-library.

@adriaanm adriaanm scaladoc needs xml and parser-combinators
Concretely, update scala-compiler's pom
to list scala-xml and scala-parser-combinators
on behalf of scaladoc.

NOTE: when spinning off scaladoc, move dependencies to its own pom
a07879d
Owner

adriaanm commented Jul 9, 2013

To get SBinary to compile: adriaanm/sbinary@7ce59f7)
SBT: adriaanm/sbt@de3664b

Now only scala-refactoring test failures remain, as some tests depend on parsers and xml
https://scala-webapps.epfl.ch/jenkins/job/pr-scala-integrate-ide/344/console

adriaanm referenced this pull request in scala-ide/scala-refactoring Jul 9, 2013

Merged

Put jars for scala.xml and scala.util.parsing on classpath for tests #22

@adriaanm adriaanm added a commit that referenced this pull request Jul 9, 2013

@adriaanm adriaanm Merge pull request #2704 from adriaanm/modularize
Separate jars for XML, Parser Combinators. Use released JLine.
4143189

@adriaanm adriaanm merged commit 4143189 into scala:master Jul 9, 2013

1 check passed

default pr-checkin-per-commit Took 43 min.
Details

OlegYch commented Jul 16, 2013

note that new jline breaks repl on windows sbt/sbt#763

@dotta dotta pushed a commit to dotta/scala-refactoring that referenced this pull request Dec 12, 2013

@adriaanm adriaanm Put jars for s.xml and s.u.parsing on classpath
As of Scala 2.11.0-M4, scala.xml and scala.util.parsing are
in [separate jars](scala/scala#2704).

Improve `CompilerProvider` to deal with this in a backwards compatible way.
5ec8519

@lrytz lrytz added a commit to lrytz/scala that referenced this pull request Jun 9, 2015

@lrytz lrytz Use the scala-jline fork
The scala compiler depending on a jline release pollutes the classpath
of projects that embed the scala compiler or REPL, such as the spark
REPL. The scala-jline fork is identical to official jline, but with
the package name changed to scala.tools.jline.

For reference, the PR that removed jline from the scala repository
and introduced stock jline was #2704.
5cf043b

@lrytz lrytz added a commit to lrytz/scala that referenced this pull request Jun 9, 2015

@lrytz lrytz Use the scala-jline fork
The scala compiler depending on a jline release pollutes the classpath
of projects that embed the scala compiler or REPL, such as the spark
REPL. The scala-jline fork is identical to official jline, but with
the package name changed to scala.tools.jline.

For reference, the PR that removed jline from the scala repository
and introduced stock jline was #2704.
b8d1ce2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment