Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

[WIP][MXNET-1302,MXNET-1331] Backport fix to v1.3.x branch #14348

Closed
wants to merge 1 commit into from

Conversation

jessr92
Copy link
Contributor

@jessr92 jessr92 commented Mar 6, 2019

Consumers of MXNet are exposed to the commons-codec and commons-io library versions MXNet uses internally. This change excludes those JARs from the JARs created with make scalapkg.

Description

This PR removes commons-codec and commons-io from the assembled JAR and adds both packages as dependencies in deploy.xml

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
  • [] Changes are complete (i.e. I finished coding on this PR)
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • 3rd party libraries removed from assembled JAR
  • [] 3rd party libraries added as dependencies to deployed POM

Comments

Backport of pull request 14000 and 14303 to v1.3.x branch

Originally reported in #13929

@jessr92 jessr92 marked this pull request as ready for review March 6, 2019 20:31
@jessr92
Copy link
Contributor Author

jessr92 commented Mar 6, 2019

@mxnet-label-bot update [maven, scala, pr-work-in-progress]

Copy link
Contributor

@ChaiBapchya ChaiBapchya left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.

Looks like it is failing on windows build. All 3 failures are related to same line

File "ci/build_windows.py", line 33, in <module>

    from util import *

  File "C:\jenkins_slave\workspace\build-gpu\ci\util.py", line 21

    def get_mxnet_root() -> str:

Have a look.

@lanking520
Copy link
Member

@yajiedesign Could you please take a look if you know what has been changed on master?

@jessr92 jessr92 force-pushed the v1.3.x branch 2 times, most recently from cdbf380 to 8187132 Compare March 7, 2019 21:32
@jessr92 jessr92 changed the title [MXNET-1302,MXNET-1331] Backport fix to v1.3.x branch [WIP][MXNET-1302,MXNET-1331] Backport fix to v1.3.x branch Mar 7, 2019
@jessr92
Copy link
Contributor Author

jessr92 commented Mar 12, 2019

Given the error, looks like the workflow migrated from Python 2 to Python 3? I think it's complaining about the "arrow" syntax.

@jessr92 jessr92 force-pushed the v1.3.x branch 2 times, most recently from ff81651 to 46be84e Compare March 14, 2019 15:45
@zachgk
Copy link
Contributor

zachgk commented Mar 18, 2019

@perdasilva Can you help take a look at this? It seems to be failing on the Windows CI

@perdasilva
Copy link
Contributor

perdasilva commented Mar 19, 2019

@zachgk right. There've been changes to the windows ci pipeline. I'll need to back port the changes to v1.3.x. I'll make that PR today for you and try to get it merged asap. The PR to bring these changes over: #14469

@lanking520
Copy link
Member

@gordon1992 Got @gordon1992 PR in. Please try to pull --rebase with 1.3.x to pass the CI. Thanks!

@jessr92
Copy link
Contributor Author

jessr92 commented Mar 28, 2019

Apologise for the delay, I've been on annual leave. I have now rebased.

@jessr92
Copy link
Contributor Author

jessr92 commented Mar 29, 2019

Looks like the new failure is to do with my exclusion of Scala modules. This makes sense since this PR is WIP with the remaining task to declare dependencies on the excluded modules. Expectation is that the build will pass after that has been completed.

@lanking520
Copy link
Member

It seemed like Clojure failed because of that

@jessr92 jessr92 force-pushed the v1.3.x branch 6 times, most recently from 5c79aa2 to c94039c Compare April 4, 2019 14:01
@jessr92 jessr92 force-pushed the v1.3.x branch 2 times, most recently from d92ecc2 to 9c8c676 Compare April 4, 2019 15:36
@zachgk
Copy link
Contributor

zachgk commented Apr 8, 2019

@gordon1992, it might be easier to test this locally. What you need to do is:

  1. Build Engine following https://mxnet.incubator.apache.org/versions/master/install/scala_setup.html
  2. Run mvn install from the scala-package folder to build Scala and load it into your maven cache
  3. Install the clojure tool following https://github.com/technomancy/leiningen/wiki/Packaging
  4. Run lein test from the contrib/clojure-package folder to run the Clojure tests (pulling the Scala jars from your cache).

@karan6181
Copy link
Contributor

@gordon1992 Could you please try the suggestion provided by @zachgk

@jessr92
Copy link
Contributor Author

jessr92 commented Jun 19, 2019

Hello,

Apologies for the really bad delays to working on this. Trying to set time aside for this has been tricky.

From what I can see, backporting the fix in a clean way is tricky given the Maven build restructuring that has occurred (and is only present in 1.5 onwards).

Thanks for your patience,
Gordon

@jessr92
Copy link
Contributor Author

jessr92 commented Jul 30, 2019

Unfortunately I'm unlikely to get to this. Sorry.

@jessr92 jessr92 closed this Jul 30, 2019
@jessr92 jessr92 deleted the v1.3.x branch July 30, 2019 10:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Maven pr-work-in-progress PR is still work in progress Scala
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants