magnars / stasis

Some Clojure functions for creating static websites.
348 stars 27 forks source link

slurp-resources fails because of chop-path #30

Closed craftybones closed 3 years ago

craftybones commented 3 years ago

slurp-resources fails when it tries to get all the file paths on class paths.

https://github.com/magnars/stasis/blob/37d3fc6a0124ea01ce6839c6202c96f90b9ca898/src/stasis/class_path.clj#L26

It seems to me as if the chop is superfluous. The canonicalPath provides the full path, not just from the project root, so in trying to chop the path's root dir from the class path, it ends up chopping the first few characters of the absolute path.

Is chop-path necessary?

craftybones commented 3 years ago
(cp/file-paths-on-class-path)
;; => ("rs/srijays/projects/clj/ipl-vis/src/ipl/.#web.clj"
;;     "rs/srijays/projects/clj/ipl-vis/src/ipl/web.clj"
;;     "rs/srijays/projects/clj/ipl-vis/src/md/xform.clj"
;;     "rs/srijays/projects/clj/ipl-vis/src/dev.clj"
;;     "ijays/projects/clj/ipl-vis/examples/static-site/build/foo.html"
;;     "ijays/projects/clj/ipl-vis/examples/static-site/src/spec.json"
;;     "ijays/projects/clj/ipl-vis/examples/static-site/src/foo.md"
;;     "applied_science/darkstar.clj"
;;     ".keep"
;;     "vega.js"
;;     "vega-lite.js")

In the above you will see that the first few entries have the "/Use" chopped off, and then "/Users/sr". This is because chop-path chops off the count of "/src" and "/examples" respectively.

magnars commented 3 years ago

Interesting. Do the tests run on your machine? If they do, any idea why this test works?

(fact (slurp-resources (str public-dir "/texts") #"\.txt$")
        => {"/banana.txt" "Banana"
            "/apple.txt" "Apple"
            "/fruit.txt" "Fruit"})
craftybones commented 3 years ago

The tests pass, however, it seems like these are mocked with with-files .

You will be able to reproduce this. Instead of using lein to run a repl, run a plain old clojure repl, require stasis.class-path and try the file-paths-on-class-path fn.

~/projects/clj/stasis> clojure
Clojure 1.10.1
user=> (require '[stasis.class-path :as cp])
nil
user=> (cp/file-paths-on-class-path)
("rs/srijays/projects/clj/stasis/src/stasis/core.clj" "rs/srijays/projects/clj/stasis/src/stasis/class_path.clj")

I suspect that since you were working with lein as your environment, it might have led to something like this.

Please let me know if you are able to reproduce.

magnars commented 3 years ago

Yes indeed, there's a difference between clojure and lein here:

➜  stasis git:(master) clojure
Clojure 1.9.0
user=> (require '[stasis.class-path :as cp])
nil
user=> (cp/file-paths-on-class-path)
("rs/magnars/projects/stasis/src/stasis/core.clj" "rs/magnars/projects/stasis/src/stasis/class_path.clj")

vs

➜  stasis git:(master) lein repl
nREPL server started on port 51540 on host 127.0.0.1 - nrepl://127.0.0.1:51540
REPL-y 0.4.3, nREPL 0.5.3
Clojure 1.10.1
Java HotSpot(TM) 64-Bit Server VM 1.8.0_131-b11
    Docs: (doc function-name-here)
          (find-doc "part-of-name-here")
  Source: (source function-name-here)
 Javadoc: (javadoc java-object-or-class-here)
    Exit: Control+D or (exit) or (quit)
 Results: Stored in vars *1, *2, *3, an exception in *e

user=> (require '[stasis.class-path :as cp])
nil
user=> (cp/file-paths-on-class-path)
("stasis/core_test.clj" "stasis/core.clj" "stasis/class_path.clj" "4.1.07.png" "META-INF/maven/stasis/stasis/pom.properties")

This is surprising and a bit disturbing.

dpsutton commented 3 years ago

I cloned this repo and ran lein classpath. The first few entries are

I suspect all of these default and convenient classpath roots provided by lein explain the differences here. I don't think this is surprising or disturbing. When doing

echo '{}' > deps.edn
~/p/c/stasis ❯❯❯ clojure -Spath
src:/Users/dan/.m2/....

The difference is quite apparent. Lein just adds more classpath roots onto the classpath than the clojure cli.

The extra namespace is found in test, the png in dev-resources, and i suspect the pom properties file was left over in target but on that one i'm not sure.

craftybones commented 3 years ago

There is more to this. Turns out that when running lein, the classpath entries are returned as absolute pathnames. This works fine when chop-path assumes the length of the path to be the amount to chop. However, when run as clojure, the entries are relative, so this assumption doesn't hold then.

magnars commented 3 years ago

Yes, this last part is what I find disturbing, in that there's quite a big difference in behavior based somehow on how you run the code. I wonder why that is.

craftybones commented 3 years ago

Leiningen does a trampoline right, and sets the classpath before a repl is launched, so it is quite different since it isn't a simple wrapper around Clojure.

This is a good read.

What about using clojure.java.io/resource instead of going through the classpath etc?

magnars commented 3 years ago

Is there some way to use that to list the files available on the class path?

craftybones commented 3 years ago

@magnars - Check this out: https://github.com/clojure/java.classpath/blob/master/src/main/clojure/clojure/java/classpath.clj

magnars commented 3 years ago

Okay, that might work. I'm not sure when I'll get the time to look at it right now, but if you want to give it a shot, then go for it.

craftybones commented 3 years ago

@magnars - I've done something absolutely trivial to solve it, tell me if this works for you. Before you get all the resource paths, I am coercing the path to be a canonical path. You've done it for all the files present inside a directory, but not at the top level.

(defn get-resource-paths [path]
  (let [path (.getCanonicalPath (File. path)) ;; line I added
        path-plus-slash-length (inc (count path))
        chop-path #(subs % path-plus-slash-length)
        file (File. path)]
    (->> (cond
          (.isDirectory file) (get-file-paths file)
          (.exists file) (get-jar-paths file)
          :else [])
         (map chop-path))))

I've tested it with 3 different REPLs, and I've got it to work in all 3. This fix should work, but it still doesn't get rid of the underlying challenges of a managed process like lein fiddling with the classpath.

magnars commented 3 years ago

Yes, that seems to be a fix. Released in 2.5.1. Thanks!