Skip to content
This repository has been archived by the owner on Feb 23, 2018. It is now read-only.

Integration build supports modularized scala. #23

Closed
wants to merge 6 commits into from

Conversation

adriaanm
Copy link

After this is merged, scala/scala#2855 should validate for IDE integration.
I also cleaned up the build script a bit.
The main change is:

+      "set (libraryDependencies in cacheSub         ) ~= { ld => ld map { case dep if (dep.organization == \"org.scala-lang.modules\") => dep cross CrossVersion.fullMapped(_ => \"$SCALA_BINARY_VERSION\") case dep => dep } }"\
+      "set (libraryDependencies in compilePersistSub) ~= { ld => ld map { case dep if (dep.organization == \"org.scala-tools.sbinary\") && (dep.name == \"sbinary\") => dep.copy(revision = (dep.revision + \"-SNAPSHOT\")) case dep if (dep.organization == \"org.scala-lang.modules\") => dep cross CrossVersion.fullMapped(_ => \"$SCALA_BINARY_VERSION\") case dep => dep } }"\
+      "set (libraryDependencies in mainSub          ) ~= { ld => ld map { case dep if (dep.organization == \"org.scala-lang.modules\") => dep cross CrossVersion.fullMapped(_ => \"$SCALA_BINARY_VERSION\") case dep => dep } }"\
+      "set (libraryDependencies in processSub       ) ~= { ld => ld map { case dep if (dep.organization == \"org.scala-lang.modules\") => dep cross CrossVersion.fullMapped(_ => \"$SCALA_BINARY_VERSION\") case dep => dep } }"\

I tested these changes manually against scala/scala#2855 (master) and scala/scala#2876 (2.10.x).
Log files here: https://gist.github.com/adriaanm/6361216
(the result against #2855 was using a slightly older commit, but very little has changed since in the PR)

review by @dragos

Show scala-instance after all relevant settings are set
Overall, the integration script should do less detecting and more setting.
Let's refactor all the builds involved so that this script becomes much,
much, much simpler. How about introducing variables in the builds wherever
necessary, instead of these elaborate sequences of sbt commands,
and maverick search/replace missions.
@@ -14,7 +14,7 @@ BASEDIR="$(pwd)"
# keep them local to the workspace also lets us diagnose problems more easily
# Make sure this is an absolute path with preceding '/'
LOCAL_M2_REPO="$BASEDIR/m2repo"
GENMVNOPTS="-e -B -X -Dmaven.repo.local=${LOCAL_M2_REPO}"
GENMVNOPTS="-e -B -Dmaven.repo.local=${LOCAL_M2_REPO}"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it's a lot of noise, but I wonder if we'll be able to track down issues like the one in scalariform otherwise..

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reality is that, when it fails on jenkins, you'll have to reproduce it locally to diagnose anyway. The scripts are designed so that you can run them locally without problems. It'd be trivial to write a script to download a build's workspace and re-run the script to see what happened. I agree we might need the switch to diagnose problems with the jenkins setup, but we can always turn it on when necessary.
In the mean time, i'd say shorter logs are better because you can better spot problems that the script misses erroneously.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory yes, but in practice, at least in the past two weeks, most failures were not reproducible. For instance, concurrent builds, and more recently scalastyle (due to network errors). I don't feel strongly about it though, and my first reaction was to remove -X as well, but I was happy to find that info in my last "debugging" sessions.

This comes up when running the build locally.
Better not to depend on jenkins variables, anyway.
This file captures the scala binary version being used
to (e.g.) resolve dependencies. We read it to do selective rewiring.
@adriaanm
Copy link
Author

@dragos, I dropped the "quiet the maven commit". Could I get an LGTM now?

@dragos
Copy link

dragos commented Aug 29, 2013

LGTM. As I said, I don't feel strongly about -X, you can remove it if you prefer.

@adriaanm
Copy link
Author

Once #24 is merged (Monday?), I'll rebase this (on Tuesday), in time for M5 closure on Thursday.

@adriaanm
Copy link
Author

adriaanm commented Sep 4, 2013

I'll need to port these changes to https://github.com/typesafehub/sbt-builds-for-ide/blob/master/sbt-on-2.11.x

@adriaanm adriaanm closed this Sep 4, 2013
adriaanm referenced this pull request in adriaanm/sbt-builds-for-ide Sep 5, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants