Re-organize Scala maven build #13626

frankfliu · 2018-12-12T17:41:20Z

This is a follow up of: https://issues.apache.org/jira/browse/MXNET-1224
A design was posted: https://cwiki.apache.org/confluence/display/MXNET/Scala+maven+build+improvement

Automatically detect which platform to build for scala.
Remove platform dependend submodules
Fix cyclic module dependencies
Fix scalatype style check
Now mvn can be executed in submodule
Maven build can be executed from any directory not only in root project
Checkin javah header file, and use verify task to detect native API changes
Improve incremental build performance
Remove unittest and integration-test profile, use proper task instead
Delete generated scala file during maven clean.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

Changes are complete (i.e. I finished coding on this PR)
All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

lanking520

I saw there are a couple of changes regarding to ${scala.binary.version} to 2.11. Any specific reason for doing that? It will make it hard to switch to 2.12 if we need to make that change

scala-package/macros/src/main/scala/org/apache/mxnet/APIDocGenerator.scala

roywei · 2018-12-14T17:38:28Z

@mxnet-label-bot add[Scala, Build, pr-awaiting-review]

frankfliu · 2018-12-14T18:07:20Z

${scala.binary.version} is a mistake in original maven.
${scala.binary.version} was used in artifact id, not for scale compiler target version.

We already release 2.11 artifact on maven central, we can't simple change it, 2.12 will be different package on maven central.
in pom, artifact id in pom.xml should be a constants, not a variable
I removed all internal reference to ${scala.binary.version}, and only kept in public facing artifact name.

piyushghai · 2018-12-14T18:10:10Z

contrib/clojure-package/project.clj

@@ -29,7 +29,7 @@
                 ;[org.apache.mxnet/mxnet-full_2.11-linux-x86_64-gpu "1.2.1"]

                 ;;; CI
-                 [org.apache.mxnet/mxnet-full_2.11-linux-x86_64-cpu "1.5.0-SNAPSHOT"]
+                 [org.apache.mxnet/mxnet-full_2.11 "1.5.0-SNAPSHOT"]


Why do you want to remove the platform type from here. ?

We removed platform and cpu/gpu specific pom.xml file, the local installed artifact name is now mxnet-full_2.11, clojure won't be able to find mxnet-full_2.11-linux-x86_64-cpu in .m2 any more.

lanking520

With some heavy testing on the published package, the generated package is working. I will give a LGTM for now. However @frankfliu Please provide more detailed information in README. @yzhliu @nswamy @gigasquid Please spend sometime to review this PR since the change is big.

Also add a C++ expert in here to check the code: @cjolivier01

gigasquid · 2018-12-15T18:55:57Z

make scalapkg failed for me on OSX locally with

[INFO] --- exec-maven-plugin:1.6.0:java (apidoc-generation) @ mxnet-macros_2.11 ---
[WARNING] 
java.lang.UnsatisfiedLinkError: /Users/cmeier/workspace/deep-learning/mxnet/scala-package/init-native/osx-x86_64/target/libmxnet-init-scala-osx-x86_64.jnilib: dlopen(/Users/cmeier/workspace/deep-learning/mxnet/scala-package/init-native/osx-x86_64/target/libmxnet-init-scala-osx-x86_64.jnilib, 1): Library not loaded: @rpath/libmxnet.so
  Referenced from: /Users/cmeier/workspace/deep-learning/mxnet/scala-package/init-native/osx-x86_64/target/libmxnet-init-scala-osx-x86_64.jnilib
  Reason: image not found
	at java.lang.ClassLoader$NativeLibrary.load(Native Method)
	at java.lang.ClassLoader.loadLibrary0(ClassLoader.java:1941)
	at java.lang.ClassLoader.loadLibrary(ClassLoader.java:1824)
	at java.lang.Runtime.load0(Runtime.java:809)
	at java.lang.System.load(System.java:1086)
	at org.apache.mxnet.init.Base$.tryLoadInitLibrary(Base.scala:51)
	at org.apache.mxnet.init.Base$.<init>(Base.scala:21)
	at org.apache.mxnet.init.Base$.<clinit>(Base.scala)
	at org.apache.mxnet.GeneratorBase.getBackEndFunctions(GeneratorBase.scala:64)
	at org.apache.mxnet.GeneratorBase.typeSafeFunctionsToGenerate(GeneratorBase.scala:53)
	at org.apache.mxnet.APIDocGenerator$.typeSafeClassGen(APIDocGenerator.scala:51)
	at org.apache.mxnet.APIDocGenerator$.main(APIDocGenerator.scala:35)
	at org.apache.mxnet.APIDocGenerator.main(APIDocGenerator.scala)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.codehaus.mojo.exec.ExecJavaMojo$1.run(ExecJavaMojo.java:282)
	at java.lang.Thread.run(Thread.java:748)

