bug#62762: 'make' often errors with "Org version mismatch" after pulling a new version of the code

[Max Nikulin <manikulin@gmail.com>]

On 05/05/2023 04:53, Stefan Monnier wrote:
>> First of all, I think, incremental builds are broken in Emacs.
> 
> Indeed, but in practice they usually work fine.
...
> Also, the use of `load-prefer-newer` in lisp/Makefile eliminates most of
> the brokenness we used to have in our incremental builds.

`load-prefer-newer' is a kludge as well. E.g. Python people migrated 
from comparison of timestamps to inscribing of .py file hash into byte 
compiled .pyc files. So if the hash in .pyc does not match .py content 
then .pyc file is recompiled or just ignored.

By the way, `load-prefer-newer' make things even worse in the case of 
`org-assert-version' when full dependency tree is not available. I have 
tried to add dependency for all lisp/org/*.elc files on 
lisp/org/org-macs.elc and lisp/org/org-version.el, but it is not enough. 
During recompilation `require' prefers .elc files (.el files not 
changed, so they are older) with inscribed stale `org-version' causing 
compilation error. If .el files were loaded, recompiling would be 
slower, but with up to date macro definitions. So in general, due to 
`load-prefer-newer' the chance to get stale macro during incremental 
build is higher than .el files are preferred. I admit, it is not common 
case when definition of a macro depends on another macro loaded from 
another file.

I am unsure if it is possible to express in make that all lisp/org/*.elc 
files must be removed on change of 
lisp/org/{org-version.el,org-macs.el}. It might alleviate the issue till 
implementation of properly dependency tracking.

>> `org-assert-version' is just the most apparent manifestation.
> 
> AFAIK `org-assert-version` tries to solve a different problem.

Of course `org-assert-version' was introduced to solve another problem, 
but it highlighted issues with dependency handling during incremental 
builds.

> Indeed,
> we recently introduced `org--inhibit-version-check` specifically so as
> not to use `org-assert-version` for Emacs's broken incremental builds
> (i.e. we decided we preferred that brokenness there).

`org--inhibit-version-check' is even uglier kludge that partially 
deactivated `org-assert-version' and just hides the issue with 
dependencies for incremental builds.

>> gcc supports generation of dependency files as a side effect of
>> preprocessing for decades, see
> [...]
>> Perhaps a similar trick may be done in elisp by advicing e.g. `load'.

Another idea: while single file is compiled per emacs invocation 
`load-history' may be compared before and after byte compilation.

> I can already warn them that an
> important problem will be the presence of cyclic dependencies.

In the C and C++ world the solution for cyclic dependencies is forward 
declarations. Some kind of such approach I see in Org as well. 
lisp/org/ol.el and lisp/org/org-element.el are mutually dependent. 
org-element.el requires 'ol, while the latter just declares functions 
from 'org-element.

When dependency files are generated, it is possible to create a report 
on cyclic dependency and to disentangle them.

>> Instead of dependency tracking Makefile in Emacs uses much more limited
>> approach with the main-first target.
> 
> Hmm... no, `main-first` doesn't have much to do with dependencies (and
> things like deciding when to *re*compile a file), it's concerned with
> compile-time performance (typically for the first compilation, when the
> `.d` dependencies wouldn't be available yet).

My impression is that main-first defines order of compilation. In this 
sense it servers for the same purpose as dependencies.

>> By the way, generated dependency map might help to properly reload a package
>> update without restarting of Emacs.
> 
> BTW, I was reminded recently that `load-history` keeps track of
> `require`s so we already have most of that info at hand.

I agree that heuristics based on `load-history' mostly works, but I 
suspect corner cases might exist. I have not inspected `org-reload' 
closely and what Emacs core offers instead.

> The idea was to check the shadowing only for one specific file, so as to
> try and keep the added cost in check.

That specific file must be checked for each file loaded from the updated 
directory. So hash O(1) should be better than scanning the list.

>> I am unsure that path comparison is able to detect a problem when a user
>> reloads Org after git pull and compiling new version.
> 
> If you mean `git pull; make` in Emacs's source code,

I mean reloading of updated Org from a clone of the org-mode repository 
to emacs installed as a system package (or as an independent custom 
built). A recent case where origin of the problem remained obscure:

Colin Baxter to emacs-orgmode. Why am I being told to use "straight.el"? 
Fri, 21 Apr 2023 10:42:27 +0100. 
https://list.orgmode.org/87ildpbmgs.fsf@yandex.com

Notice that mixed version loading may happen without changing of 
`load-path'. New org is loaded from the same directory.

> If you mean `git pull; make` in Org's source repository, then I must
> admit that I don't have much experience with it, but make Org's make
> file could set `load-prefer-newer`

Not enough, see beginning of the message.

> `my-require-with-shadow-check` is instead aimed at the case where the
> users try to load a mix of two different Org versions in the same Emacs
> sessions, either because of things like:
> - they're byte-compiling Org-2 in an Emacs that has Org-1 already loaded.

Does in help in the following case?
1. Base Org part is loaded on opening of some .org file.
2. Org in that directory is updated and recompiled.
3. New Org feature is loaded (autoloaded or by explicit call of e.g.
    (require ob-shell))

> I my own experience `git pull; make` in Emacs's source code has
> virtually never caused me trouble with Org files,

I recall issues after introducing of `org-encode-time' and finally Ihor 
replaced original macro to a less efficient function. However I am not 
sure what was the real reason:
- Some issue with conditional macro definition and native compilation
- Incomplete incremental rebuilds
- Macro was not properly defined so it was disappearing from byte 
compiled code.
However "undefined" complains might be similar to what I saw for 
`org-assert-version'.

>> In respect to incremental builds, `org-assert-version' is a disaster.
>> Any update requires full recompiling.
> 
> Indeed, I used to disable it locally (before `org--inhibit-version-check`).

By disaster I mean increased build time. `org--inhibit-version-check' 
allows mixed version compilation for macros unrelated to 
`org-assert-version', it is another disaster.

> Maybe a clear set of examples of the kinds of problems that
> `org-assert-version` aims to catch would be a good start.
> For me it's still not really clear.

- You are almost certainly going to ignore package.el issues in released 
Emacs versions including 28.
- Besides package.el there are other package managers. I have heard of 
weird issues with straight.el and org, but I have no links describing 
details.
- Some users just clone org-mode repository and add that directory to 
`load-path'. Directory remains the same after updates.
- Users tend to load some part of org before adding new version to 
load-path: explicit require, through dependencies, unsure if org file as 
startup screen may cause it as well
- User wish to update Org without restarting of Emacs. Recompiling may 
be called from Emacs (e.g. package.el) or by make or other scripts.

The goals:
- prevent mixed version compiling
- prevent mixed version loading
- error message should be clear for users explaining what actions they 
should perform for recovery from broken state.

I have not isolated an issue in Emacs-28 with package.el and difference 
with emacs binary is running from source tree or from install tree 
(.el+.elc vs. el.gz+.elc files in the directory with built-in Org and 
preference of .el vs. elc in require), so I can not tell if it affects 
other package managers in newer Emacs. It is the case when 
`org-assert-version' prevents loading of mixed compilation result, but 
user experience is terrible since simple recompilation does not help.