purcell / package-lint

A linting library for elisp package metadata
GNU General Public License v3.0
192 stars 33 forks source link

Support the new compat library #227

Closed minad closed 10 months ago

minad commented 2 years ago

The compat package by @phikal provides backports of new APIs such that they can be used in packages which which are compatible with older Emacs versions. The list of backported APIs is given in the MANUAL but probably one would also need some automatic API extraction as is used for the Emacs APIs itself.

purcell commented 2 years ago

Nice, yes, that would quite take some thought to handle well, I think...

minad commented 2 years ago

Sure, there is no hurry. compat was just released a few days ago. I think @tarsius also plans to adopt the library in magit and transient. However at this point the library will automatically see widespread usage and I assume that more authors are going to adopt it in order to avoid adding their own compatibility code. Furthermore packages get earlier access to new API additions. It seems as if core library development has accelerated/improved recently.

phikal commented 2 years ago

I would be glad to help any way possible. If there is any format like the stdlib-changes file that you might need, it should be possible to write a script that extracts the necessary data from the current macros.

purcell commented 2 years ago

Thanks. It's certainly a useful initiative. I guess this is just for core functions etc., and won't work around the whole thing of people require-ing features that were only added to Emacs in a later version?

phikal commented 2 years ago

Steve Purcell @.***> writes:

Thanks. It's certainly a useful initiative. I guess this is just for core functions etc., and won't work around the whole thing of people require-ing features that were only added to Emacs in a later version?

