Bug? org-assert-version does not prevent mixed install

Bug? org-assert-version does not prevent mixed install

[Max Nikulin]

Hi,

It seems `org-assert-version' guard is not enough to protect against 
mixed install. Some code is required to generate a *fatal* error during 
compilation and a more descriptive error during loading.

I decided to look closer at
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59882
"30.0.50; Compilation of ELPA packages during version upgrade (Org 9.6 
ELPA upgrade from older ELPA version)"
since the bug lacks explicit and clear steps to reproduce.

Let's leave that bug for emacs' stuff and concentrate on Org code here.

I have tried the following:
- Emacs-27.1 from Ubuntu-22.04 LTS jammy (elpa-org-9.5.2 is not 
important). I suppose emacs version should be less than 29.0 to avoid 
built-in Org version having `org-assert-version'.
- Since emacs does not allow to specify just `user-emacs-directory' from 
command line I had to create a new user, so default configuration maybe 
slightly modified by debian-specific stuff.
- "emacs -l org" to ensure that Org is loaded. "-q" or "-Q" are 
intentionally avoided to allow initialization and saving of modification 
as for normal emacs session.
- M-x list-packages, click org, click install
- I get some warnings
> Compiling file /home/emcs/.emacs.d/elpa/org-9.6/ob-C.el at Sun Dec 18 04:30:49 2022
> Entering directory ‘/home/emcs/.emacs.d/elpa/org-9.6/’
> 
> In end of data:
> ob-C.el:509:1:Warning: the function ‘org-assert-version’ is not known to be
>     defined.
and even errors. I have not idea why severity is different
> Compiling file /home/emcs/.emacs.d/elpa/org-9.6/ob-matlab.el at Sun Dec 18 04:30:50 2022
> ob-matlab.el:43:1:Error: Symbol’s function definition is void: org-assert-version
- org.elc is not created as well.
- Despite of warnings and errors the following added to ~/.emacs
> (custom-set-variables
>  '(package-selected-packages '(org htmlize)))
- next attempt to run "emacs -l org" causes the famous message
> byte-code: Invalid function: org-assert-version

Is it possible to signal a fatal error during installing using emacs 
machinery to prevent broken user configuration? My expectation that 
result of failed compilation is ignored by emacs.

It is possible to convert the error to a better one by checking if 
`org-assert-version' is bound in org.el.

I think, the following topics are more suitable for emacs bug tracker or 
devel mail list:
- how to compile packages to avoid issues with already loaded older 
packages,
- what changes are required in package management to roll back failed 
attempt of installing.

Re: Bug? org-assert-version does not prevent mixed install

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

> Is it possible to signal a fatal error during installing using emacs 
> machinery to prevent broken user configuration? My expectation that 
> result of failed compilation is ignored by emacs.
>
> It is possible to convert the error to a better one by checking if 
> `org-assert-version' is bound in org.el.

We might do something like

(eval-and-compile (org-assert-version))

> I think, the following topics are more suitable for emacs bug tracker or 
> devel mail list:
> - how to compile packages to avoid issues with already loaded older 
> packages,
> - what changes are required in package management to roll back failed 
> attempt of installing.

Since you have a recipe to reproduce, could you please post it to Emacs
debbugs? If we can clearly demonstrate the problem to Emacs devs, there
is more chance that they can fix it. Ideally, in the coming Emacs 29
release.

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

Re: Bug? org-assert-version does not prevent mixed install

[Max Nikulin]

On 18/12/2022 20:04, Ihor Radchenko wrote:
> Max Nikulin writes:
> 
>> It is possible to convert the error to a better one by checking if
>> `org-assert-version' is bound in org.el.
> 
> We might do something like
> 
> (eval-and-compile (org-assert-version))

This will give obscure error during compiling since `org-assert-version' 
is not defined.

Unsure what will happen during load of the org.el file since org.elc is 
not produced. Perhaps the same cryptic error, but I can not explain it.

My idea (unchecked)

;; Remove when support of Emacs-29 is dropped.
(unless (fboundp 'org-assert-version)
   (error "Org is compiled or loaded while older version loaded already. 
  Please, ensure that no other org versions are loaded and recompile."))

Re: Bug? org-assert-version does not prevent mixed install

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

>> We might do something like
>> 
>> (eval-and-compile (org-assert-version))
>
> This will give obscure error during compiling since `org-assert-version' 
> is not defined.
>
> Unsure what will happen during load of the org.el file since org.elc is 
> not produced. Perhaps the same cryptic error, but I can not explain it.

