Skip to content
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

upgrade to Scala 2.12 #4574

Merged
merged 11 commits into from
Jul 16, 2019
Merged

upgrade to Scala 2.12 #4574

merged 11 commits into from
Jul 16, 2019

Conversation

koertkuipers
Copy link
Contributor

hello!

spark 2.4 will be last spark release to support scala 2.11 and is also available for scala 2.12

flink 1.7 is available for scala 2.11 and scala 2.12. i am not familiar enough with flink or its community to say when they will drop scala 2.11

this pullrequest builds the jvm libs using scala 2.12. to make the switch i had to change java version to 1.8 (required for scala 2.12) and upgrade flink and akka.

to be able to publish artifacts for both scala 2.11 and 2.12 (scala is not compatible across minor versions) i modified the artifacts to have scala version in the name. its not pretty :( but it seems to be the convention.

to publish artifacts for scala 2.11 one just has to make a few small modifications to pom files. this can probably easily be automated with a bash script. e.g. jvm-packages/dev/change_scala_version.sh. i am not aware of an approach with maven where both scala 2.11 and scala 2.12 artifacs can be created more elegantly.

@CodingCat
Copy link
Member

isn't this PR dropping the support of 2.11 instead of support both 2.11 and 2.12?

@koertkuipers
Copy link
Contributor Author

isn't this PR dropping the support of 2.11 instead of support both 2.11 and 2.12?

i made sure to select libraries and syntax that work well for both scala 2.11 and 2.12.

maven doesnt support building artifacts for two scala versions. but artifacts for scala 2.11 can be build and tested by making a few very small changes to the pom file. this could be automated in a bash script if desired.

i think the same probably applies to the automated testing infrastructure, although it will be a little more work. for example appveyor.yml and tests/ci_build/Dockerfile.jvm_cross would have to be modified dynamically. i assume/hope environment variables can be used for this.

@CodingCat
Copy link
Member

isn't this PR dropping the support of 2.11 instead of support both 2.11 and 2.12?

i made sure to select libraries and syntax that work well for both scala 2.11 and 2.12.

maven doesnt support building artifacts for two scala versions. but artifacts for scala 2.11 can be build and tested by making a few very small changes to the pom file. this could be automated in a bash script if desired.

i think the same probably applies to the automated testing infrastructure, although it will be a little more work. for example appveyor.yml and tests/ci_build/Dockerfile.jvm_cross would have to be modified dynamically. i assume/hope environment variables can be used for this.

it's fine, we can safely just support 2.12 as we are moving to the next 1.0 release

@mhlr
Copy link

mhlr commented Jun 30, 2019

What is the time frame for a release containing this?

@hcho3
Copy link
Collaborator

hcho3 commented Jul 16, 2019

@dre-hh FYI, we are working to compile XGBoost with Scala 2.12.

@CodingCat CodingCat changed the title Support scala 2.11 and scala 2.12 upgrade to Scala 2.12 Jul 16, 2019
@CodingCat
Copy link
Member

as 2.12 would be our major goal, I don't think we still need to change the artifact name,

what do you think @koertkuipers

@koertkuipers
Copy link
Contributor Author

as 2.12 would be our major goal, I don't think we still need to change the artifact name,

what do you think @koertkuipers

i prefer to change artifact name to have scala version in it (as ugly as it is...)

folks might want to cross-build for 2.11 and 2.12 (or for 2.12 and 2.13 at some point for spark 3). for them life will be easier if you have scala version in artifact name.

@CodingCat
Copy link
Member

make sense

@CodingCat CodingCat merged commit 3c506b0 into dmlc:master Jul 16, 2019
@CodingCat
Copy link
Member

merged, thanks

@hcho3 hcho3 mentioned this pull request Oct 11, 2019
9 tasks
@lock lock bot locked as resolved and limited conversation to collaborators Oct 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants