Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add an explicit dependency on project.el #1883

Closed
wants to merge 1 commit into from
Closed

Add an explicit dependency on project.el #1883

wants to merge 1 commit into from

Conversation

raxod502
Copy link
Sponsor Contributor

@raxod502 raxod502 commented Feb 17, 2024 •
edited

This change would resolve radian-software/straight.el#1146. The issue is explained in detail in radian-software/straight.el#1146 (comment). 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):

  • The commits are consistent with our contribution guidelines
  • You've added tests (if possible) to cover your change(s)
  • All tests are passing (eldev test)
  • The new code is not generating bytecode or M-x checkdoc warnings
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the readme (if adding/changing user-visible functionality)

Thanks!

@raxod502
Copy link
Sponsor Contributor Author

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
Copy link
Sponsor Contributor Author

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
Copy link
Owner

bbatsov commented Feb 17, 2024

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
Copy link
Sponsor Contributor Author

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 radian-software/straight.el#1146 (comment).

@raxod502 raxod502 closed this Feb 26, 2024
@raxod502 raxod502 deleted the rr-dependency-project branch February 26, 2024 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can't use eglot with latest Emacs & straight, due to project.el location
2 participants
@raxod502 @bbatsov