brandonbloom / fipp

Fast Idiomatic Pretty Printer for Clojure
525 stars 44 forks source link

Work around clojure.instant bootclasspath problem #65

Closed venantius closed 4 years ago

venantius commented 4 years ago

As of Java 9, certain build tools during boot will use the bootclasspath (this includes both Lein and Boot afaik). This is a problem for clojure.instant as it relies on java.sql.Timestamp which isn't available on the bootclasspath.

With Fipp, this means that libraries and plugins requiring fipp.ednize during the build system's operation will fail unless LEIN_USE_BOOTCLASSPATH is set to no (or similar workaround for Boot).

This (admittedly somewhat evil) commit circumvents that by checking to see if clojure.instant has already been loaded and only extending the edn protocol for date/time related objects if it has been loaded.

Should resolve #60, https://github.com/venantius/ultra/issues/108 and https://github.com/venantius/ultra/issues/103

brandonbloom commented 4 years ago

As proposed, this code change will cause load-order-sensitive bugs. That is, if you don't load clojure.instant until you after you load fipp, you won't get an unresolved var error, the edn protocol won't be extended to the date & instant types, and there will be no way to later load those extensions. This highlights a real underlying issue: The lack of an explicit require for clojure.instant. I've fixed/deployed a change for that.

I'm going to close this PR and reopen issue #60. More details and next steps in a comment there: Please see https://github.com/brandonbloom/fipp/issues/60#issuecomment-529218689

I don't know much about Java 9 or the bootclasspath, but the I'd like to point out that the release notes say "The boot class path has been mostly removed in this release. "