emacs-eldev / eldev

Elisp development tool
https://emacs-eldev.github.io/eldev/
GNU General Public License v3.0
227 stars 17 forks source link

Eldev thinks my package needs a higher Emacs version but won't tell me why #29

Closed DarwinAwardWinner closed 3 years ago

DarwinAwardWinner commented 3 years ago

I am the maintainer of the amx package, which requires Emacs 24.4 or higher:

;; Package-Requires: ((emacs "24.4") (s "0"))

However, when Eldev runs in my CI tests, It complains:

Emacs version 25.1 is required (this is version 24.5.1)
Required by package `amx'

The reason for this is that one of the optional dependencies, selectrum, requires Emacs 25.1 or higher, so this is the version that Eldev demands. That all makes sense, but the problem is that Eldev gives no easy way to determine why it is demanding a higher version. I had to grep recursively for Package-Requires to discover the problem. I think it would be good for Eldev to check for consistency between the Emacs versions required in the package being tested and its dependencies. Specifically, it should probably be at least a warning, maybe an error, if any dependency demands a higher Emacs version that what is declared in the package itself. And it should probably also be a warning/error if there are multiple package files demanding different Emacs versions required. If that is not practical, then ideally the above error message should at least give some indication of where the Emacs version requirement came from, to aid in debugging the issue.

(In the specific example, the underlying problem is that my package does support 24.4, but an optional dependency doesn't, and I haven't yet figured out how to explain this to Eldev and/or my test suite.)

DarwinAwardWinner commented 3 years ago

Hmm, I may have spoken too soon. I temporarily commented out the offending dependency in Eldev and I'm still getting the same error, so I'm actually not sure why Eldev thinks my package needs Emacs 25.1.

doublep commented 3 years ago

Committed a fix, thank you. Will be released in 0.8, hopefully soon.

To test what exactly requires Emasc 25 you can also use Eldev:

~/git/amx$ eldev dtree -b test
amx 3.3
    emacs 24.4    [built-in]
    s (any)    [20180406.808 installed]
selectrum (any)    [for ‘test’; 20210118.1256 installed]
    emacs 25.1    [built-in]
helm (any)    [for ‘test’; 20210114.1521 installed]
    emacs 25.1    [built-in]
    async 1.9.4    [20210117.718 installed]
        emacs 24.3    [built-in]
    popup 0.5.3    [20210108.1821 installed]
        cl-lib 0.5    [built-in]
    helm-core 3.6.5    [20210117.951 installed]
        emacs 25.1    [built-in]
        async 1.9.4    [repeated, see above]
smex (any)    [for ‘test’; 20151212.2209 installed]
    emacs 24    [built-in]
ivy (any)    [for ‘test’; 0.13.1 installed]
    emacs 24.5    [built-in]
ido-completing-read+ (any)    [for ‘test’; 20210103.1621 installed]
    emacs 24.4    [built-in]
    seq 0.5    [built-in]
    cl-lib 0.5    [built-in]
    memoize 1.1    [20200103.2036 installed]
with-simulated-input (any)    [for ‘test’; 20200509.2010 installed]
    emacs 24.4    [built-in]

So, it is Helm that causes problems. You can probably modify file Eldev to not add it as an extra dependency on old Emacsen, though maybe some changes for tests would be required too.

DarwinAwardWinner commented 3 years ago

Thanks! With this change I was able to get my tests running on Emacs 24, with my test suite simply skipping the tests involving optional dependencies that don't support Emacs 24: https://github.com/DarwinAwardWinner/amx/commit/31aff0533e22a8ab3ba5538e0e56dbf697dfe664

DarwinAwardWinner commented 3 years ago

I guess one nice-to-have feature would be a way to explicitly declare an optional dependency, which Eldev will try to install but then keep going regardless of whether the install succeeds or fails. That would allow me to skip the version checks. And then you could also have a command-line option to disable all optional dependencies (e.g. in order to test that the package's base functionality doesn't have any unintentional hard dependencies on optional packages). But like I said, this is just nice-to-have. Most packages developers probably don't need anything like this.

So instead of:

(when (version<= "25.1" emacs-version)
  (eldev-add-extra-dependencies 'test 'helm))

I could say:

(eldev-add-extra-optional-dependencies 'test 'helm)

And then when testing on Emacs 24, Eldev would see that helm can't be installed and just skip it and keep going.

doublep commented 3 years ago

Sounds like a useful feature. Could you maybe open an issue for it?