Don't know whether the problem is in this PR or has to do with the problem fixed in
#13655

frankfliu · 2018-12-15T20:51:07Z

@gigasquid
Did you rebuild libmnxet.so file? did you do mvn clean?
It seems you are still building old scala project:

mxnet-macros_2.11 has been change to mxnet-macros
init-native so file output location should be: init-native/target/libmxnet-init-scala-osx-x86_64.jnilib, but your build is still looking for init-native/osx-x86_64/target/libmxnet-init-scala-osx-x86_64.jnilib

gigasquid · 2018-12-16T01:30:49Z

Seems like even though the init-native/osx-x86_64 directory were deleted and untracked, it was still left on my system. I needed to do a git clean -f to get rid of it. Once I did make scalapkg finished, but I got an error on make scalainstall

[INFO] 
[INFO] --- exec-maven-plugin:1.6.0:exec (verify-javah) @ mxnet-scala-init ---
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] MXNet Scala Package - Parent ....................... SUCCESS [  6.659 s]
[INFO] MXNet Scala Package - Initializer .................. FAILURE [  4.619 s]
[INFO] MXNet Scala Package - Initializer Native ........... SKIPPED
[INFO] MXNet Scala Package - Macros ....................... SKIPPED
[INFO] MXNet Scala Package - Native ....................... SKIPPED
[INFO] MXNet Scala Package - Core ......................... SKIPPED
[INFO] MXNet Scala Package - Inference .................... SKIPPED
[INFO] MXNet Scala Package - Examples ..................... SKIPPED
[INFO] MXNet Scala Package - Spark ML ..................... SKIPPED
[INFO] MXNet Scala Package - Full osx-x86_64-only ......... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 11.671 s
[INFO] Finished at: 2018-12-15T20:27:01-05:00
[INFO] Final Memory: 36M/578M
[INFO] ------------------------------------------------------------------------
[WARNING] The requested profile "osx-x86_64-cpu" could not be activated because it does not exist.
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:1.6.0:exec (verify-javah) on project mxnet-scala-init: Unable to parse configuration of mojo org.codehaus.mojo:exec-maven-plugin:1.6.0:exec for parameter successCodes: Cannot assign configuration entry 'successCodes' with value '0' of type java.lang.String to property of type java.lang.Integer[] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginConfigurationException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :mxnet-scala-init

Any ideas?

lanking520 · 2018-12-16T01:34:32Z

@gigasquid Frank's build is no longer have the requirement for the makefile. Try with mvn install

lanking520 · 2018-12-16T01:35:13Z

Apart from that, @frankfliu could you please apply the change I have made in your PR: #13655

gigasquid · 2018-12-16T14:07:53Z

@frankfliu @lanking520 - I still have the same problem when I do mvn install as well. Maybe someone else that has OSX can help give it a test drive to see if my problems or local or not. Since we don't have OSX in our CI process right now, I'd like to make sure that path is tested.

lanking520 · 2018-12-16T14:11:30Z

@gigasquid thanks for testing. I can remember that @frankfliu change the artifact of the package which remove cpu and gpu. Let's focus on this question and please feel free to add request changes.

gigasquid · 2018-12-16T14:46:13Z

To follow up - I did a new clone from the @frankfliu 's fork and ran into the same error. So I don't think it's due to any leftover files on my system. Here is a gist of the full log of mvn install https://gist.github.com/gigasquid/a07b3e64737e7dfa3eaf990aed436efb

frankfliu · 2018-12-17T17:06:12Z

@gigasquid Can you check if you have a diff command installed on your system?
It seems failed on diff command.

lanking520 · 2018-12-17T18:27:26Z

@gigasquid problem is not reproducible on my laptop. I ran mvn clean package install and it seemed to be fine.

lanking520 · 2018-12-29T16:07:37Z

@zachgk let's plan to prepare for a PR to address Frank's change. Once it's been raised, we can merge this one and get that one for review.

scala-package/deploy/pom.xml

scala-package/pom.xml

zachgk

One minor thing. Otherwise LGTM

zachgk · 2019-01-03T00:02:33Z

scala-package/README.md

 ```

 Or run a subset of unit tests, for e.g.,

 ```bash
-make SCALA_TEST_ARGS=-Dsuites=org.apache.mxnet.NDArraySuite scalaunittest
+cd scala-package
+mvn -DSCALA_TEST_ARGS=-Dsuites=org.apache.mxnet.NDArraySuite integration-test


I think this should just be mvn -Dsuites=org.apache.mxnet.NDArraySuite integration-test

lanking520

