Adopt makem.sh for build support (linting, running tests)

[bcc32]

This PR switches from CMake and custom make rules to using makem.sh, as suggested in #423.

~~It also includes a small tweak to just disable ledger-mode/test-001 (which requires running Emacs interactively) when makem.sh runs tests, since it does not provide a custom ERT selector. That change can be removed once #431 is merged.~~ #431 was merged, so that change was removed upon rebase.

Fix #423.

[bcc32]

Discovered some issues on macos, opening alphapapa/makem.sh#49

[bcc32]

Updated makem.sh to include the macOS fix.

[purcell]

I'd be generally in favour of this, because I'm not sure if any of the more active maintainers have any specific love for cmake, and it would be good to include more linting etc. in the CI.

[Thaodan]

Isn't this just replacing one big blob (well a shell script isn't technically a blob but this one is very big) with another? Compared to that a Makefile is much shorter.

If this adapted I would recommend a submodule instead copying the file into the repository to make it easier to track the script.

[purcell]

Isn't this just replacing one big blob (well a shell script isn't technically a blob but this one is very big) with another? Compared to that a Makefile is much shorter.

I'm a big Makefile fan, but the setup here is pretty overbuilt for current needs, misses common elisp build steps like linting, and none of the maintainers here are likely to want to add it. Meanwhile makem is fairly well used and actively supported, which is why I'm broadly in favour of the change. No other maintainers have expressed a preference either way.

If this adapted I would recommend a submodule instead copying the file into the repository to make it easier to track the script.

That would be fine too, though I don't know if the script has to end up in the root of the repo to work properly.

[Thaodan]

Steve Purcell @.***> writes:

purcell left a comment (ledger/ledger-mode#432)

Isn't this just replacing one big blob (well a shell script isn't technically a blob but this one is very big) with another? Compared to that a Makefile is much shorter.

I'm a big Makefile fan, but the setup here is pretty overbuilt for current needs, misses common elisp build steps like linting, and none of the maintainers here are likely to want to add it. Meanwhile makem is fairly well used and actively supported, which is why I'm broadly in favour of the change. No other maintainers have expressed a preference either way.

Personally I hope that the classical

make make install

still works.

CMake can handle test cases just fine but I get why it might not be the right tool when there's no direct CMake Elisp package implementing all the features.

If this adapted I would recommend a submodule instead copying the file into the repository to make it easier to track the script.

That would be fine too, though I don't know if the script has to end up in the root of the repo to work properly.

A symlink could work.

[purcell]

Personally I hope that the classical make; make install still works.

Seems very reasonable!

[bcc32]

I think makem.sh doesn't support installing the Elisp files into site-lisp (a la make install).

However, I think this is an acceptable removal. make install and things like it are not generally correct ways to install Elisp packages, as it doesn't do things like regenerate your autoloads file (see documentation for package-quickstart-file) or update package-selected-packages. You should instead use package-install-file from Emacs to ensure it is done correctly.

make still works fine.

[bcc32]

If this adapted I would recommend a submodule instead copying the file into the repository to make it easier to track the script. That would be fine too, though I don't know if the script has to end up in the root of the repo to work properly. A symlink could work.

Symlinks don't work reliably on Windows, AFAIK.

In the year since I first created this PR, no new release of makem.sh has occurred, and it's unlikely to change very frequently. And, indeed the upstream repo recommends just copying the script into your project instead (whereas adding it as a submodule would force downloaders of ledger-mode to also download makem.sh's tests and documentation images).

(Note, also, that submodules are not included in git clone by default---meaning, users would have to take some special steps with fairly advanced git commands just so they can run a standalone bash script.)

As a result, IMO, it's not worth the hassle to try to use a submodule.

[Thaodan]

Aaron Zeng @.***> writes:

I think makem.sh doesn't support installing the Elisp files into site-lisp (a la make install).

However, I think this is an acceptable removal. make install and things like it are not generally correct ways to install Elisp packages, as it doesn't do things like regenerate your autoloads file (see documentation for package-quickstart-file) or update package-selected-packages. You should instead use package-install-file from Emacs to ensure it is done correctly.

I don't think that's a fair argument. Packages can come from distribution packages, it's a common practice. Further package-install-file depends on package.el while site-lisp just works with any Emacs package manager.

[bcc32]

I don't think that's a fair argument. Packages can come from distribution packages, it's a common practice.

On the other hand, it's very uncommon practice for Elisp packages to provide their own installation command, though (I have never seen a package do this other than mu4e, which is an extraordinary cas c because it must be installed with the mu program at the same version).

