`jsonrpc` version detection is not generic enough

[abougouffa]

Hello,

I'm using straight.el for package management,

Recently, dape is not usable on my installation because of these lines:

https://github.com/svaante/dape/blob/3cd8d4204753f411fad8ca1801de04566d2f6643/dape.el#L58-L60

The jsonrpc version detection code above supposes that the package has been installed using the builtin package.el. So, if the user uses any other package manager (like straight.el), the package-installed-p returns nil causing dape to fail even if we have the right jsonrpc version.

I've seen that jsonrpc doesn't include a jsonrpc-version variable, but I think you can just require assume that the user have the right jsonrpc version since this is explicitly declared in the package' dependencies:

https://github.com/svaante/dape/blob/3cd8d4204753f411fad8ca1801de04566d2f6643/dape.el#L11

[svaante]

This was introduced by request in #56

I reverted the commit as I believe it to be the lesser of two evils.

I am still not sure on how to solve the initial issue. Would somebody more savvy in the all the ways people install packages write something for an FAQ or something.

@timcharper ?? maybe??

[abougouffa]

Thank you for the quick response @svaante !

[timcharper]

I deleted my previous comments in which I opined that straight.el should polyfill package-installed-p because I thought about it for a while, and came to the conclusion I was wrong.

Packages should be able to assume the package manager does its job. Packages should declare version dependencies, but its up to the package manager to assert them, not the package.

Reverting the detection is the right call.

Perhaps feature detection and a warning is a better way to go.

The core issue here is when you install dape without a package manager (a la clone, add-to-list), you get nothing in terms of support for dependency resolution and version requirements). So, as a work-around, we felt the need to add the functionality in the package itself, but really, that was working around the core issue that dape is a package with dependencies that is being installed without a package manager.

Install without a package manager? Dependency enforcement is users job. Maybe we can make that clear.

[timcharper]

I think what compounds the problem is the fact that jsonrpc is merged in to core, but dape depends on a later version than that of core.

The next emacs will have a more recent jsonrpc bundled, and this problem will be greatly reduced.

[timcharper]

Thinking further, I think one good solution would be to enhance use-package when :load-path is specified. It could be augmented to load the package dependencies from the header `(package-buffer-info), and validate dependencies.

Having at least some warnings emit when requiring packages with broken dependencies would be useful.

Then that mechanism could have a generic way to suppress warnings on a package-by-package basis.

[timcharper]

Opened https://github.com/jwiegley/use-package/issues/1071 after thinking about this for sometime. I will continue to think on it. A lightweight layer that integrates well with Emacs core packaging infrastructure to handle parsing and checking dependencies for manually installed packages could be helpful and resolve the gap that led us to reach for this assertion in the first place.

[abougouffa]

I agree 100% with @timcharper, I hope use-package adds some basic dependency checking (it should be simple to implement).

As a workaround for dape, we can add something like this to get the version without relaying on package.el:

(when (version-list-<
       (with-temp-buffer
         (insert-file-contents (find-library-name "jsonrpc"))
         (package-desc-version (package-buffer-info)))
       '(1 0 24))
  (error "dape: Requires jsonrpc version >= 1.0.24, use `list-packages'\
 to install latest `jsonrpc' release from elpa"))

It is a bit ugly but does the job!