bbatsov / projectile

Project Interaction Library for Emacs
https://docs.projectile.mx
GNU General Public License v3.0
4k stars 583 forks source link

Add an explicit dependency on project.el #1883

Closed raxod502 closed 8 months ago

raxod502 commented 9 months ago

This change would resolve https://github.com/radian-software/straight.el/issues/1146. The issue is explained in detail in https://github.com/radian-software/straight.el/issues/1146#issuecomment-1949638006. I think it would be correct for projectile to declare an explicit dependency on project, since it uses the latter library. This is generally common practice for other packages that depend on built-in Emacs libraries that are also distributed on GNU ELPA, so that it can be communicated what the minimum version requirement is. It has the side effect that package managers like straight.el know the dependencies and can correctly determine whether the latest version should be installed, or whether to fall back to the built-in version.

I'm not sure what the minimum required version of project is. I set it to the first version that was tagged in GNU ELPA. By specifying a minimum version we reduce the chance of issues, because users of most package managers will be automatically prompted to upgrade to a supported version of project if their built-in version is too old. Please feel free to bump this version to a larger value if newer features of project are required.

(Will edit PR to address below checkboxes and fix CI.)


Before submitting a PR make sure the following things have been done (and denote this by checking the relevant checkboxes):

Thanks!

raxod502 commented 9 months ago

The tests pass using eldev for me locally, but they fail in CI while trying to install project as a dependency. I will look into why that would be the case; it seems like this package should be available: https://elpa.gnu.org/packages/project.html

It may be that eldev is hardcoded to only install from MELPA, and needs to be pointed also at GNU ELPA.

raxod502 commented 9 months ago

Yes, my assessment above was correct and adding GNU ELPA to the Eldev configuration file fixed that issue. However, we have to consider how to solve a new issue: even the oldest released version of project only supports Emacs 26.1 and above, while Projectile supports a minimum Emacs version of Emacs 25.1.

To be clear, project still existed in Emacs 25.1, but it was not made available as a package. As a result, declaring a formal dependency, rather than an implicit one, is what causes the minimum version requirement to be raised.

Is it perhaps okay to bump the minimum Emacs version for Projectile from 25.1 to 26.1? The latter version was released in May 2018, almost six years ago, and I suspect that previous versions are no longer widely used.

If it's still desired to support Emacs 25, then I can perhaps come up with some alternative solutions, but I think this would be the easiest way forward.

bbatsov commented 9 months ago

All of the usage of project.el are optional, though, so I don't think it makes sense to add a hard dependency on it.

As for the version of Emacs targeted by Projectile - it will get bumped eventually, but I usually do this only when there's some practical reason for this (e.g. we need some new built-in APIs or whatever). Probably after a release or two. We'll see.

raxod502 commented 8 months ago

Well, that is inconvenient for straight.el, but I understand your reasoning. I will find another way to fix the problem; interested parties can follow https://github.com/radian-software/straight.el/issues/1146#issuecomment-1963223373.