`package-get-version` might not always return the cider version.

[ikappaki]

Expected behavior

package-get-version was recently used in CIDER to simplify the logic to retrieve the current CIDER version.

It should return the CIDER package version as set in the cider.el package header.

Actual behavior

The fn uses various heuristics to locate the the package file (in our case cider.el) where the package version is located at, and it fails if the parent directory name is not named after the package file, i.e. `cider'.

This for example, causes the Eldev compilation test to issue a warning in cider-util.el causing the corresponding CircleCI job to fail:

[00:00.929]  
             In toplevel form:
             cider-util.el:377:8:Error: value returned from (fboundp 'package-get-version) is unused
[00:00.930]  Saving target dependencies to file `.eldev/27.2/target-dependencies.build'...

This is because the the call to (package-get-version) is expanded at compile time to nil (package-get-version is marked as pure and as such its value is resolved at compile time. CircleCi checks out the code at /root/project. Because project != cider the fn returns nil since it looks for a project.el file to extract the version number from), thus the if statement returns nil in both true and false cases causing the above warning:

(defun cider--version ()
  "Retrieve CIDER's version.
A codename is added to stable versions."
  (if (string-match-p "-snapshot" cider-version)
      (let ((pkg-version (if (fboundp 'package-get-version)
                             (package-get-version)
                           nil)))
        (if pkg-version
            ;; snapshot versions include the MELPA package version
            (format "%s (package: %s)" cider-version pkg-version)
          cider-version))
    ;; stable versions include the codename of the release
    (format "%s (%s)" cider-version cider-codename)))

See https://lists.gnu.org/archive/html/emacs-devel/2021-12/msg02963.html for a discussion.

Steps to reproduce the problem

1. Checkout CIDER to a directory other than cider, e.g. git clone https://github.com/clojure-emacs/cider.git cider-other
2. checkout a commit which has the above code, e.g. git checkout 8a8ceb63f8549a9e4be05e7c649b96847a3ac5dc
3. byte compile cider-util.el: eldev compile -f cider-util.el --force

You should get the compilation warning mentioned earlier.

Environment & Version information

CIDER version information

8a8ceb63f8549a9e4be05e7c649b96847a3ac5dc

Emacs version

Emacs 27.2

Operating system

tested on Ubuntu and MS-Windows

I'm going to submit a placeholder PR on how to fix CircleCi error.

[bbatsov]

This is because the the call to (package-get-version) is expanded at compile time to nil (package-get-version is marked as pure and as such its value is resolved at compile time.

Nice research! I guess this also explains why spy-on is not stubbing the damn function properly. If the stubbing worked the actual behavior wouldn't have been that big of a deal. Perhaps we can just wrap this in another function that we can stub or copy it to cider-util.el and adjust it accordingly?

[bbatsov]

I've wrapped package-get-version in cider--pgk-version to solve the immediate problem with the tests. Down the road we can roll out a custom implementation of the version extraction.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!