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

[Ihor Radchenko <yantar92@posteo.net>]

Max Nikulin <manikulin@gmail.com> writes:

>> 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.
> ...
> 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.

> 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. I do not like the idea of
using ~/.emacs.d/elpa default. It is fragile if this default ever
changes.

> 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.

> 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?
And make repro, which is dislike in particular - make repro is supposed
to be easy to use for users unfamiliar with Emacs internals or typical
GNU make conventions.

>> +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.

This sounds fragile. I see no reason to go this far and using so complex
approach.

>> +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.

I used $(EMACS) on purpose. $(BATCH) may contain more things, which we
do not want (on purpose) here.

>> 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.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>