[elpa] externals/javaimp 8019366: Various improvements

[Filipp Gunbin]

branch: externals/javaimp
commit 801936691e464870915be74c6a6f96f334e11fc2
Author: Filipp Gunbin <fgunbin@fastmail.fm>
Commit: Filipp Gunbin <fgunbin@fastmail.fm>

    Various improvements
---
 javaimp-gradle.el |  20 ++++---
 javaimp-maven.el  |   6 +-
 javaimp-util.el   |  44 +++++++++------
 javaimp.el        | 165 ++++++++++++++++++++++++++++++------------------------
 4 files changed, 135 insertions(+), 100 deletions(-)

diff --git a/javaimp-gradle.el b/javaimp-gradle.el
index edc9107..3f4a120 100644
--- a/javaimp-gradle.el
+++ b/javaimp-gradle.el
@@ -87,7 +87,7 @@ descriptor."
                 (cdr (assq 'build-dir alist))))
    :dep-jars (javaimp--split-native-path (cdr (assq 'dep-jars alist)))
    :load-ts (current-time)
-   :dep-jars-path-fetcher #'javaimp--gradle-fetch-dep-jars-path
+   :dep-jars-fetcher #'javaimp--gradle-fetch-dep-jars
    :raw nil))
 
 (defun javaimp--gradle-id-from-semi-separated (str)
@@ -106,7 +106,7 @@ descriptor."
                        :artifact artifact
                        :version (nth 2 parts)))))
 
-(defun javaimp--gradle-fetch-dep-jars-path (module ids)
+(defun javaimp--gradle-fetch-dep-jars (module ids)
   (javaimp--gradle-call
    ;; Always invoke on orig file (which is root build script)
    ;; because module's own file may not exist, even if reported by
@@ -114,13 +114,20 @@ descriptor."
    (javaimp-module-file-orig module)
    (lambda ()
      (re-search-forward "^dep-jars=\\(.*\\)$")
-     (match-string 1))
+     (javaimp--split-native-path (match-string 1)))
    (let ((mod-path (mapconcat #'javaimp-id-artifact (cdr ids) ":")))
      (unless (string-empty-p mod-path)
        (format ":%s:" mod-path)))))
 
 (defun javaimp--gradle-call (file handler &optional mod-path)
-  (let* ((local-gradlew (concat (file-name-directory file) "gradlew"))
+  (let* (;; There is (was) "-b" switch for specifying build file,
+         ;; however it became deprecated in Gradle 7, so we try to run
+         ;; in build file directory.
+         (default-directory (file-name-directory file))
+         ;; Prefer local gradle wrapper
+         (local-gradlew (if (memq system-type '(cygwin windows-nt))
+                            "gradlew.bat"
+                          "gradlew"))
          (gradlew (if (file-exists-p local-gradlew)
                       local-gradlew
                     javaimp-gradle-program)))
@@ -128,11 +135,6 @@ descriptor."
      gradlew
      handler
      "-q"
-     ;; It's easier for us to track jars instead of classes for java-library
-     ;; projects.  See
-     ;; 
https://docs.gradle.org/current/userguide/java_library_plugin.html#sec:java_library_classes_usage
-     "-Dorg.gradle.java.compile-classpath-packaging=true"
-     "-b" (javaimp-cygpath-convert-maybe file)
      "-I" (javaimp-cygpath-convert-maybe
            (expand-file-name "javaimp-init-script.gradle"
                              javaimp--basedir))
diff --git a/javaimp-maven.el b/javaimp-maven.el
index 87e4646..cfbd14e 100644
--- a/javaimp-maven.el
+++ b/javaimp-maven.el
@@ -128,7 +128,7 @@ resulting module trees."
                  (javaimp--xml-first-child (javaimp--xml-child 'directory 
build-elt))))
      :dep-jars nil          ; dep-jars is initialized lazily on demand
      :load-ts (current-time)
-     :dep-jars-path-fetcher #'javaimp--maven-fetch-dep-jars-path
+     :dep-jars-fetcher #'javaimp--maven-fetch-dep-jars
      :raw elt)))
 
 (defun javaimp--maven-id-from-xml (elt)
@@ -188,14 +188,14 @@ are somewhat arbitrary."
                 "pom.xml")
         modules)))))
 
