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

[Max Nikulin <manikulin@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 5189 bytes --]

Ihor,

I do not like lengthy emacs commands and make functions to generate 
them. I think, it is better to move such code to a script. A proof of 
concept is attached, however it is rather rough draft

     ./epm.el -Q --epm-dir $(emacs_pkgdir)/emacs-%e install compat
     ./epm.el -Q -L ~/src/compat install compat

or

     export EPMDIR="$HOME/.cache/epm/emacs-%e"
     ./epm.el install compat

to just abort compilation due to absent dependency

     ./epm.el missing compat

On 20/04/2023 16:27, Ihor Radchenko wrote:
> Max Nikulin writes:
> 
>>> Sure. And you will have such option (EFLAGS).

By the way, accordingly to (info "(make) Command Variables") or (info 
"(standards) Command Variables") "Variables for Specifying Commands"
https://www.gnu.org/prep/standards/html_node/Command-Variables.html

it should be EMACSFLAGS rather than EFLAGS.

>> 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.
> 
> I have no problem with this approach when using system packages.
> However, it is almost guaranteed that compat.el is absent in global
> load-path as long as compat.el is not built-in.

I see that installation attempt is not performed when packages are 
available. However form my point of view it is normal when compilation 
fails when dependency are not provided. It works so for decades for 
applications that use make. To be precise, usually I expect detection of 
missed libraries from configure scrips, but in some cases they are 
missed. Maybe such experience was formed when access to network was limited.

For me it is quite natural that make does try to pull dependencies (at 
least by default) and it is my responsibility to ensure availability of 
necessary libraries.

>> 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.
> 
> This is mostly an equivalent of -L switch.

No, -L is for source directories of package (e.g. git repositories). I 
mean namely alternative `package-user-dir', but not managed by make.

> I do not like the idea of
> using ~/.emacs.d/elpa default. It is fragile if this default ever
> changes.

I still consider it as a reasonable default for a user having just one 
emacs version who is going to build and run org. Both steps may use the 
same package directory. A developer who switches between various emacs 
versions may have set of packages for each emacs version.

>> 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
> 
> It is now activated by EPACKAGES being non-empty.

And it is non-empty by default because it defines list of build 
dependencies, not whether they should be managed by make.

>> I think, it is better to require an additional command
>>
>> make autoloads
>> make fetch-dependencies
>> make compile
> 
> Maybe. Then, also make doc and make install?

In general "make install" may be executed by root while "make all" is a 
task for regular user. "make doc" is an optional step, so I do not see 
any problem. Ideally it might be

make fetch-dependencies # or specify package directory
# or load path in local.mk
make all

followed by optional doc or install

> And make repro,

I have not justified my point of view to make repro yet.

>> 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.
> 
> This sounds fragile. I see no reason to go this far and using so complex
> approach.

My idea is to ensure that *required* version is installed, not some 
stale one. I have not tried such approach though.

>>> 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.
> 
> For me, PATCH 3/7 grouping is more reasonable. So, I disagree.
> Splitting EPACKAGES modification would create transient commit with
> non-working Org.

I just do not like that a single line change in default.mk (modification 
of build process) is buried in a large patch (changes of the code). My 
idea was that

> -EPACKAGES ?=
> +EPACKAGES ?= compat

should be in the same commit as

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

Currently patch 2 requires compat, but it is provided till patch 3. 
Despite in commit 2 the package does not do anything useful, I 
considered this commit as preparations to actively use introduced 
dependency.

[-- Attachment #2: epm.el --]
[-- Type: text/x-emacs-lisp, Size: 3982 bytes --]

#!/bin/sh
":"; # -*- mode: emacs-lisp; lexical-binding: t; -*-
":"; exec emacs --script "$0" "$@"

(require 'format-spec)
(require 'package)
(require 'subr-x)

(defvar epm-dir nil
  "Overrides `package-user-dir'.")

(defvar epm-verbose nil)

(defun epm-nonempty-p (s)
  (and s (not (string-empty-p s))))

