tools.namespace

scan-dirs / scan-all can return incorrect dependencies when both clj and cljc files define same namespace

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Environment:
    tools.namespace 0.2.11 and 0.3.0-alpha2

Description

Originally filed as an issue for function scan-all, which at the time I was unaware was deprecated in 0.3.0 alphas. I have tested against the newer scan-dirs as well, and it has the same problem.

When there are both .clj and .cljc files defining the same namespace, tools.namespace scan-dirs and scan-all can use the dependencies from the .cljc file, ignoring those in the .clj file, whereas for Clojure/Java the opposite should be done.

See sample project and its README here for steps to reproduce: https://github.com/jafingerhut/tnstest

Note that Clojure/Java's behavior is to prefer .clj file over a .cljc file defining the same namespace, even if the .clj file is in a directory that is later in the classpath than a .cljc file. According to Alex Miller email on clojure-dev Google group, this behavior is by design. Link: https://groups.google.com/d/msg/clojure-dev/d5Hb1E7zfHE/sPybIAxgCwAJ

Activity

Hide
Stuart Sierra added a comment -

I can confirm this is a bug.

When I added multiple file extensions, I didn't consider the priority order.

Fixing this may be tricky.

It starts c.t.n.find. c.t.n.find/find-sources-in-dir filters by file extension (controlled by the platform argument in 0.3) but it only considers one directory at a time. Since the files could be in two different directories, it cannot necessarily discover two files with the same name but different extensions.

The next layer is c.t.n.file, which currently doesn't look at file extensions at all, but maybe it should.

In c.t.n.dir we have enough information to prioritize and filter files for the same namespace by extension. But to do it correctly, it has to handle updates too. For example, what happens to a project that has a .cljc file, then adds a .clj file. c.t.n.dir should treat that as removing the .cljc file from the dependency graph.

I think this will require storing the association between a namespace name and a file name, something I'd been hoping to avoid.

Show
Stuart Sierra added a comment - I can confirm this is a bug. When I added multiple file extensions, I didn't consider the priority order. Fixing this may be tricky. It starts c.t.n.find. c.t.n.find/find-sources-in-dir filters by file extension (controlled by the platform argument in 0.3) but it only considers one directory at a time. Since the files could be in two different directories, it cannot necessarily discover two files with the same name but different extensions. The next layer is c.t.n.file, which currently doesn't look at file extensions at all, but maybe it should. In c.t.n.dir we have enough information to prioritize and filter files for the same namespace by extension. But to do it correctly, it has to handle updates too. For example, what happens to a project that has a .cljc file, then adds a .clj file. c.t.n.dir should treat that as removing the .cljc file from the dependency graph. I think this will require storing the association between a namespace name and a file name, something I'd been hoping to avoid.
Hide
Stuart Sierra added a comment -

It might even make sense to make platform a property of the tracker, to make sure the same rules are always used when adding more files.

Show
Stuart Sierra added a comment - It might even make sense to make platform a property of the tracker, to make sure the same rules are always used when adding more files.
Hide
Andy Fingerhut added a comment -

Agreed that fixing this is not a small change to tools.namespace, hence the reason I don't have a proposed patch already. Came across this while thinking about how Eastwood could/should handle .cljc files. I'll add any proposed patches here if I come up with something.

Show
Andy Fingerhut added a comment - Agreed that fixing this is not a small change to tools.namespace, hence the reason I don't have a proposed patch already. Came across this while thinking about how Eastwood could/should handle .cljc files. I'll add any proposed patches here if I come up with something.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated: