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

Detect transitive dependencies for supported libraries #75

Closed
breandan opened this issue May 29, 2020 · 4 comments
Closed

Detect transitive dependencies for supported libraries #75

breandan opened this issue May 29, 2020 · 4 comments
Labels
bug Installation and functionality issues

Comments

@breandan
Copy link
Contributor

breandan commented May 29, 2020

I wonder if there is a way to import transitive dependencies by default for supported libraries imported with the %use keyword in the same way as the @file:DependsOn(<coordinates>)/@file:Repository(<absolute-path>) syntax does. Why is it necessary to re-declare all the transitive dependencies for supported libraries in the json library descriptor like so?

@ileasile
Copy link
Collaborator

ileasile commented Jun 9, 2020

Actually, %use keyword produces @file:DependsOn()/@file:Repository() calls, so specifying transitive dependencies shouldn't be needed. Do you have an example of library descriptor which doesn't work without specifying these dependencies?

@ileasile ileasile added the bug Installation and functionality issues label Jun 9, 2020
@breandan
Copy link
Contributor Author

breandan commented Jun 9, 2020

Hmm, maybe I did not phrase this very clearly. I am just wondering why it is necessary for supported libraries to re-declare all dependencies from the Maven POM inside of the JSON library descriptor? In all of the supported libraries, they need to declare the dependencies in the JSON like so:

"properties": {
    "v": "1.0.0-beta6",
    "slf4j": "1.7.25",
    "freemarker": "2.3.29"
  },
  "link": "https://github.com/eclipse/deeplearning4j",
  "dependencies": [
    "org.freemarker:freemarker:$freemarker",
    "org.nd4j:nd4j-api:$v",
    "org.nd4j:nd4j-native:$v",
    "org.nd4j:nd4j-native-platform:$v",
    "org.deeplearning4j:deeplearning4j-core:$v",
    "org.deeplearning4j:deeplearning4j-common:$v",
    "org.deeplearning4j:deeplearning4j-datasets:$v",
    "org.deeplearning4j:deeplearning4j-nn:$v",
    "org.deeplearning4j:deeplearning4j-nlp:$v",
    "org.deeplearning4j:deeplearning4j-ui:$v",
    "org.slf4j:slf4j-simple:$slf4j",
    "org.slf4j:slf4j-api:$slf4j"
  ],

These org.slf4j:slf4j-api:$slf4j and org.freemarker:freemarker:$freemarker dependencies have already been declared in the Maven POM, no? Unless the library author explicitly requires a different set of dependencies and versions, I would expect kotlin-jupyter to use the Maven dependencies and versions by default.

edit: Maybe the current behaviour makes sense. I guess it probably shouldn't import the transitive closure of all dependencies by default.

@ileasile
Copy link
Collaborator

ileasile commented Jun 9, 2020

Yes, I understand. I think, these transitive dependencies may be safely deleted, it just needs to be checked.

@breandan
Copy link
Contributor Author

I can confirm this now works as expected, previously there was some issue detecting dependencies that seems to have been fixed. Some of the officially supported libraries (e.g. Spark) submitted before the fix still include the transitive deps. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Installation and functionality issues
Projects
None yet
Development

No branches or pull requests

2 participants