Re: [PATCH v2] Add compat.el support to Org (was: [POLL] Use compat.el in Org? (was: Useful package? Compat.el))

[Max Nikulin]

On 13/04/2023 19:42, Ihor Radchenko wrote:
> Max Nikulin writes:
> > My expectation 
> > is that make does not attempt to manage dependencies. For me it is OK to 
> > type an additional command to install them and to fail otherwise.
>
> Sure. And you will have such option (EFLAGS).
> However, I decided to enable auto-downloading by default to not break
> the previous working compilation instructions.

For me adding external dependencies is strong enough reason to change 
compiling instructions. My vote is for clear separation of dependency 
management (even if performed through make targets) and 
compiling/testing/etc.

Last years I rarely use make directly to build software besides the 
cases when I am involved in development. I strongly prefer .deb 
packages. Earlier it was usual practice that "make" or "make all" failed 
if dependencies were not provided. Actually often it was configure that 
reported unavailable libraries. So network requests may be more 
surprising for users than failures due to missed dependencies. It is OK 
for e.g. gradle, but not for make to fetch packages. I expect that 
maintainers of packages from Linux distributions would prefer to avoid 
mixing of compiling and loading dependencies as well.

In my opinion, ideally there should be 3 options for dependency management:
1. Completely disabled. If load from default paths failed than it is a 
fatal error.
2. Use specified directory outside of Org tree (~/.emacs.d/elpa by 
default) or any other directory that you named pkgdir. Only dedicated 
target may clean this directory.
3. Install packages to Org source/build directory.

You decided to make 3 the default variant. I believe, it should be 
activated by a variable, e.g. AUTODEP = 1 in local.mk or from command 
line "make compile AUTODEP=1

I think, it is better to require an additional command

make autoloads
make fetch-dependencies
make compile

> +package-install = --eval '(unless (require '"'"'$(package) nil t) (message "%s" 
> load-path) (package-install '"'"'$(package)))'

I do not like that versions of dependencies are ignored. I have noticed 
`package-install-from-buffer'. Perhaps it can be used to generate a stub 
package (e.g. org-build-deps) with Package-Requires line obtained from 
org.el. The only purpose of this package is to pull dependencies. It is 
just an idea, I have not tried such approach.

> > $(FIND) $(pkgdir) -name \*.elc -delete
>
> Looks fine, except that now we have part of the targets using $RM
> variable and part of the targets ignoring it. I am not sure if it is a
> problem. I am slightly in favour of keeping the existing approach with
> $RM.

I have realized that -delete action is not a part of POSIX 
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/find.html
Anyway you removed this line and I like it since I suspected some issues 
when "make compile" is invoked twice.

> > Is there a way to express (or (compat "29.1.4.1") (emacs "28.1")) to
> > avoid installing compat in the case of sufficiently new emacs? E.g.
> > dpkg/apt allows such alternatives.
>
> No, AFAIK:

It is sour. Another deficiency of Emacs package management is that 
errors during compiling of dependencies do not cause immediate failure 
of make.

> +EMACS_VERSION := $(shell $(EMACS) -Q --batch --eval '(message "%s" emacs-version)' 
> 2>&1)

Ideally $(BATCH) should be used, but it is defined below. (princ 
emacs-version) is an alternative, but I have not idea which variant is 
better.

> -;; Package-Requires: ((emacs "26.1") (compat "29.1.4.1"))
> +;; Package-Requires: ((emacs "26.1") (compat "29.1.3.2"))

thanks

> Subject: [PATCH 3/7] Use compat.el library instead of ad-hoc compatibility
>  function set
>
> * mk/default.mk (EPACKAGES): Demand compat library during compile time.

when I asked for more granular commits I expected this change in

> Subject: [PATCH 2/7] org-compat: Enable compat.el

To separate adding dependency and replacing org-compat functions to compat.

Earlier I used the following procedure: git rebase -i, mark some commit 
for edit, git gui, unstage lines from commit, create new commit, reorder 
and squash/fixup commits on next iteration of rebase -i. Perhaps magit 
can do the same tricks as "git gui". If all changes of some file should 
be moved to another commit it should be easier to use git checkout 
COMMIT FILE.