melpa / package-build

Tools for assembling a package archive
https://github.com/melpa/melpa
25 stars 34 forks source link

Include commit info in built packages #35

Closed purcell closed 4 years ago

purcell commented 4 years ago

Posting here for feedback: this change ensures the source commit (if any) is included in the -pkg.el file for multi-file packages, and the main .el file for single-file packages.

I think it should be fine, and I've tested it with single and multi-file packages, but I'm wondering:

tarsius commented 4 years ago

The only functions that cask uses even though we consider them internal are package-build--checkout and package-build--package. You aren't modifying any of those, so we should be good.

Package-Commit seems good.

But here's an idea I've been carrying around for a while: Why don't we just give up on "single-file" packages altogether? I think it would be better to just always ship a tarball and to put the metadata in PKG-pkg.el instead of modifying PKG.el.

purcell commented 4 years ago

The only functions that cask uses even though we consider them internal are package-build--checkout and package-build--package. You aren't modifying any of those, so we should be good.

Package-Commit seems good.

Thanks, I'll go ahead and merge, and also update the subtree in MELPA.

But here's an idea I've been carrying around for a while: Why don't we just give up on "single-file" packages altogether? I think it would be better to just always ship a tarball and to put the metadata in PKG-pkg.el instead of modifying PKG.el.

Yeah, it's an idea worth considering. Curious, though: why do you think it would be better? I mean, obviously it may be simpler to have a single code path instead of two, but is there something more to it than that? Generally I've encouraged people to avoid -pkg.el files in their source trees (particularly for single-file packages, and partly due to -pkg.el files' current inaccessibility to package-lint, though see https://github.com/purcell/package-lint/issues/111#issuecomment-569454640), so it would be a bit weird to favour those strongly in the built packages.

tarsius commented 4 years ago

A single code path would be nice but I am afraid we wouldn't be able to remove the single-file variant because other users of this package would want to keep using that.

Generally I've encouraged people to avoid -pkg.el files in their source trees [...] so it would be a bit weird to favour those strongly in the built packages.

I guess it depend on why you recommend that. I always assumed you did so because we generate that file and thus override whatever (potentially better) information the package author has put there themselves. If there is no *-pkg.el to begin with, then authors won't be surprised if it gets "replaced".

So maybe we should even consider this an argument in favor of going tarball-only.

purcell commented 4 years ago

I guess it depend on why you recommend that. I always assumed you did so because we generate that file and thus override whatever (potentially better) information the package author has put there themselves. If there is no *-pkg.el to begin with, then authors won't be surprised if it gets "replaced".

That was partly it, yes, but also package-lint doesn't give feedback on -pkg.el files, so encouraging people to use package-lint causes them to put their metadata in the main .el file anyway, and I often tell people to avoid repeating themselves in a -pkg.el file.