eastwood breaks with cyclic dependencies

[lvh]

I know what you're thinking: "don't have cyclic dependencies"! I agree, but the project that got bit by this is cloverage. Since cloverage has to instrument existing projects, as you might expect there are some issues when it is fed a cyclic dependency. This previously caused inscrutable error messages. Cloverage now has a unit test to verify how it deals with this edge case. Because there's ostensibly no way to tell eastwood to ignore certain files, this means eastwood breaks on cloverage.

You can see the ticket where we're attempting to implement eastwood here: cloverage/cloverage#203

[gonewest818]

Actually, even when I tell eastwood to ignore the namespace containing the circular dependency, it crashes anyway.

Here's a simple repro:

$ lein new foo
$ cd foo
$ cat >! src/foo/core.clj << END
> (ns foo.core
>   (:require [foo.core :as self]))
> 
> (defn foo
>   "I don't do a whole lot."
>   [x]
>   (println x "Hello, World!"))
> END
$ lein eastwood
== Eastwood 0.2.5 Clojure 1.8.0 JVM 1.8.0_112
Exception in thread "main" clojure.lang.ExceptionInfo: Circular dependency between foo.core and foo.core {:reason :eastwood.copieddeps.dep9.clojure.tools.namespace.dependency/circular-dependency, :node foo.core, :dependency foo.core}, compiling:(/private/var/folders/q_/5jmdzlp91sxgd4ynt4hr17vw0000gn/T/form-init5497450741451784475.clj:1:125)
...
$ lein eastwood "{:exclude-namespaces [foo.core]}"
== Eastwood 0.2.5 Clojure 1.8.0 JVM 1.8.0_112
Exception in thread "main" clojure.lang.ExceptionInfo: Circular dependency between foo.core and foo.core {:reason :eastwood.copieddeps.dep9.clojure.tools.namespace.dependency/circular-dependency, :node foo.core, :dependency foo.core}, compiling:(/private/var/folders/q_/5jmdzlp91sxgd4ynt4hr17vw0000gn/T/form-init3554495570221597198.clj:1:125)

The irritating thing is that last line. I can't even exclude the namespace to avoid the exception.

BTW the behavior is different if foo.core is written like so:

(ns foo.core)

