radian-software / prescient.el

☄️ Simple but effective sorting and filtering for Emacs.
MIT License
614 stars 25 forks source link

Adding to Non-GNU ELPA #138

Open okamsn opened 1 year ago

okamsn commented 1 year ago

I would like to try to get prescient.el and the extension packages added to NonGNU ELPA. I saw in https://github.com/radian-software/selectrum/issues/225 that ELPA won't happen due to the required copyright assignment.

If they are willing to add the packages, then I wonder about the Ivy and Selectrum extension packages.

The rest I would like to add.

@minad and @raxod502, do either of you disagree with this plan?

minad commented 1 year ago

I agree. prescient, vertico-prescient, corfu-prescient and company-prescient should be added. ivy-prescient should not be added due its maintainance status. selectrum-prescient should not be added since selectrum isn't in ELPA. Also selectrum has been superseded by vertico, which is already in ELPA.

raxod502 commented 1 year ago

If you'd like to add the package to NonGNU ELPA, I'm okay with that. I see there are some moral rules about packages in NonGNU ELPA, which I'm not comfortable extending to contributors here, but as long as upstream is comfortable doing their own work to fork the package so it fits their rules (if needed), I'm okay with that. I also don't want to be personally responsible for actually doing the maintenance work, since I've had pretty terrible experiences with the development and contribution workflows for FSF projects, but if you feel up to it, go for it!

Makes sense to me not to add Selectrum and Ivy integration packages for the reasons you and @minad said.

okamsn commented 1 year ago

I have started the thread here: https://lists.gnu.org/archive/html/emacs-devel/2022-11/msg01320.html

Since it treats the file version with the most recent version-number change as stable, it looks that I need to bump the minor version number so that the right version of "prescient.el" is grabbed. That way the Corfu and Vertico packages will work after I increment the dependency.

They are requesting separate branches for the stable release to reduce duplication on their end, so I can do that manually.

Depending on how development versions are tracked in ELPA, I might try making GitHub duplicate files in the "main" branch with other "fake-dev" branches and automatically add a date to the version number.

If they add it, then in the new branches, I will replace the Maintainer: (not Author:) e-mail address with my own, so that they don't try to contact you.

minad commented 1 year ago

@okamsn

They are requesting separate branches for the stable release to reduce duplication on their end, so I can do that manually.

Hmm, I would leave the Prescient repository as is, since MELPA already distributes the packages from this repository as is. Both ELPA and MELPA have the ability to produce packages from multi-package repositories, e.g., embark and embark-consult.

Note that NonGNU ELPA is automatically synchronized. Stable releases happen when you increment the "Version" keyword. See my Vertico/Corfu/Consult repositories for repositories which are automatically synchronized to ELPA.

okamsn commented 1 year ago

@okamsn

They are requesting separate branches for the stable release to reduce duplication on their end, so I can do that manually.

Hmm, I would leave the Prescient repository as is, since MELPA already distributes the packages from this repository as is. Both ELPA and MELPA have the ability to produce packages from multi-package repositories, e.g., embark and embark-consult.

My plan was to leave the main branch alone and to just duplicate the needed files on stable-for-ELPA branches, but we'll see what they say. I'll change nothing that affects MELPA.

Note that NonGNU ELPA is automatically synchronized. Stable releases happen when you increment the "Version" keyword. See my Vertico/Corfu/Consult repositories for repositories which are automatically synchronized to ELPA.

This is good news. Thank you for telling me.

minad commented 1 year ago

My plan was to leave the main branch alone and to just duplicate the needed files on stable-for-ELPA branches, but we'll see what they say. I'll change nothing that affects MELPA.

It is a bad plan. Note that one can also pull packages from (Non)GNU ELPA-devel, see https://elpa.gnu.org/devel/index.html. If you synchronize the ELPA branches separately and manually, this will be lost. I am strongly in favor of leaving things as is here. If the package should be distributed on ELPA, ELPA should have the ability to pull from a mono-repository, where multiple packages are maintained. MELPA also has this ability and it doesn't make sense for package authors to take the burden of accommodating the various package archives. In contrast, the package archives should adapt to the developers. See also the response by @tarsius on emacs-devel: https://lists.gnu.org/archive/html/emacs-devel/2022-11/msg01433.html. @tarsius response really hits the nail on the head. The Emacs developers accumulate more and more code in a single mono-repository since it is just so convenient and then expect that package authors split up their tiny package repositories.

