Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Modularize: xml & parser-combinators #2855

Merged
merged 5 commits into from Sep 6, 2013

Conversation

Projects
None yet
3 participants
Owner

adriaanm commented Aug 19, 2013

[This PR depends on #2851]

These modules move to their own repositories:

The modularization depends on the new partest, as the old one's
classpath handling did not support a modularized scala.

The osgi tests take xml and parsers jars from the scaladoc dependencies,
and osgi-fy them, as they are no longer built locally.

Owner

adriaanm commented Aug 21, 2013

Retrying IDE validation after scala-ide/scala-ide#502 (comment) got merged. Still expect it to fail, just later. Working on fixing the build today.

Owner

adriaanm commented Aug 21, 2013

Ah right, forgot the sbinary and sbt build fixes are still only in my forks.

@adriaanm adriaanm referenced this pull request in scala-ide/scala-ide Aug 22, 2013

Merged

Fix/cross compiled modules 211 #502

Owner

adriaanm commented Aug 22, 2013

I guess this is not going to make it for M5. More details here scala-ide/scala-ide#502

Owner

adriaanm commented Aug 26, 2013

Well, it's alive!

To get the successful build linked to above, I currently need the hacks in the diffs below.
Most of them would disappear by switching to dbuild for sbt & sbinary.

The other changes I had to make were reasonable, so I submitted the following PRs:

Finally, as @dotta said, we'll have to fix the scala-maven plugin properly. Currently, it works because the compiler jars are on the boot classpath, and the right ones come first... I'll look into fixing that ASAP, but would immensely appreciate help.

adriaanm added some commits Jul 17, 2013

@adriaanm adriaanm Prepare removal of scala-xml, scala-parser-combinators
Every test deleted here has found its way to the respective
repositories of scala-xml and scala-parser-combinators,
where they will continue to be tested with partest.

The modified tests became independent of these modules,
as they should've been from the start.
9c50dd5
@adriaanm adriaanm Remove scala-xml and scala-parser-combinators
These modules move to their own repositories:
  - https://github.com/scala/scala-xml (v1.0-RC3)
  - https://github.com/scala/scala-parser-combinators (v1.0-RC1)

The modularization depends on the new partest, as the old one's
classpath handling did not support a modularized scala.

The compiler pom now depends on the artifacts published separately,
with versions specified in versions.properties.

NOTES:
  - The osgi tests resolve the xml and parsers jars and osgi-fy them,
    as they are no longer built locally.
    TODO: Can we move the osgification to the module builds?
  - Disabled local repositories: don't want to accidentally include
    unpublished artifacts in releases etc.
67600a7

@adriaanm adriaanm referenced this pull request in scala/jenkins-scripts Aug 28, 2013

Closed

Integration build supports modularized scala. #23

Why do we need sonatype repo instead of just relying on Maven central?

Nevermind. There's a comment below that says we don't want to wait for sync to happen. While I sympathize with that desire I think we should be cautious about adding additional repos. If we do that, then all downstream projects that build on top of Scala have to do that as well.

How often do we need to synchronize between repos that 24 hour delay is an issue?

Owner

adriaanm replied Aug 28, 2013

It adds another day before I can submit a PR that depends on these artifacts.
I thought everything from sonatype eventually synched to maven?

I think you are right the everything gets synched to Maven central.

The question is: how often do we need this capability and what's the risk we'll be breaking downstream projects that consume our snapshots and do not add that repo themselves (our repo declaration does not propagate transitively)?

How about we do this only for partest for now?

Nice! It looks like you became an Ant Pro 👍

Owner

adriaanm replied Aug 28, 2013

An awesome addition to my resume!

Member

gkossakowski commented Aug 28, 2013

I was wondering what's the status of this PR. I see the following steps that should be taken before we merge it:

  • review commits in this PR in isolation from IDE validation and make sure they make sense from scala repository point of view alone. I've done that and the only problem I found is adding Sonatype repo. I think we shouldn't rely on any other repos other than Maven central if possible
  • make sure that dependent projects like sbt and IDE are prepared for this PR to be merged

I think once we determine what to do with Sonatype repos in build.xml we'll be done with the first bullet.
@adriaanm: can you comment what's the status of the second bullet? From your comment above it seems like you fixed all downstream projects already but I wanted to make sure I interpret it correctly.

Owner

adriaanm commented Aug 28, 2013

Once scala/jenkins-scripts#23 is merged, PR validation will succeed for IDE integration.

@adriaanm adriaanm Don't use sonatype to resolve jars relevant to a release.
To eliminate any delay between deploying partest to sonatype
and using it for development, we resolve it through sonatype.

Not acceptable for jars that ship as part of the release,
such as scaladoc (we'd see a jar others might not see).
884bc78
Owner

adriaanm commented Aug 28, 2013

The other remaining TODO is to properly fix the scala maven plugin,
but I honestly don't think we should hold up this PR -- it was broken before,
and it works, just not in a long-term confidence-inspiring way.

@adriaanm adriaanm referenced this pull request in typesafehub/sbt-builds-for-ide Aug 29, 2013

Closed

Use Scala modules for 2.11 #3

Owner

adriaanm commented Aug 29, 2013

NOTE: the xml and parser modules are no longer included in the distribution.
I should probably change this.

Member

gkossakowski commented Aug 29, 2013

LGTM!

I assume this PR is still put on hold until other PRs are merged, right?

Owner

adriaanm commented Aug 29, 2013

It's waiting for

  • my commit that brings the jars back to the distribution,
  • for the jenkins-scripts PR to be merged.
@adriaanm adriaanm Include xml and parsers in dist, tool classpath.
This brings the external modules (xml, parsers)
back to the classpaths of build/(quick|pack)/bin/scala*.

Include the OSGIfied jars for external modules in dist.
(TODO: OSGI-fy externally)

Download javadoc/sources from maven and include in dist.
ce5c506

@gkossakowski gkossakowski was assigned Aug 30, 2013

Member

gkossakowski commented Sep 4, 2013

@adriaanm: what's the status of this PR?

Owner

adriaanm commented Sep 4, 2013

The sbt dbuild PR just got merged. Now I need to rework my PR to use dbuild.

Member

jsuereth commented Sep 6, 2013

note: Fixes needed are here: adriaanm#7

@jsuereth jsuereth referenced this pull request in typesafehub/sbt-builds-for-ide Sep 6, 2013

Merged

Fixes for scala 2.11 modularization #6

@jsuereth @adriaanm jsuereth Fix dbuild meta info: remove scaladoc project
That is, scaladoc is still in the scala-compiler artifact.
Let dbuild know so that it won't freak out.

ps: dbuild-meta.json should be kept in synch with
    src/build/dbuild-meta-json-gen.scala until we can
    automate that in the build
1515556
Owner

adriaanm commented Sep 6, 2013

@jsuereth I merged your changes from adriaanm#7

Member

jsuereth commented Sep 6, 2013

RAWK. This gets a LGTM from me. I tested by hand. I can't merge typesafehub/sbt-builds-for-ide#6 until this goes through and a 2.11.0-SNAPSHOT is publicly available with the change, so don't wait on that guy.

Owner

adriaanm commented Sep 6, 2013

Ok. I tested IDE validation locally as well. @gkossakowski, all systems merge!
The validation running right now on the last commit is irrelevant, the commit doesn't touch anything that's actually validated.

Owner

adriaanm commented Sep 6, 2013

@jsuereth, please accept some 🍰 as a token of my appreciation

Member

gkossakowski commented Sep 6, 2013

Thanks everybody.

Merging now.

@gkossakowski gkossakowski added a commit that referenced this pull request Sep 6, 2013

@gkossakowski gkossakowski Merge pull request #2855 from adriaanm/modularize-xml-parsers
Modularize: xml & parser-combinators
d6fe890

@gkossakowski gkossakowski merged commit d6fe890 into scala:master Sep 6, 2013

1 check was pending

default pr-scala started.
Details
Member

jsuereth commented Sep 6, 2013

AWESOME NEWS GUYS! Thanks for being patient with us. More than happy to help keep our stuff up to date :)

Also, glad to see this modularization progress.

@adriaanm adriaanm deleted the adriaanm:modularize-xml-parsers branch Dec 10, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment