boot-clj / boot

Build tooling for Clojure.
https://boot-clj.github.io/
Eclipse Public License 1.0
1.75k stars 180 forks source link

Bump pomegranate and dynapath to 1.0.0 #612

Closed arichiardi closed 6 years ago

arichiardi commented 7 years ago

Hello folks!

Now that https://github.com/cemerick/pomegranate/pull/83 is in, I took the time to adapt to 0.4.0-alpha1. Leiningen is testing the change too and it makes sense to keep in sync.

One notable change is:

Note that item 3 above is technically a breaking change; however I would argue it is a bugfix for a critical security flaw. It's still possible to explicitly opt-in to non-TLS HTTP repositories if you don't care for security by calling register-wagon-factory! yourself.

I wanted to write it down here so that we don't miss it.

I am going to try it locally of course and report if I find any issue with it.

arichiardi commented 7 years ago

Probably worth adding some entry to the wiki as well, see https://github.com/technomancy/leiningen/commit/809c7d33ea5ef2ffefa1dc31ace839155955600d

arichiardi commented 7 years ago

Just wanted to report that it is not breaking anything. I have also tried the :repositories key with a private Nexus with no issues.

Deraen commented 6 years ago

Is this up-to-date with Pomegranate 0.4.0 final?

Obviously not, as the dep points to alpha.

arichiardi commented 6 years ago

Yeah no, and I think I should have a look and see if this needs more bumping

martinklepsch commented 6 years ago

@arichiardi can you bump the dependency and add a changelog entry mentioning the need for register-wagon-factory! when using non-TLS HTTP repos?

martinklepsch commented 6 years ago

Thanks @arichiardi. I've noticed that pomegranate 1.0.0 has been released (changelog) with Java 9 support.

Wondering if anyone has opinions on how we should upgrade to that? The new pomegranate also depends on dynapath 1.0.0 which we probably probably should upgrade to as well.

Both of these releases are marked as BREAKING but if I understand this section on the dynapath README correctly we might be lucky and @tobias already addressed those breakages in Boot.

seancorfield commented 6 years ago

Is there also perhaps an argument in favor of shifting to the tooling that tools.deps uses (Apache Resolver?) or am I missing some element of what pomegranate provides that is fundamentally different?

martinklepsch commented 6 years ago

@seancorfield I think the big difference between these two is that Pomegranate allows modifying the classpath, something that isn't really a goal with tools.deps as far as I am aware. Might still be worth discussing using it for dependency resolution though.

arichiardi commented 6 years ago

I wonder what breaking means in that case... probably I should just try to bump

tobias commented 6 years ago

@martinklepsch correct, boot can ignore the breaking changes from dynapath, since it doesn't use a URLClassLoader since https://github.com/boot-clj/boot/commit/a046a497a8bb7f3d1e7aa8d4db4a81c51beaef7d and https://github.com/boot-clj/boot-bin/commit/172a3d2b12c8299b0bac7f1431f8a621c695e5ff

martinklepsch commented 6 years ago

Awesome, so that means we can just upgrade to Pomegranate 1.0.0 / Dynapath 1.0.0. Thanks for already taking care of that @tobias, much appreciated!

martinklepsch commented 6 years ago

@arichiardi With that in mind what do you think about the following:

This way we can git-bisect any differences between 0.4.0 and 1.0.0 Changelog entry could be done in both commits, just updating the version in the second.

If you think the two commits are not worth it just make one 🙂

arichiardi commented 6 years ago

Ops I think I have already pushed on this branch. Not too much change from 0.4.0 to 1.0.0 anyways...but let me update the commit message

arichiardi commented 6 years ago

This is working fine on my end, reject things when necessary (private Nexus with no https enabled)

martinklepsch commented 6 years ago

Merged via https://github.com/boot-clj/boot/commit/c5e5651e89e85192dc9dcb4f2bb91572bae6367a & https://github.com/boot-clj/boot/commit/f26a12c7510360709746d08bd24301cf11dbf70d with the Markdown link fix mentioned above.

We should probably release a SNAPSHOT soon so people can help testing this without building locally. Also we should release SNAPSHOTs more often :)