(require '[foo.core :as self])

(defn foo
  "I don't do a whole lot."
  [x]
  (println x "Hello, World!"))

Which eastwood can handle successfully:

$ lein eastwood
== Eastwood 0.2.5 Clojure 1.8.0 JVM 1.8.0_112
Directories scanned for source files:
  src test
== Linting foo.core ==
== Linting foo.core-test ==
== Warnings: 0 (not including reflection warnings)  Exceptions thrown: 0

But as @lvh said above, a tool like cloverage isn't in a position to rewrite the caller's code.

[jafingerhut]

I was able to reproduce this with a tiny project like the one you gave, and the exception is thrown while Eastwood is using the tools.namespace library to determine what the dependencies are between the namespaces in your project. It raises an exception when it finds any dependency cycle, whether it is within one namespace like in this example, or it is among multiple namespaces.

At least right now, Eastwood uses this library to analyze all of the initial ns forms in all files of your project, regardless of :exclude-namespaces or :namespaces options.

Among namespaces that you do want Eastwood to process, it calculates these dependencies so that it can calculate an order for analyzing namespaces that starts with namespaces depended upon, before analyzing namespaces that depend upon them.

Perhaps one enhancement to Eastwood here would be to only calculate these dependencies among namespaces that Eastwood is told to do linting on, avoiding any that it should not. That would enable you in this case to use :exclude-namespaces on the self-requiring one, and avoid the error.

[gonewest818]

Yes, that enhancement would be helpful in my case. Thanks for digging into that.

…

On Jan 20, 2018, at 1:34 PM, Andy Fingerhut ***@***.***> wrote: I was able to reproduce this with a tiny project like the one you gave, and the exception is thrown while Eastwood is using the tools.namespace library to determine what the dependencies are between the namespaces in your project. It raises an exception when it finds any dependency cycle, whether it is within one namespace like in this example, or it is among multiple namespaces. At least right now, Eastwood uses this library to analyze all of the initial ns forms in all files of your project, regardless of :exclude-namespaces or :namespaces options. Among namespaces that you do want Eastwood to process, it calculates these dependencies so that it can calculate an order for analyzing namespaces that starts with namespaces depended upon, before analyzing namespaces that depend upon them. Perhaps one enhancement to Eastwood here would be to only calculate these dependencies among namespaces that Eastwood is told to do linting on, avoiding any that it should not. That would enable you in this case to use :exclude-namespaces on the self-requiring one, and avoid the error. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[jafingerhut]

@gonewest818 @lvh I did another experiment on a toy project that had two namespaces, where each had an ns form that :require'd the other. Clojure itself gives an error message about a cyclic dependency if you attempt to require either of those namespaces, and I believe the same thing happens for cyclic dependency 'rings' with any number of namespaces in the cycle, 2 or more.

The exception is if a namespace has an ns form that requires itself, i.e. a cycle with only 1 namespace in it. Clojure does not give an error attempting to require such a namespace, the way it does for cycles with 2 or more namespaces in them.

So another possible approach would be to modify tools.namespace so that it does not throw an exception for length 1 cycles, but continues to throw an exception for cycles with length 2 or more, since Clojure will not accept those, either.

All of that said, is this issue with Cloverage that someone filed a bug against Cloverage because it failed when used on a Clojure project with namespaces that require themselves? Sure, you can't change someone else's code, but you can file bugs against it. :-) A namespace require'ing itself seems pretty useless (the require is useless that is -- the namespace might still be useful).

[gonewest818]

What I can do is segregate the sources with the circularity under a different path, and create a separate profile. Add that profile for builds and tests, but leave it out when running Eastwood.

…

On Jan 21, 2018, at 3:59 PM, Andy Fingerhut ***@***.***> wrote: @gonewest818 @lvh I did another experiment on a toy project that had two namespaces, where each had an ns form that :require'd the other. Clojure itself gives an error message about a cyclic dependency if you attempt to require either of those namespaces, and I believe the same thing happens for cyclic dependency 'rings' with any number of namespaces in the cycle, 2 or more. The exception is if a namespace has an ns form that requires itself, i.e. a cycle with only 1 namespace in it. Clojure does not give an error attempting to require such a namespace, the way it does for cycles with 2 or more namespaces in them. So another possible approach would be to modify tools.namespace so that it does not throw an exception for length 1 cycles, but continues to throw an exception for cycles with length 2 or more, since Clojure will not accept those, either. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[jafingerhut]

I found this issue in tools.namespace where the change was made that causes it to throw an exception for a namespace depending upon itself. Before this change, tools.namespace would not raise an exception in this case, although the commit message suggests that some other tools.namepace functions may go into an infinite loop before this change: stuartsierra/dependency#1 (reference)

[jafingerhut]

A little more background: It looks like origin of this test in cloverage was this issue: cloverage/cloverage#117

The creator of that issue said they probably would have found the namespace that depended upon itself earlier if Eastwood / kibit ran on their project, but their project was using Clojure 1.9 namespaced maps before either Eastwood or kibit were updated to support those (Eastwood version 0.2.5 does, but it was not released until around the time Clojure 1.9.0 was released, probably a year to a year and a half after namespaced maps were added to Clojure 1.9 alpha releases).

[vemv]

I don't think there's an actionable item here. Cloverage currently uses Eastwood with no major workarounds.

I see it has some special handling for a ns named cloverage.sample.exercise-instrumentation but this special handle is simply for adding some per-ns linter disabling, not something related to cyclic deps.

cyclic deps make a project unworkable for certain dev workflows, e.g. tools.namespace will straight out reject such a project. So I don't think this is something that should be supported.