It is possible (though slightly dangerous as it involves advising `require'), though as Compat isn't complete the only feature that is supported that way is subr-x for Emacs 24.3.

-- Philip Kaludercic

purcell commented 2 years ago

Does sound a bit scary, but I don't think you'd have much choice. You could make people use compat-require instead, potentially, with that being a no-op in Emacs 28.

phikal commented 2 years ago

Steve Purcell @.***> writes:

Does sound a bit scary, but I don't think you'd have much choice. You could make people use compat-require instead,

That is a good idea, I hadn't considered it as setting aside subr-x, no new features were implemented in Compat.

                                            potentially, with that

being a no-op in Emacs 28.

No-op, or just a regular `require'?

-- Philip Kaludercic

purcell commented 2 years ago

No-op, or just a regular `require'?

I (confusingly) meant "no-op" more as a direct pass-through, so yeah, just a regular require.

minad commented 1 year ago

@purcell

Thanks. It's certainly a useful initiative. I guess this is just for core functions etc., and won't work around the whole thing of people require-ing features that were only added to Emacs in a later version? Does sound a bit scary, but I don't think you'd have much choice. You could make people use compat-require instead, potentially, with that being a no-op in Emacs 28.

Just fyi, I recently took over as maintainer of Compat. I solved the require problem in Compat by making functions of missing features available unconditionally. In the case of text-property-search, the package developer has to do the following:

(require 'compat)
(require 'text-property-search nil t)

The special case of subr-x is not a problem anymore since I dropped support for 24.3, which was just too painful to maintain. I plan to create a new release soon, where this changes are included. The new release will also come with Emacs 29 functionality.

Could you please let me know what is needed to make package-lint understand Compat?

jdtsmith commented 1 year ago

I want to second the idea of linter support for checking whether or not call-compat is necessary for a given function or macro call, based on the emacs min version a package supports.

I'm not a frequent emacs linter user, but this would be I think a killer feature and take much of the tedium and arcane guesswork out of supporting older emacsen. From personal experience, the desire to use modern version capabilities but not leave older versions behind is a drag on package development.

minad commented 1 year ago

@jdtsmith Checking compat-call is not that important, given that these calls are explicitly marked with compat-call. You can easily validate them one by one yourself after grepping. compat-call is a nice and important feature of Compat, but it addresses an edge case.

More importantly package-lint needs to become aware of functions provided by Compat such that we don't get warnings for packages, which depend on Emacs 27 and Compat 29 and use Emacs 29 functions. This issue is mainly about that problem.

Furthermore if package-lint would check the arity of functions (#100), we could be sure that API changes are not missed. In such cases one could use compat-call if the corresponding extended definition is provided by Compat. But again, checking the validity of such compat-calls would be a secondary concern. Checking all normal function calls would yield the biggest benefit for the overall ecosystem.

jdtsmith commented 1 year ago

package-lint needs to become aware of functions provided by Compat such that we don't get warnings for packages

Right, of course... that goes without saying!

For the case I mentioned, I meant primarily checking for missing uses of compat-call. A common case: you're developing a package with emacs 27 as your working minimum version, then get requests to support emacs 26. With linter compat support, you could simply change the emacs min-version then check for new linter warnings.

I guess an automatic arity check would mean compat doesn't need to explicitly export a list of interface-changed "extended functions" by version, but to me that seems a small detail, since it's a relatively small number per emacs version.

tarsius commented 1 year ago

Adding cross-reference at https://github.com/melpa/melpa/issues/8361.

minad commented 1 year ago

@tarsius Didn't I already cross-reference this above? :)

EDIT: Ah, I see. There wasn't such a dedicated link below the MELPA issue.

purcell commented 1 year ago

Very open to adding support for this, but it's unlikely to be something that I can tackle myself in the near term.

purcell commented 10 months ago

Now that this has had some time to settle, and compat is becoming more successful, what do we think is the best way to add support here?

Given some sort of manifest of "things that are provided by compat", it should at least be possible for package-lint to not emit spurious warnings if people are using compat to provide functions/features, and maybe even suggest compat where appropriate.

minad commented 10 months ago

@purcell I can provide such a list. Should the list kept here or would it make more sense to extract this dynamically from Compat, such that we don't have to update the list? Extracting it is relatively easily. It is basically sufficient to grep the source for compat-defun/defmacro/defvar.

purcell commented 10 months ago

Should the list kept here or would it make more sense to extract this dynamically from Compat, such that we don't have to update the list?

Good question. I could update it periodically here in an Action like with stdlib-changes, or package-lint could just depend on compat and then find and inspect its source code. The latter sounds better, I guess, since compat will change more often than every emacs lib.

It is basically sufficient to grep the source for compat-defun/defmacro/defvar.

Ah nice. Features too?

purcell commented 10 months ago

I guess technically we'd only want hints based on the latest release version of compat, so that they're accurate for package authors making stable releases, but I probably wouldn't worry too much about that.

minad commented 10 months ago

Compat currently doesn't provide extra features, it only adds variables and functions missing in existing preloaded or autoloaded libraries.

When new libraries are added to Emacs we could also distribute them via Compat, but the current policy is that it is better to distribute them separately on ELPA (e.g. like seq or let-alist). Otherwise Compat would be blown up too much. I think the only small exception to this rule is text-property-search.

Regarding the function detection I guess it would be best to extract the list dynamically from the installed Compat if available. Downside is that this would not work if Compat is not available and that the Compat version may be outdated. On the other hand package-lint could simply depend on Compat.

For now I would be happy with a heuristic which works in most cases. Some of the fine details are not handled by package-lint anyway right now, e.g., function arity changes.

purcell commented 10 months ago

Downside is that this would not work if Compat is not available and that the Compat version may be outdated. On the other hand package-lint could simply depend on Compat.

Yeah, I figured we'd just add the dependency. And if the info was bundled into package-lint, then package-lint could also be outdated, so it's the same either way.

purcell commented 10 months ago

Initial version working in #254.

minad commented 10 months ago

Thanks, this is great!

purcell commented 10 months ago

One consequence is dropping support here for Emacs < 24.4, but I think that's bearable. It's going to break CI matrixes elsewhere, but ultimately it's not necessary to run package-lint with old Emacs versions — while one might compile and test a package on many emacsen, it only needs linting once.

purcell commented 10 months ago

Alrighty, merged and released, thanks all.