-(defun javaimp--maven-fetch-dep-jars-path (module _ids)
+(defun javaimp--maven-fetch-dep-jars (module _ids)
   (javaimp--call-build-tool
    javaimp-mvn-program
    (lambda ()
      (goto-char (point-min))
      (search-forward "Dependencies classpath:")
      (forward-line 1)
-     (thing-at-point 'line))
+     (javaimp--split-native-path (thing-at-point 'line)))
    ;; always invoke for this module's pom.ml
    "-f" (javaimp-cygpath-convert-maybe
          (javaimp-module-file module))
diff --git a/javaimp-util.el b/javaimp-util.el
index c87e811..22dc917 100644
--- a/javaimp-util.el
+++ b/javaimp-util.el
@@ -25,8 +25,6 @@
 (require 'cl-lib)
 (require 'seq)
 
-(defconst javaimp-debug-buf-name "*javaimp-debug*")
-
 (defconst javaimp--basedir (file-name-directory load-file-name))
 
 (defconst javaimp--classlike-scope-types
@@ -54,6 +52,10 @@
     (interface . "in")
     (enum . "en")))
 
+(defvar javaimp-tool-output-buf-name "*javaimp-tool-output*"
+  "Name of the buffer to which `javaimp--call-build-tool' copies
+build tool output.  Can be let-bound to nil to suppress copying.")
+
 
 
 ;; Structs
@@ -71,8 +73,9 @@
   dep-jars
   load-ts
   ;; Function to retrieve DEP-JARS for MODULE, called with two
-  ;; arguments: MODULE and list of parent IDs
-  dep-jars-path-fetcher
+  ;; arguments: MODULE and list of parent IDs.  Should return a list
+  ;; of strings - jar file names.
+  dep-jars-fetcher
   raw                                   ;used only during parsing
   )
 
@@ -281,20 +284,29 @@ unchanged."
     path))
 
 (defun javaimp--call-build-tool (program handler &rest args)
-  "Runs PROGRAM with ARGS, then calls HANDLER in the temporary
-buffer and returns its result"
-  (message "Calling %s on args: %s" program args)
+  "Run PROGRAM with ARGS, then call HANDLER in the temporary buffer
+with point set to eob and return its result."
+  (message "Calling program: %s %s" program (string-join args " "))
   (with-temp-buffer
-    (let ((status (let ((coding-system-for-read
-                         (if (eq system-type 'cygwin) 'utf-8-dos)))
-                    ;; TODO check 
 in output on Gnu/Linux
-                    (apply #'process-file program nil t nil args)))
+    (let ((status
+           (let ((coding-system-for-read
+                  (when (eq system-type 'cygwin) 'utf-8-dos))
+                 (process-environment
+                  (cons (format "JAVA_HOME=%s" javaimp-java-home)
+                        process-environment)))
+             (apply #'process-file program nil t nil args)))
          (buf (current-buffer)))
-      (with-current-buffer (get-buffer-create javaimp-debug-buf-name)
-       (erase-buffer)
-       (insert-buffer-substring buf))
-      (or (and (numberp status) (= status 0))
-         (error "\"%s\" failed with status \"%s\"" program status))
+      (when javaimp-tool-output-buf-name
+        (with-current-buffer (get-buffer-create
+                              javaimp-tool-output-buf-name)
+          (setq buffer-read-only nil)
+         (erase-buffer)
+         (insert-buffer-substring buf)
+          (setq buffer-read-only t)))
+      (unless (and (numberp status) (= status 0))
+        (when javaimp-tool-output-buf-name
+          (display-buffer javaimp-tool-output-buf-name))
+       (error "%s exit status: %s" program status))
       (goto-char (point-min))
       (funcall handler))))
 
diff --git a/javaimp.el b/javaimp.el
index 9dcb086..75fef2c 100644
--- a/javaimp.el
+++ b/javaimp.el
@@ -38,9 +38,6 @@
 ;;
 ;;   Some details:
 ;;
-;; If Maven/Gradle failed, you can see its output in the buffer named
-;; by `javaimp-debug-buf-name' (default is "*javaimp-debug*").
-;;
 ;; Contents of jar files and Maven/Gradle project structures are
 ;; cached, so usually only the first command should take a
 ;; considerable amount of time to complete.  If a module's build file
@@ -194,6 +191,22 @@ gradlew program, it is used in preference."
     st)
   "Enables parsing angle brackets as lists")
 
+(defconst javaimp--jar-error-header
+  "There were errors when reading some of the dependency files,
+they are listed below.
+
+Note that if you're using java-library plugin in Gradle for any
+modules inside the project tree, then Gradle may avoid creating
+jars for them.  You need to put this into
+$HOME/.gradle/gradle.properties to force that:
+
+systemProp.org.gradle.java.compile-classpath-packaging=true
+
+For more info, see
+https://docs.gradle.org/current/userguide/java_library_plugin.html\
+#sec:java_library_classes_usage
+")
+
 
 ;;;###autoload
 (defun javaimp-visit-project (dir)
@@ -249,7 +262,7 @@ any module file."
        need-update ids)
     ;; check if deps are initialized
     (unless (javaimp-module-dep-jars module)
-      (message "Loading dependencies: %s" (javaimp-module-id module))
+      (message "Loading dependencies for module: %s" (javaimp-module-id 
module))
       (setq need-update t))
     ;; check if this or any parent build file has changed since we
     ;; loaded the module
@@ -272,68 +285,65 @@ any module file."
           (push (javaimp-module-id cur) ids))
        (setq tmp (javaimp-node-parent tmp))))
     (when need-update
-      (let* ((path (funcall (javaimp-module-dep-jars-path-fetcher module)
-                            module ids))
-             (new-dep-jars (javaimp--split-native-path path))
-            (new-load-ts (current-time)))
-       (setf (javaimp-module-dep-jars module) new-dep-jars)
-       (setf (javaimp-module-load-ts module) new-load-ts)))))
+      (setf (javaimp-module-dep-jars module)
+            (funcall (javaimp-module-dep-jars-fetcher module) module ids))
+      (setf (javaimp-module-load-ts module)
+            (current-time)))))
 
 (defun javaimp--get-jar-classes (file)
-  (let ((cached (cdr (assoc file javaimp-cached-jars))))
-    (cond ((null cached)
-          ;; create, load & put into cache
-          (setq cached
-                (make-javaimp-cached-jar
-                 :file file
-                 :read-ts (javaimp--get-file-ts file)
-                 :classes (javaimp--fetch-jar-classes file)))
-          (push (cons file cached) javaimp-cached-jars))
-         ((> (float-time (javaimp--get-file-ts (javaimp-cached-jar-file 
cached)))
-             (float-time (javaimp-cached-jar-read-ts cached)))
-          ;; reload
-          (setf (javaimp-cached-jar-classes cached) 
(javaimp--fetch-jar-classes file))
-          ;; update read-ts
-          (setf (javaimp-cached-jar-read-ts cached) (current-time))))
-    ;; return from cached
-    (javaimp-cached-jar-classes cached)))
-
-(defun javaimp--fetch-jar-classes (file)
+  (condition-case err
+      (let ((jar (alist-get file javaimp-cached-jars nil nil #'equal)))
+        (when (or (not jar)
+                  ;; If the file doesn't exist this will be current
+                  ;; time, and thus condition always true
+                  (> (float-time (javaimp--get-file-ts file))
+                    (float-time (javaimp-cached-jar-read-ts jar))))
+          (setq jar (make-javaimp-cached-jar
+                    :file file
+                    :read-ts (javaimp--get-file-ts file)
+                    :classes (javaimp--read-jar-classes file))))
+        (setf (alist-get file javaimp-cached-jars) jar)
+        (javaimp-cached-jar-classes jar))
+    (t
+     (setf (alist-get file javaimp-cached-jars nil 'remove #'equal) nil)
+     (signal (car err) (cdr err)))))
+
+(defun javaimp--read-jar-classes (file)
+  "Read FILE which should be a .jar or a .jmod and return classes
+contained in it as a list."
   (let ((ext (downcase (file-name-extension file))))
     (unless (member ext '("jar" "jmod"))
       (error "Unexpected file name: %s" file))
-    (message "Reading classes in file: %s" file)
-    (with-temp-buffer
-      (let ((coding-system-for-read (when (eq system-type 'cygwin)
-                                      'utf-8-dos)))
-        (process-file
-         (symbol-value (intern (format "javaimp-%s-program" ext)))
-         nil
-         t
-         nil
-         (if (equal ext "jar") "tf" "list")
-         ;; On cygwin, "jar/jmod" is a windows program, so file path
-         ;; needs to be converted appropriately.
-         (javaimp-cygpath-convert-maybe file 'windows)))
-      (goto-char (point-min))
-      (let (result curr)
-       (while (re-search-forward
-                (rx (and bol
-                         (? "classes/") ; prefix output by jmod
-                         (group (+ (any alnum "_/$")))
-                         ".class"
-                         eol))
-                nil t)
-          (setq curr (match-string 1))
-          (unless (or (string-suffix-p "module-info" curr)
-                      (string-suffix-p "package-info" curr)
-                      ;; like Provider$1.class
-                      (string-match-p "\\$[[:digit:]]" curr))
-            (push
-             (string-replace "/" "."
-                             (string-replace "$" "." curr))
-             result)))
-        result))))
+    (let ((javaimp-tool-output-buf-name nil)) ;don't log
+      (javaimp--call-build-tool
+       (symbol-value (intern (format "javaimp-%s-program" ext)))
+       #'javaimp--read-jar-classes-handler
+       (if (equal ext "jar") "tf" "list")
+       ;; On cygwin, "jar/jmod" is a windows program, so file path
+       ;; needs to be converted appropriately.
+       (javaimp-cygpath-convert-maybe file 'windows)))))
+
+(defun javaimp--read-jar-classes-handler ()
+  "Used by `javaimp--read-jar-classes' to handle jar program
+output."
+  (let (result curr)
+    (while (re-search-forward
+            (rx (and bol
+                     (? "classes/")     ; prefix output by jmod
+                     (group (+ (any alnum "_/$")))
+                     ".class"
+                     eol))
+            nil t)
+      (setq curr (match-string 1))
+      (unless (or (string-suffix-p "module-info" curr)
+                  (string-suffix-p "package-info" curr)
+                  ;; like Provider$1.class
+                  (string-match-p "\\$[[:digit:]]" curr))
+        (push
+         (string-replace "/" "."
+                         (string-replace "$" "." curr))
+         result)))
+    result))
 
 
 ;; Some API functions
@@ -404,13 +414,25 @@ current `symbol-at-point'."
             ;; module dependencies
             (when module
              ;; We're not caching full list of classes coming from
-             ;; module dependencies because jars may change and we
-             ;; need to reload them
-             (apply #'append
-                    (mapcar #'javaimp--get-jar-classes
-                             (javaimp-module-dep-jars module))))
+             ;; module dependencies because jars may change
+              (let (jar-errors)
+                (prog1
+                    (seq-mapcat (lambda (jar)
+                                  (condition-case err
+                                      (javaimp--get-jar-classes jar)
+                                    (t
+                                     (push (error-message-string err) 
jar-errors)
+                                     nil)))
+                                (javaimp-module-dep-jars module))
+                  (when jar-errors
+                    (with-output-to-temp-buffer "*Javaimp Jar errors*"
+                      (princ javaimp--jar-error-header)
+                      (terpri)
+                      (dolist (err (nreverse jar-errors))
+                        (princ err)
+                        (terpri)))))))
             ;; current module or source tree
-            (when javaimp-enable-parsing
+            (when (and module javaimp-enable-parsing)
               (seq-mapcat #'javaimp--get-directory-classes
                           (javaimp--get-current-source-dirs module)))
             ))
@@ -437,12 +459,12 @@ current `symbol-at-point'."
         ;; pre-jdk9
         (setq dir (mapconcat #'file-name-as-directory
                              `(,javaimp-java-home "jre" "lib") nil))
-        (message "jmods directory not found, fallback to reading jars from 
\"%s\"" dir)
+        (message "jmods directory not found, reading jars from \"%s\"" dir)
         (if (file-directory-p dir)
             (apply #'append
                   (mapcar #'javaimp--get-jar-classes
                            (directory-files dir t "\\.jar\\'")))
-          (user-error "JRE lib dir \"%s\" doesn't exist" dir))))))
+          (user-error "jre/lib dir \"%s\" doesn't exist" dir))))))
 
 (defun javaimp--get-current-source-dirs (module)
   "Return list of source directories to check for Java files."
@@ -584,13 +606,12 @@ is `ordinary' or `static'.  Interactively, NEW-IMPORTS is 
nil."
         ;; if there were any imports, we start inserting at the same place
         (goto-char start))
        ((re-search-forward "^\\s *package\\s " nil t)
-        ;; if there's a package directive, insert one blank line below and
-        ;; leave point after it
+        ;; if there's a package directive, insert one blank line
+        ;; below it
         (end-of-line)
         (if (eobp)
             (insert ?\n)
           (forward-line))
-        ;; then insert one blank line and we're done
         (insert ?\n))
        (t
         ;; otherwise, just go to bob