noir-clojure / lib-noir

A set of libraries for ring apps, including stateful sessions.
Eclipse Public License 1.0
483 stars 47 forks source link

Upgrade compojure and clout deps for compatibility with Clojure 1.7.0-alpha6 #112

Open jafingerhut opened 9 years ago

jafingerhut commented 9 years ago

See recent compojure and clout commits as of Apr 1 2015 for updates they have made to pull in latest Instaparse (version 1.3.6). Previous versions of Instaparse were not compatible.

yogthos commented 9 years ago

I just updated the dependencies, and I'll push out a new version to clojars tonight

jafingerhut commented 9 years ago

Looks like maybe something else besides upgrading compojure and clout deps is needed for compatibility with Clojure 1.7.0-alpha6, but I don't yet know what it is.

With latest version of lib-noir as of Apr 6 2015, but adding profiles to its project.clj for different versions of Clojure like this:

I see this behavior:

lein with-profile +1.6 do clean, test ; no errors lein with-profile +1.7a5 do clean, test ; no errors lein with-profile +1.7a6 do clean, test ; errors, an excerpt of them shown below

Exception in thread "main" java.lang.RuntimeException: Unable to resolve symbol: options in this context, compiling:(ring/middleware/format_params.clj:217:28) at clojure.lang.Compiler.analyze(Compiler.java:6543) at clojure.lang.Compiler.analyze(Compiler.java:6485) at clojure.lang.Compiler$InvokeExpr.parse(Compiler.java:3791)

I encountered this error on Mac OS X 10.10.2, Oracle Java 1.8.0_11, Leiningen 2.5.0. Also Ubuntu Linux 14.04.2, Oracle Java 1.8.0_25, Lein 2.5.0 (also Oracle Java 1.7.0_71)

yogthos commented 9 years ago

I just bumped up ring-middleware-format to the latest version, so hopefully that helps. Otherwise, have to wait until it's compatible with the latest Clojure release.

MichaelBlume commented 9 years ago

I'm gonna bet it's this https://github.com/ngrunwald/ring-middleware-format/pull/37

jafingerhut commented 9 years ago

Thanks, Michael.

From your description of the issue at the link, that jogs my memory that Rich Hickey did commit a change in Clojure 1.7.0-alpha6 such that small literal maps will be PersistentArrayMaps instead of PersistentHashMaps, and I think at the same time he reversed the order that PersistentArrayMaps stored their elements (not sure about that last statement, though -- just a quick guess from looking at the source code change). Here is the commit: https://github.com/clojure/clojure/commit/692645c73c86d12c93a97c858dc6e8b0f4280a0b

(only the last two 'hunks' of that commit are relevant)

jafingerhut commented 9 years ago

With the latest commit to lib-noir on Apr 6 2015, it is no longer throwing an exception. I see 2 tests failing, though. I haven't looked at them closely yet, but it might be due to the tests depending upon the order of key/value pairs in a map, and the change in this I guessed about above in 1.7.0-alpha6.

yogthos commented 9 years ago

yeah the key ordering is the likely culprit

jafingerhut commented 9 years ago

Looks like it might be key ordering, but another possible factor is that deftests are run in an order that can depend upon the seq order of hashes, too. I believe that with Clojure 1.7.0-alpha5, the set-timeout! test is actually running before the set-size! test, because I see a :timeout key in the cache when printing out its contents in the set-size! test with that version of Clojure, but I do not see that key with Clojure 1.7.0-alpha6.

Maybe clear! should be returning the cache back to its initial state of {} rather than only removing the items? Either that or combine some of your deftest contents together so you can control the order they are executed.

yogthos commented 9 years ago

I won't have time to look into this in the near future, but it sounds like your analysis is correct. I'm not opposed to modify clear! behavior and/or changing the flow for the tests though. If you'd like to give it a shot I can push out the update.