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

missing dependencies #35

Open
magomimmo opened this issue Nov 14, 2015 · 6 comments
Open

missing dependencies #35

magomimmo opened this issue Nov 14, 2015 · 6 comments

Comments

@magomimmo
Copy link

By switching from "0.2.0" release to "0.3.0" release I got the fallowing dependencies warning and error:

...
Writing boot_reload.cljs...
You are missing necessary dependencies for boot-cljs-repl.
Please add the following dependencies to your project:
[com.cemerick/piggieback "0.2.1" :scope "test"]
[weasel "0.7.0" :scope "test"]
[org.clojure/tools.nrepl "0.2.12" :scope "test"]
....
Adding :require adzerk.boot-reload to main.cljs.edn...
java.io.FileNotFoundException: Could not locate cemerick/piggieback__init.class or cemerick/piggieback.clj on classpath.
...
Elapsed time: 1.578 sec

It seems that the dependencies' transitivity is lost on the way. Obviously if I explicitly add the missing deps it works.

@Deraen
Copy link
Contributor

Deraen commented Nov 14, 2015

Working as intended.

In 0.3.0 I made a decision to require direct dependencies to CLJS Repl libs because with current implementation it's impossible to reliably and easily add those automatically. Having boot-cljs-repl depend on the libs transitively is problematic because there is no guarantee that transitive dependencies from boot-cljs-repl are used. If several libs depend on e.g. piggieback, the nearest and first version is selected, which could be wrong version.

@magomimmo
Copy link
Author

uhm, in this way we're breaking the most standard behaviour of any maven-based lib:
https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Transitive_Dependencies

At least we should write an ATTENTION NOTE in the readme.md and explain the reason why as well. When you say with the current implementation you mean that you're working on a new implementation?

thanks

@Deraen
Copy link
Contributor

Deraen commented Nov 15, 2015

I don't see how this breaks any behavior?

Boot tasks preferably shouldn't have any transitive dependencies and if they need to depend on something they should use pods. But the current implementation runs REPL on the "main" pod (where your app runs). Automatically adding dependencies to this pod (by calling set-env! :dependencies in task or transitive dependencies) can cause side-effects to the application so it should be avoided.

I've been talking about this with @micha and we think it should be eventually possible to run the REPL inside a pod.

@Deraen
Copy link
Contributor

Deraen commented Nov 15, 2015

Okay, so maybe saying "doesn't break any behavior" is a bit wrong as this was breaking change. I have now fixed the change log to mention this and fixed a typo there.

I just think that standard behaviors of maven libs don't completely hold here because Boot tasks, like Leiningen plugins, are a bit of special case. Both should use their own mechanisms to add the dependencies in a way that doesn't have side-effects on application dependencies. (Leiningen tasks occasionally have to break this, which can cause problems like the plugin dependencies leaking to pom.xml on the jar).

I also added a mention and explanation on the readme.

@magomimmo
Copy link
Author

Thanks!. Are those dependencies to be added always the same? In this case I would be more explicit in the note about them, otherwise the user has to first run the boot command to discover what are the deps to be added, add them e then rerun the boot command again.

@Deraen
Copy link
Contributor

Deraen commented Nov 15, 2015

Yes. Though now that I think of, it's possible that we want to update them in the future.

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

No branches or pull requests

2 participants