(defun epm-init ()
  (unless (epm-nonempty-p epm-dir)
    (setq epm-dir (getenv "EPMDIR")))
  (when (epm-nonempty-p epm-dir)
    (let* ((fmt-expanded (format-spec epm-dir `((?e . ,emacs-version))))
	   (dir (directory-file-name (expand-file-name fmt-expanded))))
      ;; `package-user-dir' ~/.emacs.d/elpa by default
      ;; `package-directory-list' does not include it
      (setq package-user-dir dir)))
  (package-initialize))

(defun epm-library-unavailable-p (lib)
  (unless (locate-library lib)
    lib))

(defun epm-missing (libs)
  ;; TODO consider `require' catching load errors
  (delq nil (mapcar #'epm-library-unavailable-p libs)))

(defun epm-cmd-help (_cmd _args)
  "List commands."
  (princ "Usage: epm [--dbg|--debug-on-error] [--epm-dir] COMMAND ARGS...

CLI tool to install ELPA packages.

Any Emacs option may be specified, e.g. --quck,-Q or --directory,-L DIR

--dbg, --debug-on-error
    Enable `debug-on-error'

--epm-dir DIR
    Set `package-user-dir'.
    \"%e\" is replaced by `emacs-version'.
    Alternatively EPMDIR environment may be specified.
\n")
  (pcase-dolist (`(,name . ,func) epm-commands)
    (princ (concat name "\n"))
    (princ
     (replace-regexp-in-string
      "\\`\\|\n" "\\1    "
      (documentation func) 'fixedcase nil))
    (princ "\n\n")
    ))

(defun epm-cmd-missing (_ libs)
  "Report not installed libraries and exit with non-zero code."
  (let ((missing (epm-missing libs)))
    (when missing
      (princ (mapconcat #'identity missing " "))
      (princ "\n")
      (kill-emacs 1))))

(defun epm-cmd-install (_ libs)
  "Install packages from LIBS that are not available yet"
  ;; TODO force option or update command
  (let ((missing (epm-missing libs)))
    (when missing
      (package-refresh-contents)
      (make-directory package-user-dir 'parents))
    (dolist (pkg missing)
      (package-install (intern pkg)))))

(defun epm-cmd-report (_ libs)
  "Report paths of available libraries"
  (princ (format "package-user-dir: %s\n" package-user-dir))
  ;; (princ (format "load-path: %s\n" load-path))
  (dolist (name libs)
    ;; (version-to-list version)
    (princ (format "%-20s %s " name
		   (if (package-installed-p (intern name))
		       "package "
		     "        ")))
    (princ (locate-library name))
    (princ "\n")))

(defvar epm-commands
  '(("help" . epm-cmd-help)
    ("install" . epm-cmd-install)
    ("missing" . epm-cmd-missing)
    ("report" . epm-cmd-report)))

;; Perhaps there is a way to use `command-switch-alist'.
(defun epm-args-parse (arg-list)
  (let ((parse-opts t)
	unprocessed
	cmd)
    (while (and arg-list (or parse-opts (not cmd)))
      (pcase (pop arg-list)
	("--"
	 (setq parse-opts nil))
	((and (guard parse-opts) ;; otherwise processed after script exit
	      (or "-L" (pred (lambda (x) (string-prefix-p x "--directory")))))
	 (push
	  (expand-file-name (command-line-normalize-file-name
			     (pop arg-list)))
	  load-path))
	((and (guard parse-opts) "--epm-dir")
	 (setq epm-dir (pop arg-list)))
	((and (guard parse-opts) (or "--dbg" "--debug-on-error"))
	 ;; -d is handled as --display, --debug as --debug-init
	 (setq debug-on-error t))
	((and (guard (not cmd)) (pred (string-match-p "\\`[^-]")) arg)
	 (push arg cmd)
	 (unless parse-opts
	   (push "--" cmd)))
	(arg
	 (if cmd (push arg cmd) (push arg unprocessed)))))
  (cons (nreverse cmd) (nreverse unprocessed))))

(pcase-let ((`(,cmd . ,unprocessed) (epm-args-parse command-line-args-left)))
  (unless (setq command-line-args-left unprocessed)
    (epm-init)
    (let ((func (cdr (assoc (car cmd) epm-commands))))
      (if func
	  (funcall func (car cmd) (cdr cmd))
	(error "Unknown command %s" (car cmd))))))