okamsn commented 1 year ago

@minad, I understand what you are saying, but there is still the matter of making sure that raxod502 isn't contacted regarding the maintenance of the ELPA version of the packages. This means changing the Maintainer: description in the files for ELPA, yes? To do that, I would still be duplicating the files in another branch anyway.

Do you know of a different way to do it?

minad commented 1 year ago

You could simply remove @raxod502 from the Maintainer field in the main branch and instead add your email address there.

raxod502 commented 1 year ago

While changing the contact information for maintenance is great, at the end of the day if I am to maintain this repository then the responsibility ultimately falls on me to ensure that any conventions are maintained and kept up to date. I don't think it's sensible to maintain multiple different branches. Surely the ELPA folks can handle this on their end.

minad commented 1 year ago

@raxod502 The question is if the ELPA maintainance mails bother you. If not, you and okamsn could both be listed as maintainers.

Regarding the branch proposal, as I already wrote I strongly disagree. ELPA should be able to handle the multipackage repository as is. It already does for other packages, e.g.,. embark+embark-consult.

raxod502 commented 1 year ago

I think it is fine to leave the current email metadata. I can set up a filter to redirect or discard any relevant automated mail.

okamsn commented 1 year ago

In the e-mail thread, they proposed shipping all of the files (excluding the ignored ones, I assume) as a single package: https://lists.gnu.org/archive/html/emacs-devel/2022-12/msg00684.html

However, they ran into the issue that the byte compilation signals an error. They go on to propose not signalling an error if the require-ing fails, along with some other changes: https://lists.gnu.org/archive/html/emacs-devel/2022-12/msg00784.html

Stefan Monnier describes it:

Usually the way we expect this to work is:

  • the user install prescient
  • the user installs company/corfu/younameit
  • it just works

Sample (and incomplete) patch below (a lot of it is unrelated, e.g. I make it use add-function instead of manually saving the old function and restoring it, and I move the "turn off the mode" to the beginning instead of the weird recursive call which would inevitably mess up anyone using <foo>-prescient-mode-hook as well as confuse Custom's tracking of whether the mode was set (and whether pragmatically or not)).

I think that these changes are fine:

I would like to ask for opinions on the other changes:

minad commented 1 year ago

I think shipping the files as a single package is a good idea. My experience with integration packages is mixed at best. We should make sure that everything compiles cleanly even if the optional dependencies are not available and also remove the integration packages from MELPA.

minad commented 1 year ago

what are your thoughts on his using add-function for the Corfu sorting settings instead of storing the previous Corfu settings in variables?

I don't agree with this change. The function variables are there to be overridden. It is a bad idea to go via a second indirection (advice) on top. There is no advantage here if we use the advice mechanism. My recommendation is to leave this as is. But please check why Stefan Monnier proposed that. Maybe the setup/teardown wasn't written correctly. Note that they have to be idempotent.

okamsn commented 1 year ago

But please check why Stefan Monnier proposed that. Maybe the setup/teardown wasn't written correctly. Note that they have to be idempotent.

He said this:

Two main reasons:

  • It's simpler (less code).
  • It has a chance of working correctly if some other package also comes and installs its own function there.
minad commented 1 year ago

I looked into the Emacs codebase itself and it seems quite common to use add-function to do this. I find the pattern weird and I am not really convinced that there are advantages due to this approach. But I still think we should still use it due to the convention.

okamsn commented 1 year ago

I think shipping the files as a single package is a good idea. My experience with integration packages is mixed at best. We should make sure that everything compiles cleanly even if the optional dependencies are not available and also remove the integration packages from MELPA.

Where would that leave the unmaintained/obsolete extensions for Ivy and Selectrum? Do you think that they should be included in the multi-file package? On the one hand, I don't wish to imply support that might not exist, but on the other hand, if the additional UIs are not being installed as dependencies, then I see no immediate negative effect.

minad commented 1 year ago

Adding these too wouldn't hurt then. We can still deprecated them slowly one by one.

minad commented 1 year ago

@okamsn Any progress on this?

okamsn commented 1 year ago

@minad Not yet. I've been trying to finish a change in my own package that I started earlier than this issue's PR.

Since the changes they request are for packaging reasons instead of bug fixes or performance improvements, I've been prioritizing it less highly.

I still intend to see this done, just not as soon as I originally planned. In the meantime, please keep this issue and the WIP PR open. This is next on my list after I finish a group of changes on my own package.