Thanks for the contribution on MKLDNN! Please address the comments

lanking520 · 2019-01-08T18:03:19Z

scala-package/core/src/main/scala/org/apache/mxnet/util/NativeLibraryLoader.scala

+    saveLibraryToTemp("libmklml_intel.so", "/lib/native/libmklml_intel.so", false)
+    saveLibraryToTemp("libmklml.dylib", "/lib/native/libmklml.dylib", false)
+    saveLibraryToTemp("libmkldnn.so.0", "/lib/native/libmkldnn.so.0", false)
+    saveLibraryToTemp("libmkldnn.0.dylib", "/lib/native/libmkldnn.0.dylib", false)


Is there a way we can obtain every file name in the native folder and apply them here?

https://stackoverflow.com/questions/5694385/getting-the-filenames-of-all-files-in-a-folder

There actually might be unnecessary .so files. If the script didn't clean them, we might accidentally includes some garbage in the jar.

It's much safer to explicitly include files that we need. And it's also a good for documentation purpose.

lanking520

Approve for now to reduce the crosses 👍 . Please address the rest of them and I think we are good to go.

lanking520 · 2019-01-08T18:27:41Z

Makefile

-			-Dcflags="$(CFLAGS)" -Dldflags="$(LDFLAGS)" \
-			-Dcurrent_libdir="$(ROOTDIR)/lib" \
-			-Dlddeps="$(LIB_DEP) $(ROOTDIR)/lib/libmxnet.a")
+	(cd $(ROOTDIR)/scala-package && mvn install -DskipTests)


Could we change this into mvn package and skiptests?

This should be install, package doesn't do verify.

lanking520 · 2019-01-08T18:42:30Z

scala-package/assembly/src/main/assembly/assembly.xml

+      <directory>${MXNET_DIR}/3rdparty</directory>
+      <includes>
+        <include>cub/LICENSE.TXT</include>
+        <include>mkldnn/external/mklml_mac_2019.0.1.20180928/license.txt</include>


Please add this section in README to remind user update the licence

scala-package/pom.xml

1. Automatically detect which platform to build for scala. 2. Remove platform dependend submodules 3. Fix cyclic module dependencies 4. Fix scalatype style check 5. Now mvn can be executed in submodule 6. Maven build can be executed from any directory not only in root project 7. Checkin javah header file, and use verify task to detect native API changes 8. Improve incremental build performance 9. Remove unittest and integration-test profile, use proper task instead 10. Delete generated scala file during maven clean.

1. Removed maven release plugin. 2. Make maven build friendly to CI, allow cli override version. 3. Moved gpg signing to deploy stage. 4. Created a separeated deploy module. 5. Updated Makefile to new maven build change. 6. Remove unused nexus-staging-plugin 7. Added nightly and staging profile for CI.

Makefile

* Re-organize scala maven build 1. Automatically detect which platform to build for scala. 2. Remove platform dependend submodules 3. Fix cyclic module dependencies 4. Fix scalatype style check 5. Now mvn can be executed in submodule 6. Maven build can be executed from any directory not only in root project 7. Checkin javah header file, and use verify task to detect native API changes 8. Improve incremental build performance 9. Remove unittest and integration-test profile, use proper task instead 10. Delete generated scala file during maven clean. * Redo maven deploy related tasks. 1. Removed maven release plugin. 2. Make maven build friendly to CI, allow cli override version. 3. Moved gpg signing to deploy stage. 4. Created a separeated deploy module. 5. Updated Makefile to new maven build change. 6. Remove unused nexus-staging-plugin 7. Added nightly and staging profile for CI. * Support mkldnn for Scala.

frankfliu requested review from nswamy, szha and yzhliu as code owners December 12, 2018 17:41

lanking520 self-requested a review December 13, 2018 01:25

lanking520 reviewed Dec 13, 2018

View reviewed changes

scala-package/macros/src/main/scala/org/apache/mxnet/APIDocGenerator.scala Show resolved Hide resolved

frankfliu force-pushed the scala branch 2 times, most recently from f2f1280 to eb31f49 Compare December 14, 2018 05:02

frankfliu requested a review from gigasquid as a code owner December 14, 2018 05:02

marcoabreu added Build pr-awaiting-review PR is waiting for code review Scala labels Dec 14, 2018

piyushghai reviewed Dec 14, 2018

View reviewed changes

frankfliu force-pushed the scala branch 2 times, most recently from a779b60 to 0603c23 Compare December 14, 2018 23:48

lanking520 approved these changes Dec 15, 2018

View reviewed changes

frankfliu force-pushed the scala branch 2 times, most recently from 8d74d3a to d9eff8a Compare December 15, 2018 21:44

