Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-06-04 Thread Max Nikulin

On 10/04/2023 23:58, Ihor Radchenko wrote:

Max Nikulin writes:

Currently I do not understand:
- Why presence of .el files in the same directory (old Org version) with
.elc files affects result of compilation (at least for Emacs-28)?


I may be missing your point, but `load-prefer-newer' maybe?


I mean installing ELPA Org package to Emacs < 29 when another Org 
version is loaded. The result is different when Emacs is running from 
source tree (raw org .el sources are available) or from install tree 
(lisp files are compressed to .el.gz). In the former case Org is 
compiled correctly. In the latter case compressed sources causes 
mixed-compilation issue with following failures to load Org. I think, it 
is a reason why you were unable to reproduce mixed-compilation issue on 
Gentoo.


The following commit

https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9dfd945a2c2
Fix byte compilation of package built-ins
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=49708
(no discussion in the bug report)

introduced `package--reload-previously-loaded` instead of 
`package--reload-previously-loaded` and `package--list-loaded-files' (A 
side note. I missed a public function that reloads subset of files from 
a given directory that are present in `load-path' to use it in 
`org-reload').


`package--list-of-conflicts' had a couple of bugs:

1. Looking up for loaded earlier files, composition of 
`file-name-sans-extension` and `find-library-name' strips just `.gz', 
not `.el.gz' thus comparing e.g. '//org.el' (.gz stripped) and 
'//org' (.elc stripped). As a result new .el Org files are not 
reloaded over Org .elc files from an older version.


2. A debian-specific issue when elpa-org deb package is installed:

ls -l /usr/share/emacs/site-lisp/elpa/org-9.3.1/org.*
lrwxrwxrwx 1 root root 53 Jan 29  2021 
/usr/share/emacs/site-lisp/elpa/org-9.3.1/org.el -> 
/usr/share/emacs/site-lisp/elpa-src/org-9.3.1//org.el
-rw-r--r-- 1 root root 710150 Jan 29  2021 
/usr/share/emacs/site-lisp/elpa/org-9.3.1/org.elc


Notice that .el files are symlinks to another directory. Due to 
`file-truename' in `package--list-of-conflicts' .el files are not 
reloaded because full paths are compared.


So prior to version 29, Emacs had code that is intended to reload 
earlier loaded files when ELPA package is installed, but it was buggy. 
It seems both issues are fixed in new `package--reload-previously-loaded'.


Now I understand what happens, but I have not figure out what is the 
best strategy to prevent failures. Of course, it should be clearly 
stated in the manual and in the README that Org should not be loaded yet 
when `package-install' is called.