If a distribution has a standard way of packaging and installing Elisp packages, it's should just byte-compile, generate autoloads, etc. the way it usually does. I don't think ledger-mode devs have any special insights on the right way to prepare such a package for every platform.

Further package-install-file depends on package.el while site-lisp just works with any Emacs package manager.

Yes, that's true of package-install-file specifically, but surely other package managers can install ledger-mode from a checkout, or from a tarball off of MELPA, no? (Genuine question, since I have never used an Emacs package manager other than package.el). My point is that those package managers may have some steps they want to take in addition to the simple copying of files to some hardcoded path, and we shouldn't second guess them. It does not appear to be typical practice in the ecosystem of Elisp packages I have seen.

[Thaodan]

Aaron Zeng @.***> writes:

bcc32 left a comment (ledger/ledger-mode#432)

I don't think that's a fair argument. Packages can come from distribution packages, it's a common practice.

On the other hand, it's very uncommon practice for Elisp packages to provide their own installation command, though (I have never seen a package do this other than mu4e, which is an extraordinary cas c because it must be installed with the mu program at the same version).

It's not uncommon even autotools provides some handling for that. It's just not as common anymore for new packages.

If a distribution has a standard way of packaging and installing Elisp packages, it's should just byte-compile, generate autoloads, etc. the way it usually does. I don't think ledger-mode devs have any special insights on the right way to prepare such a package for every platform.

The package is the same for every platform just the exact path to site-lisp is slightly different.

Further package-install-file depends on package.el while site-lisp just works with any Emacs package manager.

Yes, that's true of package-install-file specifically, but surely other package managers can install ledger-mode from a checkout, or from a tarball off of MELPA, no? (Genuine question, since I have never used an Emacs package manager other than package.el). My point is that those package managers may have some steps they want to take in addition to the simple copying of files to some hardcoded path, and we shouldn't second guess them. It does not appear to be typical practice in the ecosystem of Elisp packages I have seen.

Of course they can just the common path when you install packages from the distribution is using site-lisp which just works without any involvement of the package manager. The other option to use Elpa is to use package-directory-list which requires that the package manager can deal with package.el's package directories.

Anyway the point is just to not break existing workflows, backwards compability and such.

[bcc32]

If a distribution has a standard way of packaging and installing Elisp packages, it's should just byte-compile, generate autoloads, etc. the way it usually does. I don't think ledger-mode devs have any special insights on the right way to prepare such a package for every platform.

The package is the same for every platform just the exact path to site-lisp is slightly different.

Right, but what I'm saying is, if a distribution knows how to deal with typical Elisp packages by just copying them into site-lisp, it can surely handle a package not having a make install command as the vast majority of Elisp packages don't. So I don't think having the make install command provides much value to distributions.

Yes, that's true of package-install-file specifically, but surely other package managers can install ledger-mode from a checkout, or from a tarball off of MELPA, no? (Genuine question, since I have never used an Emacs package manager other than package.el). My point is that those package managers may have some steps they want to take in addition to the simple copying of files to some hardcoded path, and we shouldn't second guess them. It does not appear to be typical practice in the ecosystem of Elisp packages I have seen.

Of course they can just the common path when you install packages from the distribution is using site-lisp which just works without any involvement of the package manager. The other option to use Elpa is to use package-directory-list which requires that the package manager can deal with package.el's package directories.

So, my question is, who is the make install command for? Whose workflow would we be breaking by removing it? I think it's not for distributions, as mentioned above. I think it's not for typical users, who will download this package from MELPA via package.el or an alternative package manager. And for the case of users who install Elisp packages from source, most Elisp packages do not provide a Makefile at all, and certainly not a make install command, so they should be accustomed to using M-x package-install-file or their favorite package manager's equivalent, no?

We don't even advertise the make install command in the README or CONTRIBUTING docs--- the documented installation procedure is to use MELPA or the bare-bones "add this to load-path".

Anyway the point is just to not break existing workflows, backwards compability and such.

Yes, backwards compatibility is important. But, in my opinion, this pretty niche workflow, that other Elisp packages do not make any attempt to provide, is not worth blocking a clear improvement to the development experience and maintainability of this project.

I'll let @purcell make the call here, if he thinks keeping make install is important, as he is more familiar with the history and conventions of this project in particular than I am.

I will add that of course, one could add a make install command after this PR, but I have no idea how it would work and also don't run any of the systems that would be amenable to testing such a command myself. (I run Emacs from nix, where my only distribution-provided package is mu4e. It is installed via a wrapper around package-unpack from package.el, simply by setting package-user-dir to the desired output directory.)