Yes, but we will at least abort the compilation this way.

> My idea (unchecked)
>
> ;; Remove when support of Emacs-29 is dropped.
> (unless (fboundp 'org-assert-version)
>    (error "Org is compiled or loaded while older version loaded already. 
>   Please, ensure that no other org versions are loaded and recompile."))

I guess we can use the same error with what `org-assert-version' itself
throws:

(error "Org version mismatch.  Make sure that correct `load-path' is set early in init.el")

If you can, please check it. (AFAIU, you do have a setup where you can
reproduce the installation problem)

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

Re: Bug? org-assert-version does not prevent mixed install

[Max Nikulin]

On 18/12/2022 21:26, Ihor Radchenko wrote:
> Max Nikulin writes:
> 
>>> We might do something like
>>>
>>> (eval-and-compile (org-assert-version))
>>
>> This will give obscure error during compiling since `org-assert-version'
>> is not defined.
> 
> Yes, but we will at least abort the compilation this way.

You are right that it prevents creation of .elc files. Unfortunately 
`byte-recompile-directory' converts failure into a message instead of 
aborting package install completely.

When used consistently, it should cause load of Org completely 
uncompiled during next Emacs session. It is better than having 
incorrectly compiled files.

Notice that `org-assert-version' works for compiled files only, so if a 
user loads some third-party package (hyperbole?) that loads built-in Org 
and later adds new Org to `load-path' then we get an obscure error again.

Currently I am experimenting with minimal examples, not with full Org 
package, so I may confuse something.

>> ;; Remove when support of Emacs-29 is dropped.
>> (unless (fboundp 'org-assert-version)
>>     (error "Org is compiled or loaded while older version loaded already.
>>    Please, ensure that no other org versions are loaded and recompile."))

Another idea to have more concise code in each file is to put a new 
macro to a *new* file to avoid the issue with old org-macs.el without 
definition of the new macro `org-assert-version'. So each file should 
contain

(require 'org-assert-version-undefined)
(org-assert-version-undefined)

`org-assert-version' remains in org-macs.el. The goal is to generate an 
instructive error. The problem is that after fix in 
org-assert-version-undefined.el, an old version will be active for some 
users.

In addition I have noticed that e.g. org-matlab.el contains

      (require 'org-macs)
      (org-assert-version)

Shouldn't it be (eval-when-compile (require 'org-macs)) since no 
functions are used from this file? Some files have duplicated (require 
'org-macs).

Re: Bug? org-assert-version does not prevent mixed install

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

>>>> We might do something like
>>>>
>>>> (eval-and-compile (org-assert-version))
>>>
>>> This will give obscure error during compiling since `org-assert-version'
>>> is not defined.
>> 
>> Yes, but we will at least abort the compilation this way.
>
> You are right that it prevents creation of .elc files. Unfortunately 
> `byte-recompile-directory' converts failure into a message instead of 
> aborting package install completely.

> When used consistently, it should cause load of Org completely 
> uncompiled during next Emacs session. It is better than having 
> incorrectly compiled files.
>
> Notice that `org-assert-version' works for compiled files only, so if a 
> user loads some third-party package (hyperbole?) that loads built-in Org 
> and later adds new Org to `load-path' then we get an obscure error again.

I think that most reliable approach in this situation would be pulling
`org-assert-version' into a dedicated new file, similar to what you
suggested below. That way, we will not have feature cashes.

However, I am concerned about what is going to happen if wrong
org-version is defined during compilation. `org-assert-version' can
then be compiled with wrong org-version value and later produce similar
obscure error. Can you test such scenario using your Debian reproducer?

> In addition I have noticed that e.g. org-matlab.el contains
>
>       (require 'org-macs)
>       (org-assert-version)
>
> Shouldn't it be (eval-when-compile (require 'org-macs)) since no 
> functions are used from this file? Some files have duplicated (require 
> 'org-macs).

org-macs is pulled in by Org feature dependencies anyway. So, it does
not matter in practice. I also left the duplicates consciously to keep
the same pattern with `org-assert-version' usage.

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

Re: Bug? org-assert-version does not prevent mixed install

[Max Nikulin]

Ihor,

I am still puzzled why you can not reproduce messed version compilation 
issue. I can not figure out what I am missing. I think, the case below 
is easier than dealing with packages:

Consider the following file compile-broken.el where ~/src/org-mode
contains git main HEAD version:

(load "org")
(let ((org-dir "~/src/org-mode/lisp"))
   (add-to-list 'load-path org-dir)
   (byte-recompile-directory org-dir 0 :force))

remove lisp/*.elc files, run "make autoloads" and

     emacs -Q -l compile-broken.el

It causes messed version compilation.

Side note: On attempt to quit (C-x C-c) I get
org-clock-kill-emacs-query: Symbol’s function definition is void: 
org-clocking-buffer
and I had to use `kill-emacs' at this point. Exit hooks should not be so 
aggressive.

Next session demonstrates the issue:

     emacs -Q -L ~/src/org-mode/lisp/ -l org

There is a way to get Org completely broken. Let's simulate pulling of 
bundled org through some dependency and configuring new directory 
afterwards:

     emacs -Q  -l org --eval "(add-to-list 'load-path 
\"~/src/org-mode/lisp/\")" /tmp/test.org

Version mismatch is not detected, but RET causes

    eval-buffer: Symbol’s function definition is void: org-assert-version

I have no idea how to fight with such cases.

If you still can not reproduce with Emacs <= 28.1 then I may only 
suggest to check (locate-library "org-macs") and to remind about 
(list-load-path-shadows). I have no other ideas besides your environment 
is not clean and development Org version is available to Emacs.

On 28/12/2022 16:46, Ihor Radchenko wrote:
> Max Nikulin writes:
>>
>> Notice that `org-assert-version' works for compiled files only.
> 
> I think that most reliable approach in this situation would be pulling
> `org-assert-version' into a dedicated new file, similar to what you
> suggested below. That way, we will not have feature cashes.

I considered it before and discarded at first since when an Org version 
having such file is loaded then old version is used in the case of 
messed version compilation. As a result, modification of 
`org-assert-version' is ignored in the compiled version.

I reconsidered my opinion, though I still can not get really robust 
problem detection. Emacs-27.1 and its built-in Org version is assumed below.

Since `org-assert-version' has effect in compiled files only, we need 
some file with minimal dependencies that certainly will be compiled. It 
might be org-assert-version.el itself. However then it be required 
always rather than wrapped into `eval-when-compile' and should have 
(org-assert-version) call inside.

> However, I am concerned about what is going to happen if wrong
> org-version is defined during compilation. `org-assert-version' can
> then be compiled with wrong org-version value and later produce similar
> obscure error.

If an old Org version is not loaded then the warning appears, so the 
assertion works. A couple of issues:
- it does not suggest messed version compilation (reinstalling or 
removing *.elc files and recompiling)
- due to long text just last paragraph about straight may be visible to 
the user. Perhaps it is better to expand 
https://orgmode.org/worg/org-faq.html#mixed-install and use a more 
concise message with this link.

I have tried the following:
- move `org-assert-version' definition to org-assert-version.el, add 
(org-assert-version) call to this file to have at least one file compiled.
- add (require 'org-assert-version) before (org-assert-version) to other 
files.

The warning appears unless built-in Org is loaded before adding new 
version to `load-path'. So we may proceed this way.

Some details concerning compilation errors in the case of Emacs-27.1 and 
Org main:

- org.el is compiled (to my surprise)
- most of libraries not loaded before compilation, e.g. all ox-*.el 
files fails.

The reason is that they require org-elemnt that requires org-persist. 
The latter fails to call `org-file-name-concat' (that is missed in 
loaded org-compat) and breaks creation of the .elc file.

Finally, there is a way to not have org-assert-version.el loaded during 
regular sessions. Most of elisp files should have

(eval-when-compile (require 'org-assert-version))
(org-assert-version)

It should ensure latest `org-assert-version' definition during 
installing of a new Org version. The issue is that compilation of 
arbitrary file may fail and I am unsure if it is reasonable to introduce 
another file to ensure compiled variant and to require it everywehere

(eval-when-compile (require 'org-assert-version))
(org-assert-version)
(require 'org-assert-version-call)

with org-assert-version-call.el

(eval-when-compile (require 'org-assert-version))
(org-assert-version)
(provide 'org-assert-version-call)