frankfliu force-pushed the scala branch from d9eff8a to 164f77a Compare December 16, 2018 02:26

frankfliu force-pushed the scala branch 2 times, most recently from b3ea348 to b94b1a0 Compare December 21, 2018 03:29

gigasquid approved these changes Dec 21, 2018

View reviewed changes

lanking520 reviewed Dec 29, 2018

View reviewed changes

scala-package/deploy/pom.xml Outdated Show resolved Hide resolved

scala-package/deploy/pom.xml Outdated Show resolved Hide resolved

scala-package/pom.xml Show resolved Hide resolved

frankfliu force-pushed the scala branch from b94b1a0 to 207fb3b Compare January 2, 2019 18:36

zachgk mentioned this pull request Jan 2, 2019

[MXNET-862] Basic maven jenkins pipeline #13450

Merged

5 tasks

zachgk approved these changes Jan 3, 2019

View reviewed changes

frankfliu force-pushed the scala branch from 207fb3b to e63ba74 Compare January 8, 2019 00:21

lanking520 reviewed Jan 8, 2019

View reviewed changes

lanking520 approved these changes Jan 8, 2019

View reviewed changes

frankfliu added 2 commits January 8, 2019 11:38

frankfliu force-pushed the scala branch from e63ba74 to b8e16e4 Compare January 8, 2019 19:45

lanking520 reviewed Jan 8, 2019

View reviewed changes

Makefile Outdated Show resolved Hide resolved

Support mkldnn for Scala.

2f650a6

frankfliu force-pushed the scala branch from b8e16e4 to 2f650a6 Compare January 8, 2019 21:07

lanking520 merged commit 9c24df2 into apache:master Jan 9, 2019

frankfliu deleted the scala branch January 9, 2019 17:08

frankfliu mentioned this pull request Jan 9, 2019

Fix scala doc build break for v1.3.1 #13820

Merged

4 tasks

zachgk mentioned this pull request Jan 12, 2019

[Scala][macOS] Trying to build from source #12568

Closed

zachgk mentioned this pull request Feb 22, 2019

Running the mxnet scala jar in the interpreter throws an error #13468

Closed

vkorenev mentioned this pull request Jul 20, 2019

mvn install produces packages with invalid pom xml #15618

Closed

barumrho mentioned this pull request Jul 24, 2019

1.5.0 Scala package missing from Maven Central #15650

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-organize Scala maven build #13626

Re-organize Scala maven build #13626

frankfliu commented Dec 12, 2018

lanking520 left a comment

roywei commented Dec 14, 2018

frankfliu commented Dec 14, 2018

piyushghai Dec 14, 2018

frankfliu Dec 15, 2018

lanking520 left a comment

gigasquid commented Dec 15, 2018 •

edited

Loading

frankfliu commented Dec 15, 2018

gigasquid commented Dec 16, 2018 •

edited

Loading

lanking520 commented Dec 16, 2018

lanking520 commented Dec 16, 2018

gigasquid commented Dec 16, 2018 •

edited

Loading

lanking520 commented Dec 16, 2018

gigasquid commented Dec 16, 2018 •

edited

Loading

frankfliu commented Dec 17, 2018

lanking520 commented Dec 17, 2018

lanking520 commented Dec 29, 2018

zachgk left a comment

zachgk Jan 3, 2019

lanking520 left a comment

lanking520 Jan 8, 2019

frankfliu Jan 8, 2019

lanking520 left a comment

lanking520 Jan 8, 2019

frankfliu Jan 8, 2019

lanking520 Jan 8, 2019

Re-organize Scala maven build #13626

Re-organize Scala maven build #13626

Conversation

frankfliu commented Dec 12, 2018

Checklist

Essentials

lanking520 left a comment

Choose a reason for hiding this comment

roywei commented Dec 14, 2018

frankfliu commented Dec 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lanking520 left a comment

Choose a reason for hiding this comment

gigasquid commented Dec 15, 2018 • edited Loading

frankfliu commented Dec 15, 2018

gigasquid commented Dec 16, 2018 • edited Loading

lanking520 commented Dec 16, 2018

lanking520 commented Dec 16, 2018

gigasquid commented Dec 16, 2018 • edited Loading

lanking520 commented Dec 16, 2018

gigasquid commented Dec 16, 2018 • edited Loading

frankfliu commented Dec 17, 2018

lanking520 commented Dec 17, 2018

lanking520 commented Dec 29, 2018

zachgk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lanking520 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lanking520 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gigasquid commented Dec 15, 2018 •

edited

Loading

gigasquid commented Dec 16, 2018 •

edited

Loading

gigasquid commented Dec 16, 2018 •

edited

Loading

gigasquid commented Dec 16, 2018 •

edited

Loading