Add ability to skip dynapath-based functionality

Closes clojure-emacs#112
Related clojure-emacs#103
Related clojure-emacs#105

.circleci/config.yml

@@ -28,6 +28,9 @@ executors:
   openjdk15:
     docker:
       - image: circleci/clojure:openjdk-15-lein-2.9.5-buster
+  openjdk16:
+    docker:
+      - image: circleci/clojure:openjdk-16-lein-2.9.5-buster
     <<: *defaults
 
 # Runs a given set of steps, with some standard pre- and post-
@@ -114,9 +117,13 @@ jobs:
       clojure_version:
         description: Version of Clojure to test against
         type: string
+      test_profiles:
+        description: Maps directly to the TEST_PROFILES var in Makefil
+        type: string
     executor: << parameters.jdk_version >>
     environment:
       VERSION: << parameters.clojure_version >>
+      TEST_PROFILES: << parameters.test_profiles >>
     steps:
       - checkout
       - with_cache:
@@ -146,53 +153,11 @@ workflows:
   ci-test-matrix:
     jobs:
       - test_code:
-          name: Java 8, Clojure 1.8
-          clojure_version: "1.8"
-          jdk_version: openjdk8
-      - test_code:
-          name: Java 8, Clojure 1.9
-          clojure_version: "1.9"
-          jdk_version: openjdk8
-      - test_code:
-          name: Java 8, Clojure 1.10
-          clojure_version: "1.10"
-          jdk_version: openjdk8
-      - test_code:
-          name: Java 8, Clojure master
-          clojure_version: "master"
-          jdk_version: openjdk8
-      - test_code:
-          name: Java 11, Clojure 1.8
-          clojure_version: "1.8"
-          jdk_version: openjdk11
-      - test_code:
-          name: Java 11, Clojure 1.9
-          clojure_version: "1.9"
-          jdk_version: openjdk11
-      - test_code:
-          name: Java 11, Clojure 1.10
-          clojure_version: "1.10"
-          jdk_version: openjdk11
-      - test_code:
-          name: Java 11, Clojure master
-          clojure_version: "master"
-          jdk_version: openjdk11
-      - test_code:
-          name: Java 15, Clojure 1.8
-          clojure_version: "1.8"
-          jdk_version: openjdk15
-      - test_code:
-          name: Java 15, Clojure 1.9
-          clojure_version: "1.9"
-          jdk_version: openjdk15
-      - test_code:
-          name: Java 15, Clojure 1.10
-          clojure_version: "1.10"
-          jdk_version: openjdk15
-      - test_code:
-          name: Java 15, Clojure master
-          clojure_version: "master"
-          jdk_version: openjdk15
+          matrix:
+            parameters:
+              jdk_version: [openjdk8, openjdk11, openjdk15, openjdk16]
+              clojure_version: ["1.8", "1.9", "1.10", "master"]
+              test_profiles: ["+test", "+test,+no-dynapath"]
       - util_job:
           name: Code Linting, JDK8 (Eastwood only)
           jdk_version: openjdk8

.gitignore

@@ -3,6 +3,7 @@
 /checkouts
 /classes
 /gh-pages
+/unzipped-jdk-source
 /target
 autodoc.sh
 pom.xml

CHANGELOG.md

@@ -8,6 +8,8 @@
 
 ### Changes
 
