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

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

Open
jafingerhut opened this issue Apr 1, 2015 · 9 comments

Comments

@jafingerhut
Copy link
Contributor

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
Copy link
Contributor

yogthos commented Apr 1, 2015

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

@jafingerhut
Copy link
Contributor Author

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:

  • :profiles {:1.6 {:dependencies [[org.clojure/clojure "1.6.0"]]}
  •         :1.7a5 {:dependencies [[org.clojure/clojure "1.7.0-alpha5"]]}
    
  •         :1.7a6 {:dependencies [[org.clojure/clojure "1.7.0-alpha6"]]}}
    

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
Copy link
Contributor

yogthos commented Apr 6, 2015

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
Copy link

I'm gonna bet it's this ngrunwald/ring-middleware-format#37

@jafingerhut
Copy link
Contributor Author

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: clojure/clojure@692645c

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

@jafingerhut
Copy link
Contributor Author

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
Copy link
Contributor

yogthos commented Apr 6, 2015

yeah the key ordering is the likely culprit

@jafingerhut
Copy link
Contributor Author

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
Copy link
Contributor

yogthos commented Apr 7, 2015

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.

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

3 participants