- Why even when the `org-assert-version' macro is defined, an error is
signaled on attempt to load a compiled file with unexpanded
(org-assert-version) call (a file compiled with warning "the function
‘org-assert-version’ is not known")?


This is because `org-assert-version' was not defined in Emacs during
compile time. During compilation, Emacs produces byte code calling a
function. AFAIU, the byte code is equivalent to
(funcall #'org-assert-version), which fails with error (try it with M-:).


You are right. I did not realized that actually 2 kinds of errors happens:
- Invalid function: org-assert-version
- Symbol’s function definition is void: org-assert-version
  (and compile Warning: the function ‘org-assert-version’ is not known 
to be defined.


"void"/"not known" happens when old org-macs.el is loaded. "Invalid 
function" is the sign of mixed compilation. `org-assert-version' is 
defined, but it is a macro, so can not be called during loading of an 
.elc file since it was not expanded during compilation.


So `fboundp' is not the correct way to generate meaningful message 
during loading result of mixed compilation. `macrop' is more selective 
(and `functionp' as well).


Relevant section of the elisp manual
- (info "(elisp) Function Cells")
- (info "(elisp) Calling Functions")
- (info "(elisp) What Is a Function")




Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-04-10 Thread Ihor Radchenko
Max Nikulin  writes:

>> What about creating org-assert-version.el file that will contain
>> something like
>> 
>> (if (fboundp 'org-assert-version)
>>   (org-assert-version)
>>   (warn ""))
>> 
>> Then, instead of (org-assert-version) call, we can put
>> (load "org-assert-version.el") in Org libraries.
>
> I believed that the only way to make `org-assert-version' effective is 
> to put org version in every Org compiled .elc file, so you suggestion 
> makes version check useless.

Hmm. Not completely useless - it will still effectively compare between
Org version in org-version.el and org-assert-version.el. But indeed less
accurate.

>  An earlier idea was to put 
> `org-assert-version' *definition* to a separate file keeping call in 
> each file as one time workaround to ensure that `org-assert-version' is 
> defined while compiling when an older Org version is loaded to Emacs < 29.

Does it solve the problem with "make install" on Debian?

> Currently I do not understand:
> - Why presence of .el files in the same directory (old Org version) with 
> .elc files affects result of compilation (at least for Emacs-28)?

I may be missing your point, but `load-prefer-newer' maybe?

> - Why even when the `org-assert-version' macro is defined, an error is 
> signaled on attempt to load a compiled file with unexpanded 
> (org-assert-version) call (a file compiled with warning "the function 
> ‘org-assert-version’ is not known")?

This is because `org-assert-version' was not defined in Emacs during
compile time. During compilation, Emacs produces byte code calling a
function. AFAIU, the byte code is equivalent to
(funcall #'org-assert-version), which fails with error (try it with M-:).

> Unfortunately I did not bookmarked discussions containing details 
> related to straight.el issues, so I am unsure if the problems are the 
> same as for installing from ELPA by package.el.

straight.el has a special arrangement for compiling Org. See
https://github.com/radian-software/straight.el/blob/master/straight.el#L3133
Basically, it spawns a child Emacs process and thus does not struggle
with mixed compilation state. (I am not 100% sure about the "make
install" issue you encountered).

The main issue with straight.el is its design - straight.el has no
equivalent of `package-selected-packages' and instead loads the packages
as they appear in the init.el. It is a problem when some statement has
(require 'org) early in the config, before straight sets up the newer
Org version.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-04-10 Thread Max Nikulin

On 09/04/2023 15:29, Ihor Radchenko wrote:

Max Nikulin writes:


I would consider adding to ELPA README and Org manual something like:

If compiling of ELPA packages causes a lot of warnings related to
`org-assert-version' then delete just installed Org package, start new
Emacs session ensuring that Org is not loaded (-q?) and try to install
from such clean state.


What about creating org-assert-version.el file that will contain
something like

(if (fboundp 'org-assert-version)
  (org-assert-version)
  (warn ""))

Then, instead of (org-assert-version) call, we can put
(load "org-assert-version.el") in Org libraries.


I believed that the only way to make `org-assert-version' effective is 
to put org version in every Org compiled .elc file, so you suggestion 
makes version check useless. An earlier idea was to put 
`org-assert-version' *definition* to a separate file keeping call in 
each file as one time workaround to ensure that `org-assert-version' is 
defined while compiling when an older Org version is loaded to Emacs < 29.


Currently I do not understand:
- Why presence of .el files in the same directory (old Org version) with 
.elc files affects result of compilation (at least for Emacs-28)?
- Why even when the `org-assert-version' macro is defined, an error is 
signaled on attempt to load a compiled file with unexpanded 
(org-assert-version) call (a file compiled with warning "the function 
‘org-assert-version’ is not known")?


Unfortunately I did not bookmarked discussions containing details 
related to straight.el issues, so I am unsure if the problems are the 
same as for installing from ELPA by package.el.





Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-04-09 Thread Ihor Radchenko
Max Nikulin  writes:

> I would consider adding to ELPA README and Org manual something like:
>
> If compiling of ELPA packages causes a lot of warnings related to 
> `org-assert-version' then delete just installed Org package, start new 
> Emacs session ensuring that Org is not loaded (-q?) and try to install 
> from such clean state.

What about creating org-assert-version.el file that will contain
something like

(if (fboundp 'org-assert-version)
 (org-assert-version)
 (warn ""))

Then, instead of (org-assert-version) call, we can put
(load "org-assert-version.el") in Org libraries.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-04-08 Thread Max Nikulin

On 29/03/2023 23:52, Ihor Radchenko wrote:

I was able to reproduce on Debian using virtual machine.
Also, several people were able to reproduce in Matrix when I asked them.
And we got one report from Fedora user. + Several from Ubuntu users.
Recently, there was also
https://old.reddit.com/r/orgmode/comments/123qnqq/workaround_for_orgassertversion_problem_not/


I would consider adding to ELPA README and Org manual something like:

If compiling of ELPA packages causes a lot of warnings related to 
`org-assert-version' then delete just installed Org package, start new 
Emacs session ensuring that Org is not loaded (-q?) and try to install 
from such clean state.


I never expected that result of compiling may depend on whether the 
directory with built-in Org contains besides .elc files gzipped sources 
.el.gz (as after make install) or original .el files.
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59882#134 (sorry, I sent 
this message without Cc to emacs-orgmode.)





Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-04-05 Thread Max Nikulin

Ihor, I am sorry, but I sent this message off-list at first.

On 04/04/2023 20:29, Ihor Radchenko wrote:


Sure. Anything Emacs devs can act on. If you can prove that the problem
is in Emacs source code, we can report it in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59882 and let Emacs devs
fix things.


I do not expect any result that may be useful for Emacs dev. I hope, the 
code added to emacs-29 branch fixes the issue. Outcome may be helpful 
for choosing better variant of `org-assert-version' and for demystifying 
causes why you can not reproduce it in gentoo.



This looks like
https://orgmode.org/list/jwvsfkv5s7l.fsf-monnier+em...@gnu.org


I have not looked into make rules used in Emacs source tree, but I 
expect it is another issue. I do not like that elisp can not generate 
list of dependencies during compilation similar to "gcc -MD". Otherwise 
I do not see a way to implement incremental rebuild if stale .elc files 
are preferred to updated .el sources by `require'.



But Emacs >26 + package-install specifically arranges unloading the
older version of the library.


What changes are you writing about? I mean 
`package--reload-previously-loaded' 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59882#71


9dfd945a2c2 2021-11-07 01:28:47 +0100 dickmao: Fix byte compilation of 
package built-ins


and I do not see similar changes even in Emacs-28.




Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-04-04 Thread Ihor Radchenko
Max Nikulin  writes:

> On 02/04/2023 23:44, Ihor Radchenko wrote:
>> 1. The old problem with mixed installation that manifests itself for
>> some users
>> 2. The new problem with org-assert-version, which appears to be even
>> more frequent, despite trying preventing (1).
>
> I leave decision up to you. My opinion that both cases are weird. With 
> `org-assert-version' users become aware that Org is broken rather 
> quickly, without it they have a time bomb that gives impression of bugs 
> in Org. From my point of view the real issue is that in both cases error 
> messages are not instructive. The downside of current implementation of 
> `org-assert-version' is that Org does not work at all.

I guess we might try to run a poll. My view on this bug is rather skewed
as I saw it too many times. The true number of Org users affected may
actually vary.

> Ihor will you consider the issue as reproduced if I compile emacs from 
> some tag in git repository on Debian or Ubuntu, not with provided binary 
> packages?

Sure. Anything Emacs devs can act on. If you can prove that the problem
is in Emacs source code, we can report it in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59882 and let Emacs devs
fix things.

Currently, it looks like Debian modifies certain aspects of Emacs
loading. In such scenario, we cannot expect things to be fixed on Emacs
side.

> Do you consider the following as similar to package install issue for 
> Emacs < 29? See the attachment

This looks like
https://orgmode.org/list/jwvsfkv5s7l.fsf-monnier+em...@gnu.org

But Emacs >26 + package-install specifically arranges unloading the
older version of the library. It should not suffer from this (in
theory).

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-04-04 Thread Max Nikulin

On 02/04/2023 23:44, Ihor Radchenko wrote:

1. The old problem with mixed installation that manifests itself for
some users
2. The new problem with org-assert-version, which appears to be even
more frequent, despite trying preventing (1).


I leave decision up to you. My opinion that both cases are weird. With 
`org-assert-version' users become aware that Org is broken rather 
quickly, without it they have a time bomb that gives impression of bugs 
in Org. From my point of view the real issue is that in both cases error 
messages are not instructive. The downside of current implementation of 
`org-assert-version' is that Org does not work at all.


The above is related to *mixed compilation*, attempt to compile when 
some files from old version is loaded. The result is that some files are 
compiled with old macro definitions, some files are not compiled at all.


A different issue is *mixed load* when Org might be properly compiled, 
but at first some files from old version are loaded prior to adding the 
directory containing new version to load path.


I am not familiar with straight.el.

Ihor will you consider the issue as reproduced if I compile emacs from 
some tag in git repository on Debian or Ubuntu, not with provided binary 
packages? There are some funtoo LXC images, I never use it, so I have no 
idea how much time it will take to create build environment enough to 
build Emacs and how close it to Gentoo.


Do you consider the following as similar to package install issue for 
Emacs < 29? See the attachment


cd update-issue/
emacs -Q -l load-old-compile-new.el
emacs -Q -l load-new.el

==> old/upissue.el <==

(require 'upissue-macro)
(defun upissue-example (x) x)
(provide 'upissue)

==> old/upissue-macro.el <==

(defun upissue-macro-example (x) x)
(provide 'upissue-macro)

==> new/upissue.el <==

(require 'upissue-macro)
(upissue-macro-check)
(defun upissue-example (x) x)
(provide 'upissue)

==> new/upissue-macro.el <==

(defmacro upissue-macro-check ()
  (message "upissue-macro-check called"))
(defun upissue-macro-example (x) x)
(provide 'upissue-macro)

==> compile-new.el <==
;; Compile new version
(add-to-list 'load-path (expand-file-name "new"))
(byte-recompile-directory "new" 0 :force)

==> load-new.el <==
;; Load new version
(setq debug-on-error t)
(add-to-list 'load-path (expand-file-name "new"))
(require 'upissue)

==> load-old-compile-new.el <==
;; Load old version
(add-to-list 'load-path (expand-file-name "old"))
(byte-recompile-directory "old" 0 :force)
(kill-buffer "*Compile-Log*")
(require 'upissue)
;; Compile new version
(add-to-list 'load-path (expand-file-name "new"))
(byte-recompile-directory "new" 0 :force)


update-issue.tar.gz
Description: application/gzip


Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-04-02 Thread Ihor Radchenko
Max Nikulin  writes:

>> Does it mean that you are able to?
>
> I reproduced the issue with packages available in Debian and Ubuntu, 
> including -Q, so minimally affected by distribution-specific 
> configuration, but I have not tried to compile Emacs myself. I am 
> judging from changes in Emacs code made after release of version 28.

My attempts to reproduce with emacs -Q only worked on Debian.

>  apt build-dep emacs
>
> and perhaps "apt install" for some …-dev packages for new features. 
> However my surprise is namely dependence on specific 
> packaging/distribution. My expectation that it should be reproducible in 
> Gentoo as well.

It is not, on my side. I tried multiple times.

>> It won't, but the very reason that message appeared is the need to do
>> that awkward workaround. It must not be needed - we are causing way too
>> much inconvenience to users of Emacs versions we claim to support.
>
> Either I missed your point or the issue will be just postponed. Patched 
> variant will not prevent mixed version compilation, so users still may 
> experience calls of undefined functions or incompatible argument types.

There are two issues here:
1. The old problem with mixed installation that manifests itself for
   some users
2. The new problem with org-assert-version, which appears to be even
   more frequent, despite trying preventing (1).

So, we replace less frequent problem and more frequent, which is not useful.
Better leave (1) as is until we support Emacs versions where
org-assert-version does not cause issues.

Of course, ideally we need to solve this issue with org-assert-version,
but I see no good way.

>>> I am not sure. Perhaps it should be (or (fboundp 'org-assert-version)
>>> (new-package-management-code)). Since testing for private function is
>>> not a reliable solution, only version check is available.
>> 
>> May you elaborate?
>
> I do not think it is good idea to rely on
>
>  (or (fboundp 'org-assert-version)
>  (fboundp 'package--reload-previously-loaded))

This is probably slightly better, but the gain appears to be marginal.
We will still miss mixed installation caused by straight.el - much more
frequent problem; and the reason I decided to write org-assert-version.

> I have not checked if some public function may be used for feature 
> detection of code appeared in Emacs-29. That is why I would consider
>
>  (when (or (fboundb 'org-assert-version)
>(version<= "29" emacs-version))
> (org-assert-version))

> However I am still in doubts if it is improvement in comparison to 
> simple (org-assert-verions) without any conditions.

This won't solve the problem I am trying to work around. Emacs 28 and
Emacs 27 are what people use most, and it is where (on Debian)
org-assert-version does not work with emacs -Q.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-04-02 Thread Max Nikulin

On 02/04/2023 15:35, Ihor Radchenko wrote:

I was able to reproduce on Debian using virtual machine.


To be precise, I am surprised that you are unable to reproduce the issue
with older Emacs version compiled from source.


Does it mean that you are able to?


I reproduced the issue with packages available in Debian and Ubuntu, 
including -Q, so minimally affected by distribution-specific 
configuration, but I have not tried to compile Emacs myself. I am 
judging from changes in Emacs code made after release of version 28.



I only tried on Gentoo, because my Debian-foo is not good enough to hunt
all the required compile-time dependencies for Emacs.


apt build-dep emacs

and perhaps "apt install" for some …-dev packages for new features. 
However my surprise is namely dependence on specific 
packaging/distribution. My expectation that it should be reproducible in 
Gentoo as well.



https://old.reddit.com/r/orgmode/comments/123qnqq/workaround_for_orgassertversion_problem_not/


I have no clue why your patch should help in this case.


It won't, but the very reason that message appeared is the need to do
that awkward workaround. It must not be needed - we are causing way too
much inconvenience to users of Emacs versions we claim to support.


Either I missed your point or the issue will be just postponed. Patched 
variant will not prevent mixed version compilation, so users still may 
experience calls of undefined functions or incompatible argument types.



I am in doubts if emacs version should be checked or it should be e.g.
(fboundp 'org-assert-version).


It is indeed a cleaner approach.


I am not sure. Perhaps it should be (or (fboundp 'org-assert-version)
(new-package-management-code)). Since testing for private function is
not a reliable solution, only version check is available.


May you elaborate?


I do not think it is good idea to rely on

(or (fboundp 'org-assert-version)
(fboundp 'package--reload-previously-loaded))

I have not checked if some public function may be used for feature 
detection of code appeared in Emacs-29. That is why I would consider


(when (or (fboundb 'org-assert-version)
  (version<= "29" emacs-version))
   (org-assert-version))

However I am still in doubts if it is improvement in comparison to 
simple (org-assert-verions) without any conditions.






Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-04-02 Thread Ihor Radchenko
Max Nikulin  writes:

>> I was able to reproduce on Debian using virtual machine.
>
> To be precise, I am surprised that you are unable to reproduce the issue 
> with older Emacs version compiled from source.

Does it mean that you are able to?
I only tried on Gentoo, because my Debian-foo is not good enough to hunt
all the required compile-time dependencies for Emacs.

>> https://old.reddit.com/r/orgmode/comments/123qnqq/workaround_for_orgassertversion_problem_not/
>
> I have no clue why your patch should help in this case.

It won't, but the very reason that message appeared is the need to do
that awkward workaround. It must not be needed - we are causing way too
much inconvenience to users of Emacs versions we claim to support.

>>> I am in doubts if emacs version should be checked or it should be e.g.
>>> (fboundp 'org-assert-version).
>> 
>> It is indeed a cleaner approach.
>
> I am not sure. Perhaps it should be (or (fboundp 'org-assert-version) 
> (new-package-management-code)). Since testing for private function is 
> not a reliable solution, only version check is available.

May you elaborate?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-04-01 Thread Max Nikulin

On 29/03/2023 23:52, Ihor Radchenko wrote:

Max Nikulin writes:

Sorry, I have not tried to compile Emacs myself and I am really puzzled
why you was not able to reproduce the issue.


I was able to reproduce on Debian using virtual machine.


To be precise, I am surprised that you are unable to reproduce the issue 
with older Emacs version compiled from source.



https://old.reddit.com/r/orgmode/comments/123qnqq/workaround_for_orgassertversion_problem_not/


I have no clue why your patch should help in this case.

My hypothesis: the user has `use-package' org-roam in their init file. 
Attempt to update org caused compiling new org when old version is 
already loaded due to Emacs-27 does not contain fixes that reload 
package that is installed. Next time load of org failed because it is 
result of mixed version compilation.



I am in doubts if emacs version should be checked or it should be e.g.
(fboundp 'org-assert-version).


It is indeed a cleaner approach.


I am not sure. Perhaps it should be (or (fboundp 'org-assert-version) 
(new-package-management-code)). Since testing for private function is 
not a reliable solution, only version check is available.






Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-03-29 Thread Ihor Radchenko
Max Nikulin  writes:

> On 29/03/2023 20:38, Ihor Radchenko wrote:
>> Even though we identified that the problem is related to a combination
>> of old Emacs version and OS-specific Emacs customization
>
> Ihor, who should notice improvement due to this patch? Is it related to 
> Emacs-29.1 release? The issue is weird, but on the other hand I do not 
> remember complains on the mailing list last months (besides some period 
> close to the Org release).
>
> Sorry, I have not tried to compile Emacs myself and I am really puzzled 
> why you was not able to reproduce the issue. So from my point of view it 
> is unlikely related to distribution-specific configuration or patches. 
> Changes related to `package--reload-previously-loaded` sound more 
> reasonable for me.

I was able to reproduce on Debian using virtual machine.
Also, several people were able to reproduce in Matrix when I asked them.
And we got one report from Fedora user. + Several from Ubuntu users.
Recently, there was also
https://old.reddit.com/r/orgmode/comments/123qnqq/workaround_for_orgassertversion_problem_not/

It is natural that we are getting fewer reports away from the release,
after most people upgraded Org. Now, most of the affected users are the
new users trying to install Org after installing Emacs. These users are
much less likely to report bugs.

> I am in doubts if emacs version should be checked or it should be e.g. 
> (fboundp 'org-assert-version).

It is indeed a cleaner approach.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-03-29 Thread Max Nikulin

On 29/03/2023 20:38, Ihor Radchenko wrote:

Even though we identified that the problem is related to a combination
of old Emacs version and OS-specific Emacs customization


Ihor, who should notice improvement due to this patch? Is it related to 
Emacs-29.1 release? The issue is weird, but on the other hand I do not 
remember complains on the mailing list last months (besides some period 
close to the Org release).


Sorry, I have not tried to compile Emacs myself and I am really puzzled 
why you was not able to reproduce the issue. So from my point of view it 
is unlikely related to distribution-specific configuration or patches. 
Changes related to `package--reload-previously-loaded` sound more 
reasonable for me.


I am in doubts if emacs version should be checked or it should be e.g. 
(fboundp 'org-assert-version).


Commit message:


tweaks and built-in Org not definition `org-assert-version'.


Looks like a typo.




Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2023-03-29 Thread Ihor Radchenko
Bastien  writes:

> That said, do you have any idea how to fix the bug people encounter
> when installing Org from ELPA and being bitten by "invalid-function
> org-assert-version"?

Even though we identified that the problem is related to a combination
of old Emacs version and OS-specific Emacs customization
(https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59882), no further
progress is there on fixing the issue.

Considering the mixed installation problem was less frequent compared to
this new one, I suggest the attached workaround - simply not calling
`org-assert-version' in Emacs <29, where built-in Org does not provide
this macro. It should at least resolve the immediate pain for many
people at the cost of possible mixed installation problems.

Unless there are immediate concerns about the patch, I am going to
install it soon onto bugfix.

>From a4b25e0fc0844c48fd89cb76cc242fca08a66c02 Mon Sep 17 00:00:00 2001
Message-Id: 
From: Ihor Radchenko 
Date: Wed, 29 Mar 2023 15:27:50 +0200
Subject: [PATCH] Work around `org-assert-version' bug on older Emacs

Do not use `org-assert-version' for Emacs builds where Org does not
define this macro inside built-in Org.

See Emacs bug#59882, where the problem is triggered by some unclear
combination of OS-specific (at least Debian, Ubuntu, and Fedora) Emacs
tweaks and built-in Org not definition `org-assert-version'.

This is not a proper fix of the mixed installation problem that
`org-assert-version' attempts to solve, but still better as
`org-assert-version' triggers problems more consistently than mixed
installation.
---
 lisp/ob-C.el   | 3 ++-
 lisp/ob-R.el   | 3 ++-
 lisp/ob-calc.el| 3 ++-
 lisp/ob-clojure.el | 3 ++-
 lisp/ob-comint.el  | 3 ++-
 lisp/ob-core.el| 3 ++-
 lisp/ob-css.el | 3 ++-
 lisp/ob-ditaa.el   | 3 ++-
 lisp/ob-dot.el | 3 ++-
 lisp/ob-emacs-lisp.el  | 3 ++-
 lisp/ob-eshell.el  | 3 ++-
 lisp/ob-eval.el| 3 ++-
 lisp/ob-exp.el | 3 ++-
 lisp/ob-forth.el   | 3 ++-
 lisp/ob-fortran.el | 3 ++-
 lisp/ob-gnuplot.el | 3 ++-
 lisp/ob-groovy.el  | 3 ++-
 lisp/ob-haskell.el | 3 ++-
 lisp/ob-java.el| 3 ++-
 lisp/ob-js.el  | 3 ++-
 lisp/ob-julia.el   | 3 ++-
 lisp/ob-latex.el   | 3 ++-
 lisp/ob-lilypond.el| 3 ++-
 lisp/ob-lisp.el| 3 ++-
 lisp/ob-lob.el | 3 ++-
 lisp/ob-lua.el | 3 ++-
 lisp/ob-makefile.el| 3 ++-
 lisp/ob-matlab.el  | 3 ++-
 lisp/ob-maxima.el  | 3 ++-
 lisp/ob-ocaml.el   | 3 ++-
 lisp/ob-octave.el  | 3 ++-
 lisp/ob-org.el | 3 ++-
 lisp/ob-perl.el| 3 ++-
 lisp/ob-plantuml.el| 3 ++-
 lisp/ob-processing.el  | 3 ++-
 lisp/ob-python.el  | 3 ++-
 lisp/ob-ref.el | 3 ++-
 lisp/ob-ruby.el| 3 ++-
 lisp/ob-sass.el| 3 ++-
 lisp/ob-scheme.el  | 3 ++-
 lisp/ob-screen.el  | 3 ++-
 lisp/ob-sed.el | 3 ++-
 lisp/ob-shell.el   | 3 ++-
 lisp/ob-sql.el | 3 ++-
 lisp/ob-sqlite.el  | 3 ++-
 lisp/ob-table.el   | 3 ++-
 lisp/ob-tangle.el  | 3 ++-
 lisp/ob.el | 3 ++-
 lisp/oc-basic.el   | 3 ++-
 lisp/oc-biblatex.el| 3 ++-
 lisp/oc-bibtex.el  | 3 ++-
 lisp/oc-csl.el | 3 ++-
 lisp/oc-natbib.el  | 3 ++-
 lisp/oc.el | 3 ++-
 lisp/ol-bbdb.el| 3 ++-
 lisp/ol-bibtex.el  | 3 ++-
 lisp/ol-docview.el | 3 ++-
 lisp/ol-doi.el | 3 ++-
 lisp/ol-eshell.el  | 3 ++-
 lisp/ol-eww.el | 3 ++-
 lisp/ol-gnus.el| 3 ++-
 lisp/ol-info.el| 3 ++-
 lisp/ol-irc.el | 3 ++-
 lisp/ol-man.el | 3 ++-
 lisp/ol-mhe.el | 3 ++-
 lisp/ol-rmail.el   | 3 ++-
 lisp/ol-w3m.el | 3 ++-
 lisp/ol.el | 3 ++-
 lisp/org-agenda.el | 3 ++-
 lisp/org-archive.el| 3 ++-
 lisp/org-attach-git.el | 3 ++-
 lisp/org-attach.el | 3 ++-
 lisp/org-capture.el| 3 ++-
 lisp/org-clock.el  | 3 ++-
 lisp/org-colview.el| 3 ++-
 lisp/org-crypt.el  | 3 ++-
 lisp/org-ctags.el  | 3 ++-
 lisp/org-cycle.el  | 3 ++-
 lisp/org-datetree.el   | 3 ++-
 lisp/org-duration.el   | 3 ++-
 lisp/org-element.el| 3 ++-
 lisp/org-entities.el   | 3 ++-
 lisp/org-faces.el  | 3 ++-
 lisp/org-feed.el   | 3 ++-
 lisp/org-fold-core.el  | 3 ++-
 lisp/org-fold.el   | 3 ++-
 lisp/org-footnote.el   | 3 ++-
 lisp/org-goto.el   | 3 ++-
 lisp/org-habit.el  | 3 ++-
 lisp/org-id.el | 3 ++-
 lisp/org-indent.el | 3 ++-
 lisp/org-inlinetask.el | 3 ++-
 lisp/org-keys.el   | 3 ++-
 lisp/org-lint.el   | 3 ++-
 lisp/org-list.el   | 3 ++-
 lisp/org-macro.el  | 3 ++-
 lisp/org-mobile.el | 3 ++-
 lisp/org-mouse.el  | 3 ++-
 lisp/org-num.el| 3 ++-
 lisp/org-pcomplete.el  | 3 ++-
 lisp/org-persist.el| 3 ++-
 lisp/org-plot.el   | 3 ++-
 lisp/org-protocol.el   | 3 ++-
 lisp/org-refile.el | 3 ++-
 lisp/org-src.el| 3 ++-
 lisp/org-table.el  | 3 ++-
 lisp/org-tempo.el 

Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2022-12-07 Thread Max Nikulin

On 07/12/2022 18:43, Ihor Radchenko wrote:

That said, I find David Masterson's suggestion promising:

  (if (and (featurep 'org) (< org-version 9.6))


`version<'?

Mixed install may be still a problem in the case of bug fix ("patch" 
component of the version) release.





Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2022-12-07 Thread Ihor Radchenko
Bastien  writes:

> That said, do you have any idea how to fix the bug people encounter
> when installing Org from ELPA and being bitten by "invalid-function
> org-assert-version"?

I just asked Emacs devs as it does not look like we do anything wrong
here. It is probably Emacs package.el doing something wrong.
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59882

That said, I find David Masterson's suggestion promising:

  (if (and (featurep 'org) (< org-version 9.6))
  (unload-feature 'org))

We may force unloading the current Org version at the beginning of all
the libraries.

Another possibility could be

  (eval-when-compile (load-file "./org-macs.el"))

instead of require.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2022-12-05 Thread David Masterson
 writes:

> On Mon, Dec 05, 2022 at 07:54:25PM -0800, David Masterson wrote:
>
>> Question: which end of load-path is "front" if you look at load-path via
>> describe-variable?  It's been far too long since my days of Elisp
>> hacking.
>
> This goes "left to right" when looking at it on a conventional
> display. The first hit "wins".

So I did have it backwards.  Hmm.

> [...]
> meaning that Emacs first looks in my personal directories, then
> in the system local ones (I have a "locally" installed Emacs
> from the distribution's POV).
>
> The distribution's directories would go even later, were there
> any.

But this is a matter of when things are added to load-path.  If
something from the distribution is loaded before you setup your
load-path (before ,emacs), it gets very difficult to override it locally
(somewhat by design).  The load can also happen by dependency loads
before you get to setting up your local (Org) package.

I'll have to test when the distribution org is getting loaded.

Thanks

-- 
David Masterson



Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2022-12-05 Thread tomas
On Mon, Dec 05, 2022 at 07:54:25PM -0800, David Masterson wrote:

[...]

> Question: which end of load-path is "front" if you look at load-path via
> describe-variable?  It's been far too long since my days of Elisp
> hacking.

This goes "left to right" when looking at it on a conventional
display. The first hit "wins". My load path looks like this
(well, a part of it, I don't want to bore everyone ;-)

   ( "~/.emacs.d/lisp"
 "/home/tomas/.emacs.d/elpa/geiser-0.10/"
 "/home/tomas/.emacs.d/elpa/geiser-0.10"
 "/home/tomas/.emacs.d/elpa/markdown-mode-2.1" 
 ...
 "/usr/local/share/emacs/29.0.50/lisp/emacs-lisp"
 "/usr/local/share/emacs/29.0.50/lisp/cedet"
 ... )

meaning that Emacs first looks in my personal directories, then
in the system local ones (I have a "locally" installed Emacs
from the distribution's POV).

The distribution's directories would go even later, were there
any.

Cheers
-- 
t


signature.asc
Description: PGP signature


Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2022-12-05 Thread David Masterson
Kyle Meyer  writes:

> Bastien writes:
>
>> Kyle Meyer  writes:
> [...]
>
>>> Every spot that calls org-assert-version is preceded by a line that
>>> requires org-macs, so isn't this error likely due to a mixed
>>> installation/load-path issue where the wrong/older org-macs is taking
>>> precedence?
>>
>> Yes: I thought `org-assert-version' job was to help catching mixed
>> installations, so my reasoning was that, even when an old org-macs
>> version has been loaded, autoloading `org-assert-version' would help
>> report about mixed installation.
>
> I see.  So your goal isn't to resolve any errors due to mixed
> installation but instead to get the more informative error message from
> org-assert-version?
>
> I don't think autoloading will help because it will look in the
> org-macs file at the front of the load-path, and, in the bad case, that
> does not have the org-assert-version definition.

Question: which end of load-path is "front" if you look at load-path via
describe-variable?  It's been far too long since my days of Elisp
hacking.

Another typical use case is Org bundled with Emacs and you want to
install a later version of Org via package-install.

>> That said, do you have any idea how to fix the bug people encounter
>> when installing Org from ELPA and being bitten by "invalid-function
>> org-assert-version"?
>
> Sorry, I don't.  Given a mixed installation where the org-macs at the
> front of load-path does _not_ have org-assert-version, I don't see a way
> to catch that.  Not helpful at the moment, but I suppose the situation
> improves with time as Org versions that do not have org-assert-version
> become less abundant.

Is Org the only thing that will call 'org-assert-version'? Then assume
that a pre-built version of Emacs/Org is self-consistent.  Then
(perhaps?) patch an early spot in Org-9.6 to do:

  (if (and (featurep 'org) (< org-version 9.6))
  (unload-feature 'org))

-- 
David Masterson



Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2022-12-05 Thread David Masterson
Kyle Meyer  writes:

> Bastien writes:
>
>> Some users reported an (invalid-function org-assert-version) error
>> when installing Org from ELPA:
>>
>> https://lists.sr.ht/~bzg/emacsfr/%3Cd091463e1615422eb00070727d6a094ec0ae3c73.camel%40adocentyn.io%3E
>> https://www.reddit.com/r/emacs/comments/z7qulo/comment/iyd9vam/?context=3
>>
>> This patch autoloads `org-assert-version'.
>
> I don't understand the rationale behind this.  Every spot that calls
> org-assert-version is preceded by a line that requires org-macs, so
> isn't this error likely due to a mixed installation/load-path issue
> where the wrong/older org-macs is taking precedence?

Bingo!  This sounds like my problem -- installing/loading org-9.6 when
org-9.3 was already loaded by the startup process (Debian pre-built
Emacs 27.1) results in errors related to 'org=assert-version'.

>> It also removes the ;; generated-autoload-file: "org-loaddefs.el"
>> footer in all files and let Make create org-autoloads.el instead.
>
> I'm worried with how this will play with the Org bundled with Emacs.

It doesn't play in my case.

-- 
David Masterson



Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2022-12-02 Thread Kyle Meyer
Bastien writes:

> Kyle Meyer  writes:
[...]

>> Every spot that calls org-assert-version is preceded by a line that
>> requires org-macs, so isn't this error likely due to a mixed
>> installation/load-path issue where the wrong/older org-macs is taking
>> precedence?
>
> Yes: I thought `org-assert-version' job was to help catching mixed
> installations, so my reasoning was that, even when an old org-macs
> version has been loaded, autoloading `org-assert-version' would help
> report about mixed installation.

I see.  So your goal isn't to resolve any errors due to mixed
installation but instead to get the more informative error message from
org-assert-version?

I don't think autoloading will help because it will look in the
org-macs file at the front of the load-path, and, in the bad case, that
does not have the org-assert-version definition.

Here's a minimal example:

--8<---cut here---start->8---
#!/bin/sh

set -eu

tdir=$(mktemp -d "${TMPDIR:-/tmp}"/autoload-shadow-test-XXX)
cd "$tdir"

mkdir a
cat<<'EOF' >a/foo.el
(defun foo-do ()
  (message "foo"))

(provide 'foo)
EOF

mkdir b
echo "(provide 'foo)" >b/foo.el

cat<<'EOF' >test.el
(add-to-list 'load-path
 (concat default-directory (file-name-as-directory "a")))
(add-to-list 'load-path
 (concat default-directory (file-name-as-directory "b")))

(autoload 'foo-do "foo")
(foo-do)
EOF

emacs -Q --batch --load=test.el
--8<---cut here---end--->8---

If I run that, I see:

  Debugger entered--Lisp error: (error "Autoloading file 
/tmp/autoload-shadow-test-gmlO35u...")
(foo-do)
load-with-code-conversion("/tmp/autoload-shadow-test-gmlO35u/test.el" 
"/tmp/autoload-shadow-test-gmlO35u/test.el" nil t)
command-line-1(("--load=test.el"))
command-line()
normal-top-level()

And commenting out the add-to-list call for b confirms that it works
without the shadowing.

> That said, do you have any idea how to fix the bug people encounter
> when installing Org from ELPA and being bitten by "invalid-function
> org-assert-version"?

Sorry, I don't.  Given a mixed installation where the org-macs at the
front of load-path does _not_ have org-assert-version, I don't see a way
to catch that.  Not helpful at the moment, but I suppose the situation
improves with time as Org versions that do not have org-assert-version
become less abundant.



Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2022-12-01 Thread Bastien
Kyle Meyer  writes:

>> This patch autoloads `org-assert-version'.
>
> I don't understand the rationale behind this.  

It was my naive attempt at fixing the "invalid-function
org-assert-version" error several people reported.

> Every spot that calls
> org-assert-version is preceded by a line that requires org-macs, so
> isn't this error likely due to a mixed installation/load-path issue
> where the wrong/older org-macs is taking precedence?

Yes: I thought `org-assert-version' job was to help catching mixed
installations, so my reasoning was that, even when an old org-macs
version has been loaded, autoloading `org-assert-version' would help
report about mixed installation.

Also you're right about org-loaddefs.el, we should keep it.

That said, do you have any idea how to fix the bug people encounter
when installing Org from ELPA and being bitten by "invalid-function
org-assert-version"?

-- 
 Bastien



Re: [PATCH] Autoload `org-assert-version' and remove org-loaddefs.el

2022-12-01 Thread Kyle Meyer
Bastien writes:

> Some users reported an (invalid-function org-assert-version) error
> when installing Org from ELPA:
>
> https://lists.sr.ht/~bzg/emacsfr/%3Cd091463e1615422eb00070727d6a094ec0ae3c73.camel%40adocentyn.io%3E
> https://www.reddit.com/r/emacs/comments/z7qulo/comment/iyd9vam/?context=3
>
> This patch autoloads `org-assert-version'.

I don't understand the rationale behind this.  Every spot that calls
org-assert-version is preceded by a line that requires org-macs, so
isn't this error likely due to a mixed installation/load-path issue
where the wrong/older org-macs is taking precedence?

> It also removes the ;; generated-autoload-file: "org-loaddefs.el"
> footer in all files and let Make create org-autoloads.el instead.

I'm worried with how this will play with the Org bundled with Emacs.
The autoloads that are now written to lisp/org/org-loaddefs.el will
instead pollute the top-level lisp/loaddefs.el.

When I tested this out in the Emacs repo, running `make` shows many
lines of

WARNING: No org-autoloads.el file could be found from where org.el
is loaded.

and

You need to run "make" or "make autoloads" from Org lisp directory

during the build.  And loading/using Org shows the same.

> Before the patch, installing from ELPA creates both org-loaddefs.el
> and org-autoloads.el, which is confusing.

Hmm, isn't this in line with the lisp/org-loaddefs.el and
lisp/loaddefs.el split that happens for the Emacs repo?

A similar split is used other packages in the Emacs repo as well:

$ find lisp | grep '\-loaddefs.el' | head
lisp/ps-print-loaddefs.elc
lisp/org/org-loaddefs.el
lisp/org/org-loaddefs.elc
lisp/net/tramp-loaddefs.el
lisp/net/tramp-loaddefs.elc
lisp/calc/calc-loaddefs.el
lisp/calc/calc-loaddefs.elc
lisp/textmodes/reftex-loaddefs.el
lisp/textmodes/reftex-loaddefs.elc
lisp/textmodes/texinfo-loaddefs.elc

$ find lisp | grep -c '\-loaddefs.el'
29
$ find lisp | grep -c '\-autoloads.el'
0

And with the port of Emacs's 2682ea1b3 (Org commit 19c3969e7,
2022-10-04), there's also one spot that uses ;;;###org-autoload, which
tells loaddefs-gen.el to direct the autoload to org-loaddefs.el.

> Unless anyone objects, I'll install this patch in the bugfix branch
> tomorrow and release 9.6.1.

I think as is the patch would cause issues, and more generally I'm not
convinced we should be moving away from having an org-loaddefs.el file.
I'm also concerned about this sort of tree-wide change in a patch
release that will be synced to the emacs-29 branch.

Thanks for posting this for discussion, and please let me know if I'm
mistaken about the interaction with the Emacs repo.