+* [#113](https://github.com/clojure-emacs/orchard/issues/113) Add ability to skip functionality that works by altering the classpath
+  * You an opt in to this choice by setting `"-Dorchard.fetch-java-sources-via-dynapath=false"`.
 * The _class info cache_ is now initialized silently by default. This results in less confusing output.
   * You can now `@orchard.java/cache-initializer` for deterministically waiting for this cache workload to complete.
   * You can control its verbosity by setting `"-Dorchard.initialize-cache.silent=false"` (or `[...]=true`).

Makefile

@@ -2,7 +2,7 @@
 
 VERSION ?= 1.10
 
-TEST_PROFILES := +test
+TEST_PROFILES ?= +test
 
 resources/clojuredocs/export.edn:
 curl -o $@ https://github.com/clojure-emacs/clojuredocs-export-edn/raw/master/exports/export.compact.edn

README.md

@@ -91,8 +91,14 @@ functionality that's provided.
 So far, Orchard follows these options, which can be specified as Java system properties
 (which means that end users can choose to set them globally without fiddling with tooling internals):
 
+* `"-Dorchard.fetch-java-sources-via-dynapath=false"` (default: true)
+  * if `false`, all features that currently depend on dynapath (and therefore alter the classpath) will be disabled.
+  * This is a way to avoid a number of known issues.
+  * Note that if this option is `false`, Orchard clients will have to figure out themselves a way to e.g. fetch Java sources.
+    * It is foreseen that soon enough this will be reliably offered as a Lein plugin.
 * `"-Dorchard.initialize-cache.silent=true"` (default: `true`)
-  * if false, the _class info cache_ initialization may print warnings (possibly spurious ones).
+  * if `false`, the _class info cache_ initialization may print warnings (possibly spurious ones).
+xx
 
 ## History
 

project.clj

@@ -1,3 +1,62 @@
+(require 'clojure.java.io)
+
+(import '[java.util.zip ZipInputStream]
+        '[java.io FileOutputStream])
+
+(defmacro while-let [[sym expr] & body]
+  `(loop [~sym ~expr]
+     (when ~sym
+       ~@body
+       (recur ~expr))))
+
+(defn jdk-find [f]
+  (let [home (clojure.java.io/file (System/getProperty "java.home"))
+        parent (.getParentFile home)
+        paths [(clojure.java.io/file home f)
+               (clojure.java.io/file home "lib" f)
+               (clojure.java.io/file parent f)
+               (clojure.java.io/file parent "lib" f)]]
+    (->> paths (filter #(.canRead ^java.io.File %)) first str)))
+
+(def jdk-sources
+  (let [java-path->zip-path (fn [path]
+                   (some-> (clojure.java.io/resource path)
+                           ^java.net.JarURLConnection (. openConnection)
+                                      (. getJarFileURL)
+                                      clojure.java.io/as-file
+                                      str))]
+    (or (java-path->zip-path "java.base/java/lang/Object.java") ; JDK9+
+                   (java-path->zip-path "java/lang/Object.java") ; JDK8-
+        (jdk-find "src.zip"))))
+
+;;         while ((len = zis.read(buffer)) > 0) {
+;;             fos.write(buffer, 0, len);
+;;         }
+
+(defn uncompress [path target]
+  (let [zis (-> target io/input-stream ZipInputStream.)]
+    (while-let [entry (-> zis .getNextEntry)]
+      (let [size (-> entry .getSize)
+            bytes (byte-array 1024)
+            dest (->> entry .getName (clojure.java.io/file path))
+            dir (-> entry .getName (clojure.string/split #"/") butlast)
+            _ (->> (clojure.string/join "/" dir) (java.io.File. path) .mkdirs)
+            output (FileOutputStream. dest)]
+        (loop [len (-> zis (.read bytes))]
+          (when (pos? len)
+            (-> output (.write bytes 0 len))
+            (recur (-> zis (.read bytes)))))
+        (-> output .close)))))
+
+(defn unzipped-jdk-source []
+  (let [choice jdk-sources]
+    (when-not (-> "unzipped-jdk-source" clojure.java.io/file .exists)
+      (-> "unzipped-jdk-source" clojure.java.io/file .mkdirs)
+      (uncompress "./unzipped-jdk-source/" choice))
+    "unzipped-jdk-source"))
+
+(def jdk8? (->> "java.version" System/getProperty (re-find #"^1.8.")))
+
 (defproject cider/orchard "0.6.5"
   :description "A fertile ground for Clojure tooling"
   :url "https://github.com/clojure-emacs/orchard"
@@ -23,6 +82,8 @@
                                     :password :env/clojars_password
                                     :sign-releases false}]]
 
+  :jvm-opts ["-Dorchard.fetch-java-sources-via-dynapath=true"]
+
   :profiles {
              ;; Clojure versions matrix
              :provided {:dependencies [[org.clojure/clojure "1.10.1"]
@@ -38,10 +99,17 @@
                       :dependencies [[org.clojure/clojure "1.11.0-master-SNAPSHOT"]
                                      [org.clojure/clojure "1.11.0-master-SNAPSHOT" :classifier "sources"]]}
 
-             :test {:resource-paths ["test-resources"]
+             :test {:dependencies [[org.clojure/java.classpath "1.0.0"]]
+                    :resource-paths ["test-resources"]
                     ;; Initialize the cache verbosely, as usual, so that possible issues can be more easily diagnosed:
                     :jvm-opts ["-Dorchard.initialize-cache.silent=false"]}
 
+             :no-dynapath {:jvm-opts ["-Dorchard.fetch-java-sources-via-dynapath=false"]
+                           :resource-paths [~(unzipped-jdk-source)]
+                           :plugins ~(if jdk8?
+                                       '[[lein-jdk-tools "0.1.1"]]
+                                       [])}
+
              ;; Development tools
              :dev {:dependencies [[pjstadig/humane-test-output "0.10.0"]]
                    :resource-paths ["test-resources"]
@@ -63,8 +131,6 @@
                          {:dependencies [[clj-kondo "2021.03.31"]]}]
 
              :eastwood  {:plugins  [[jonase/eastwood "0.4.0"]]
-                         :eastwood {:exclude-namespaces [~(if (-> "java.version"
-                                                                  System/getProperty
-                                                                  (.contains "1.8."))
+                         :eastwood {:exclude-namespaces [~(if jdk8?
                                                             'orchard.java.parser
                                                             'orchard.java.legacy-parser)]}}})

src/orchard/java.clj

@@ -58,17 +58,24 @@
                (io/file parent "lib" f)]]
     (->> paths (filter #(.canRead ^File %)) first io/as-url)))
 
+(def fetch-java-sources-via-dynapath?
+  "Should orchard use the dynapath library to use \"fetch Java sources\" functionality?
+
+  Note that using dynapath currently implies some bugs, so you might want to disable this option."
+  (contains? #{"true" "1"} (System/getProperty "orchard.fetch-java-sources-via-dynapath" "true")))
+
 (def jdk-sources
   "The JDK sources path. If found on the existing classpath, this is the
   corresponding classpath entry. Otherwise, the JDK directory is searched for
   the file `src.zip`."
-  (let [base-url (fn [path]
-                   (some-> (io/resource path)
-                           ^JarURLConnection (. openConnection)
-                           (. getJarFileURL)))]
-    (or (base-url "java.base/java/lang/Object.java") ; JDK9+
-        (base-url "java/lang/Object.java")           ; JDK8-
-        (jdk-find "src.zip"))))
+  (when fetch-java-sources-via-dynapath?
+    (let [base-url (fn [path]
+                     (some-> (io/resource path)
+                             ^JarURLConnection (. openConnection)
+                             (. getJarFileURL)))]
+      (or (base-url "java.base/java/lang/Object.java") ; JDK9+
+          (base-url "java/lang/Object.java")           ; JDK8-
+          (jdk-find "src.zip")))))
 
 (def jdk-tools
   "The `tools.jar` path, for JDK8 and earlier. If found on the existing
@@ -78,15 +85,17 @@
     (or (some-> (io/resource "com/sun/javadoc/Doc.class")
                 ^JarURLConnection (. openConnection)
                 (. getJarFileURL))
-        (some-> (jdk-find "tools.jar") cp/add-classpath!))))
+        (and fetch-java-sources-via-dynapath?
+             (some-> (jdk-find "tools.jar") cp/add-classpath!)))))
 
 (defn ensure-jdk-sources
   "If `jdk-sources` is present, check that this entry is on the context
   classpath and if not, add it."
   []
-  (let [classpath (set (cp/classpath))]
-    (when (and jdk-sources (not (classpath jdk-sources)))
-      (cp/add-classpath! jdk-sources))))
+  (when fetch-java-sources-via-dynapath?
+    (let [classpath (set (cp/classpath))]
+      (when (and jdk-sources (not (classpath jdk-sources)))
+        (cp/add-classpath! jdk-sources)))))
 
 ;;; ## Javadoc URLs
 ;;

test/orchard/java/classpath_test.clj

@@ -4,6 +4,7 @@
    [clojure.set :as set]
    [clojure.string :as str]
    [clojure.test :refer [deftest is testing]]
+   [orchard.java]
    [orchard.java.classpath :as cp]
    [orchard.misc :as misc])
   (:import
@@ -48,31 +49,33 @@
            (set (cp/system-classpath))
            (set (cp/classpath)))))))
 
-(deftest classpath-resources-test
-  (testing "Iterating classpath resources"
-    (testing "returns non-empty lists"
-      (is (every? seq (map cp/classpath-seq (cp/classpath)))))
-    (testing "returns relative paths"
-      (is (every? #(not (.isAbsolute (File. %)))
-                  (mapcat cp/classpath-seq (cp/classpath)))))))
+(when orchard.java/fetch-java-sources-via-dynapath?
+  (deftest classpath-resources-test
+    (testing "Iterating classpath resources"
+      (testing "returns non-empty lists"
+        (is (every? seq (map cp/classpath-seq (cp/classpath)))))
+      (testing "returns relative paths"
+        (is (every? #(not (.isAbsolute (File. %)))
+                    (mapcat cp/classpath-seq (cp/classpath))))))))
 
-(deftest classloader-test
-  (testing "Classloader hierarchy contains current classloader"
-    (is (contains? (set (cp/classloaders)) (cp/context-classloader))))
-  (testing "Classpath modification"
-    (let [orig-classloaders (cp/classloaders)
-          orig-classpath (cp/classpath)
-          url (-> (System/getProperty "java.io.tmpdir")
-                  (io/file "test.txt")
-                  (io/as-url))]
-      (cp/add-classpath! url)
-      (testing "adds the URL"
-        (is (contains? (set (cp/classpath)) url)))
-      (testing "preserves prior classpath URLs"
-        (is (set/subset?
-             (set orig-classpath)
-             (set (cp/classpath)))))
-      (testing "preserves the classloader hierarchy"
-        (is (set/subset?
-             (set orig-classloaders)
-             (set (cp/classloaders))))))))
+(when orchard.java/fetch-java-sources-via-dynapath?
+  (deftest classloader-test
+    (testing "Classloader hierarchy contains current classloader"
+      (is (contains? (set (cp/classloaders)) (cp/context-classloader))))
+    (testing "Classpath modification"
+      (let [orig-classloaders (cp/classloaders)
+            orig-classpath (cp/classpath)
+            url (-> (System/getProperty "java.io.tmpdir")
+                    (io/file "test.txt")
+                    (io/as-url))]
+        (cp/add-classpath! url)
+        (testing "adds the URL"
+          (is (contains? (set (cp/classpath)) url)))
+        (testing "preserves prior classpath URLs"
+          (is (set/subset?
+               (set orig-classpath)
+               (set (cp/classpath)))))
+        (testing "preserves the classloader hierarchy"
+          (is (set/subset?
+               (set orig-classloaders)
+               (set (cp/classloaders)))))))))

test/orchard/java/classpath_test/third_party_compat_test.clj

@@ -0,0 +1,13 @@
+(ns orchard.java.classpath-test.third-party-compat-test
+  (:require
+   [orchard.java]
+   [clojure.java.classpath]
+   [clojure.test :refer [deftest is]]))
+
+;; make this namespace's tests deterministic:
+@orchard.java/cache-initializer
+
+(when-not orchard.java/fetch-java-sources-via-dynapath?
+  (deftest works
+    (is (seq (clojure.java.classpath/classpath-directories))
+        "The presence of `clojure.java` does not affect third-party libraries")))

test/orchard/java_test.clj

@@ -9,8 +9,11 @@
 
 (def jdk-parser? (or (>= misc/java-api-version 9) jdk-tools))
 
-(when-not jdk-parser? (println "No JDK parser available!"))
-(when-not jdk-sources (println "No JDK sources available!"))
+(assert jdk-parser? "No JDK parser available!")
+(assert (if orchard.java/fetch-java-sources-via-dynapath?
+          jdk-sources
+          true)
+        "No JDK sources available!")
 
 (javadoc/add-remote-javadoc "com.amazonaws." "http://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/")
 (javadoc/add-remote-javadoc "org.apache.kafka." "https://kafka.apache.org/090/javadoc/")
@@ -25,7 +28,8 @@
                              (dp/all-classpath-urls)
                              (set))]
       (testing "of defined vars"
-        (is (= jdk-sources jdk-sources-path))
+        (when orchard.java/fetch-java-sources-via-dynapath?
+          (is (= jdk-sources jdk-sources-path)))
         (is (= jdk-tools jdk-tools-path)))
       (testing "of dynamically added classpath entries"
         (is (= jdk-sources (classpath-urls jdk-sources)))