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.

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

Please make some adjustments to satisfy Melpa requirements #282

Closed
tarsius opened this issue Sep 28, 2024 · 9 comments · Fixed by #284
Closed

Please make some adjustments to satisfy Melpa requirements #282

tarsius opened this issue Sep 28, 2024 · 9 comments · Fixed by #284

Comments

@tarsius
Copy link
Contributor

tarsius commented Sep 28, 2024

Melpa requires that every package comes with a library whose name matches the name of the package. This package satisfies this basic requirement because elisp/edts/etds.el exists.

However, this file does have the expected content. Melpa extracts metadata from this library, i.e., it expects it to be the "main library" of the package. As a result the description of the edts package on Melpa is "Misc edts-related functionality" and a lot of metadata that is available for most packages is missing.

I am currently making some changes to package-build.el (the tool used to build the packages on Melpa) and that brought up an additional discrepancy:

package.el expects all elisp files to be located in the top-level directory of the package. This is not an absolute requirement and one can get around it, as you have done by (among other things) writing a rather complex Melpa recipe.

IMO it is okay that you do that, you probably have your reasons, but of course doing something that almost no other packages do, can lead to issues.

Due to some recent changes to package-build.el that "combine" the two rules (1) there must be a library matching the package name, and (2) libraries must be located at the top-level (except in very special cases), package-build.el now expects the "main library" to be located at the top-level.

I recommend that you address this like so:

  1. Rename elisp/edts/edts.el to something like elisp/edts/edts-misc.el.
  2. Add a new edts.el at the top-level, which contains all the required metadata.
    • Add a summary such as "Development tools for Erlang"
    • Add a Commentary: section with contents similar to the Introduction in the README.md.
    • Add library headers Author, URL, Keywords, Package-Version, Package-Requires and last but not least SPDX-License-Identifier.

Thanks!

@tarsius
Copy link
Contributor Author

tarsius commented Oct 20, 2024

Friendly ping!

When I opened this issue, seven packages by four maintainers existed, which did not provide a library that matches the package name. We are now down to five packages by only two maintainers. The other remaining maintainer has already responded, but so far isn't convinced.

I therefore intend to make the switch soonish, at which point packages that don't provide this file, will have to be removed from Melpa.

Cheers!

tarsius added a commit to melpa/melpa that referenced this issue Nov 15, 2024
Packages must provide a library that matches the package name and
while "edts.el" does not exist "edts-mode.el" does.  The maintainer
unfortunately is unresponsive, leaving us with no other option but
to rename.

sebastiw/edts#282.
@sebastiw
Copy link
Owner

sorry missed this message, looking into it now

@tarsius
Copy link
Contributor Author

tarsius commented Nov 18, 2024

Well, I probably should have tried to contact you via different channels before renaming the package on Melpa. But that happened only three days ago, so we can easily revert it.

@sebastiw
Copy link
Owner

I've done 1. Rename elisp/edts/edts.el to something like elisp/edts/edts-misc.el..

For 2 I guess that https://github.com/sebastiw/edts/blob/master/edts-pkg.el is not the way to do it anymore then, should I remove that file?

@sebastiw
Copy link
Owner

sebastiw commented Nov 18, 2024

@tarsius does this look ok?
#284

@tarsius
Copy link
Contributor Author

tarsius commented Nov 18, 2024

Uh... I just realized that your package does in fact already contain a library edts.el that matches the package name edts.

You are not really treating it as the "main file" but as a file that contains some "misc functionality". Melpa extracts metadata from that file, so you will have put the metadata in there. You might be able to get away with nothing else, but I will have to investigate. I might also have to make adjustments to my tooling to account for this package. At least I will have to figure out why that reported that there was no edts.el.

I'll rename edts-mode back to edts shortly, after investigating a bit more.

@tarsius
Copy link
Contributor Author

tarsius commented Nov 18, 2024

I'll rename edts-mode back to edts shortly, after investigating a bit more.

Actually, lets just do that first.

tarsius added a commit to melpa/melpa that referenced this issue Nov 18, 2024
@tarsius
Copy link
Contributor Author

tarsius commented Nov 19, 2024

I've taken a closer look and it turns out package-build.el does not only expect <name>.el to exist, but for it to end up at the root of the tarball.

(The file doesn't have to be located at the root of the repository, but when that is the case, then the recipe's :files must move it there. (If the files are located in lisp/ in the repository, then that does not have to be specified explicitly, because the default :files rules handle that case.)

I could start looking for the main file in subdirectories, but given that edts is the only package, which satisfies the first requirement but not the second, I think this would be overkill.

So even though I was mistaken about what exactly was going on, I still think you should go though with the pr you already opened.

does this look ok?

For the most part, yes, but some tweaks are still needed:

The summary becomes the short package description used on Melpa, so:

-;;; edts.el --- EDTS package declaration.
+;;; edts.el --- Erlang Development Tool Suite

(Also note that the convention is to not end these summaries with a period, much like headings in texts.)

The file must provide the matching feature:

+
+;;; Code:
+
+(provide 'edts)
+
+;;; edts.el ends here

(The convention that every library should end with "file ends here", is IMO rather silly, but as long as that is the convention, you might as well comply [at least when adding a new file, and in particular in the "main file"]. (The Emacs developers are actually working on lifting that requirement, but they appear to be doing it in a way that takes several major releases.))

tarsius added a commit to melpa/melpa that referenced this issue Nov 24, 2024
The Emacsmirror patches patches this package to fix the mainfile.

We should be able to revert this soon again,
see sebastiw/edts#282.
@tarsius
Copy link
Contributor Author

tarsius commented Nov 29, 2024

Thanks, I've updated the Melpa recipe again, to get this package directly from your repository again.

Please consider tagging 1.2.1 as well. Since the main library now contains the Package-Version header, please also bump that. It's important to bump the number in both "places" at the same time, i.e., create a "Release 1.2.1" commit, which bumps the embedded version string, and then tag that very commit with 1.2.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants