damballa / inet.data

Clojure library for modeling various Internet-related conceptual entities as data
Eclipse Public License 1.0
38 stars 6 forks source link

Unable to resolve symbol: address in this context #3

Closed daveyarwood closed 9 years ago

daveyarwood commented 9 years ago

Hi,

I'm getting this exception when I require inet.data.ip:

boot.user=> (merge-env! :dependencies '[[inet.data "0.5.5"]])
Retrieving hier-set-1.1.2.jar from https://clojars.org/repo/
Retrieving inet.data-0.5.5.jar from https://clojars.org/repo/
nil
boot.user=> (require '[inet.data.ip :as ip])

clojure.lang.Compiler$CompilerException: java.lang.RuntimeException: Unable to resolve symbol: address in this context, compiling:(inet/data/ip.clj:103:3)
             java.lang.RuntimeException: Unable to resolve symbol: address in this context

I think there may be a typo here?

https://github.com/damballa/inet.data/blob/master/src/clojure/inet/data/ip.clj#L99-L108

My first thought was "address should be addr on line 108," but then I saw that it's in a threading form. Is there supposed to be an address function defined somewhere?

daveyarwood commented 9 years ago

I figured it out -- it looks like it should be -address. I made the change (address => -address) on lines 108 and 117, and tried all the examples in the README, and all is well.

I also found a similar typo in inet.data.dns, which was easily fixed by changing domain to -domain on line 80.

Pull request on the way!

llasram commented 9 years ago

The current code looks wrong and the PR looks reasonable (modulo the Clojure 1.4 errors, which I don't care to support anymore anyway), but I'm not actually able to reproduce the exception myself! What versions of Clojure and the JVM are you using? And are you able to reproduce the problem from the Leiningen REPL?

daveyarwood commented 9 years ago

Hmm... this is really interesting!

I also cannot reproduce the error in a Leiningen REPL with Clojure 1.6.0. I am able to require inet.data.ip and use its functions without problems. The interesting thing, though, is that, as far as I can tell, the error should be reproducible in a Leiningen REPL, but it isn't happening for some weird reason.

The exception I got was in a Boot REPL with Clojure 1.7.0, but I tried with Clojure 1.6.0 as well and I still got the error. So, I guess it may be a boot vs lein thing?

What's interesting to me is that the address function ( defined here: https://github.com/damballa/inet.data/blob/master/src/clojure/inet/data/ip.clj#L205-L208 ) is referred to 100 lines earlier on line 103 (where the exception occurs in the Boot REPL), and yet in the Leiningen REPL, no exception is thrown -- it's as if the environment "knows" about the address function even though it hasn't been defined or declared yet. This may be a bug with Leiningen, IMHO.

llasram commented 9 years ago

Ah ha! I think I've figured it out. And am able to reproduce by directly invoking clojure.main, not just Leiningen. It looks like boot / your configuration is causing inet.data's data_readers.clj file to not be loaded. Running the Leiningen REPL or launching via clojure.main causes data_readers.clj to be loaded, which has the side-effect of creating referenced reader-function namespaces and vars. These vars are unbound (as via declare) until defined, which allows the namespace to load without throwing an exception.

Now that I understand the problem, I'd like to fix it without circumventing the domain and address protocol-wrappers, even though they're currently pure pass-throughs. Would you mind updating your PR (via force-push) to instead either declare the vars or (my preference) move the wrapper functions to earlier in the namespace?

Thanks!

daveyarwood commented 9 years ago

Ah! Yeah, that makes perfect sense. I've amended and force-pushed my PR.

In inet.data.ip, I was able to move the address protocol wrapper earlier in the file.

It's a little trickier in inet.data.dns, though -- the domain wrapper has a DNSDomain tag, and DNSDomain is not defined until further down, around line 208. Hopefully declaring domain after the protocol definition is OK.

llasram commented 9 years ago

I've released version 0.5.6 including the fix. Thanks for the report and PR!