org-assert-version considered harmful

org-assert-version considered harmful

[Stefan Monnier]

I can see the reason for this macro, and I don't really have a better
solution to offer, but as someone who has a Git clone of Org that is
regularly updated&compiled using (basically) the normal compilation
system used in Emacs (i.e. not systematically recompiling all the files,
but only those whose `.el` is more recent than their `.elc`), it means
that whenever Org's version changes, I have to manually erase all the
`.elc` files otherwise this macro will (incorrectly)
complain everywhere  :-(


        Stefan

Re: org-assert-version considered harmful

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I can see the reason for this macro, and I don't really have a better
> solution to offer, but as someone who has a Git clone of Org that is
> regularly updated&compiled using (basically) the normal compilation
> system used in Emacs (i.e. not systematically recompiling all the files,
> but only those whose `.el` is more recent than their `.elc`), it means
> that whenever Org's version changes, I have to manually erase all the
> `.elc` files otherwise this macro will (incorrectly)
> complain everywhere  :-(

Does it help if you run make autoloads after git fetch?

-- 
Ihor Radchenko,
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: org-assert-version considered harmful

[Timothy]

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

Hi Ihor and Stefan,

Considering the separate cases of:
• Installing an Org version different to the bundled Org
• Having a development Org version with some slightly-out-of-date cache/autoload
  files.

I’d think the second case has a dramatically reduced chance of issues. Could it
be reasonable to change the `org-assert-version' check to act like so?
• Check if `org-version' matches, if not error
• If `org-version' matches, but `org-git-version' does not, show a warning message.

Alternatively, we could create a variable `org-assert-version-allow-git-mismatch'
which can be set before loading Org (by people who know what they’re doing, e.g.
Stefan) to enable this proposed behaviour.

What do you think?

All the best,
Timothy

Re: org-assert-version considered harmful

[Stefan Monnier]

Ihor Radchenko [2022-09-13 09:52:37] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> I can see the reason for this macro, and I don't really have a better
>> solution to offer, but as someone who has a Git clone of Org that is
>> regularly updated&compiled using (basically) the normal compilation
>> system used in Emacs (i.e. not systematically recompiling all the files,
>> but only those whose `.el` is more recent than their `.elc`), it means
>> that whenever Org's version changes, I have to manually erase all the
>> `.elc` files otherwise this macro will (incorrectly)
>> complain everywhere  :-(
> Does it help if you run make autoloads after git fetch?

Not that I can tell, no.  But note that I'm not using Org's makefile to
compile the files, I'm using the elpa.git scripts instead, which don't
take into account dependencies between files.

So the situation is simply something like:

- git pull  =>  switches to 9.5.5, but several .el files are left unchanged.
- make autoloads  =>  this refreshes the autoloads, but the .elc files
  of those .el files which didn't change still won't be recompiled.

I'm not really reporting a bug; I'm not sure how to solve the problem
without throwing away the benefits of `org-assert-version`.  But I just
wanted to mention that it has unintended side effects :-)


        Stefan


PS: BTW, I notice that when Org is installed as a normal `package.el`
    package, in Emacs≥28 it will be activated before the `.emacs` file
    is loaded, which should prevent occurrence of the problems that
    `org-assert-version` aims to catch (unless you use, say, an
    org-babel file for the `early-init.el` :-)

PPS: Maybe instead of calling `org-assert-version` everywhere, the
     `org-autoloads.el` (i.e. the file that sets up the `load-path` and
     the autoloads) could look for traces of Org files in the
     `load-history` and signal an error if such files are found coming
     from a different directory.

Re: org-assert-version considered harmful

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Does it help if you run make autoloads after git fetch?
>
> Not that I can tell, no.  But note that I'm not using Org's makefile to
> compile the files, I'm using the elpa.git scripts instead, which don't
> take into account dependencies between files.
>
> So the situation is simply something like:
>
> - git pull  =>  switches to 9.5.5, but several .el files are left unchanged.
> - make autoloads  =>  this refreshes the autoloads, but the .elc files
>   of those .el files which didn't change still won't be recompiled.

Isn't it a bug in the elpa scripts then?
If a macro definition is changed and the .elc file using that macro is
not changed, it still needs to be re-compiled. Otherwise, all kinds of
unexpected side effects may appear.

> I'm not really reporting a bug; I'm not sure how to solve the problem
> without throwing away the benefits of `org-assert-version`.  But I just
> wanted to mention that it has unintended side effects :-)

I understand. As Timothy proposed, we can make less strict checks for Org
version consistency. But the question is whether we really need to make
a more lax assertion or maybe it is better to keep the assertion as is
and use it to catch potential issues e.g. in Elpa.

> PS: BTW, I notice that when Org is installed as a normal `package.el`
>     package, in Emacs≥28 it will be activated before the `.emacs` file
>     is loaded, which should prevent occurrence of the problems that
>     `org-assert-version` aims to catch (unless you use, say, an
>     org-babel file for the `early-init.el` :-)

Indeed. Version mismatch issue has been fixed in package.el years back.
But it is starting to pop up again as alternative package managers are
getting traction. There is a constant influx of bug reports caused by
mixed installation. Especially by users of Doom Emacs and straight.el.

Unfortunately, the problem cannot be easily solved, say, on straight.el
side --- straight.el basic design is causing the issue to appear.

> PPS: Maybe instead of calling `org-assert-version` everywhere, the
>      `org-autoloads.el` (i.e. the file that sets up the `load-path` and
>      the autoloads) could look for traces of Org files in the
>      `load-history` and signal an error if such files are found coming
>      from a different directory.

No, unfortunately.

org-autoloads, when loaded from built-in Emacs version will not help
to catch newer Org libraries being loaded after built-in Org version is
loaded.

Moreover, I consider loading personal forks of built-in Org libraries a
valid use-case. Demanding all the org libraries to be loaded from the
same directory will limit this possibility.

-- 
Ihor Radchenko,
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: org-assert-version considered harmful

[Stefan Monnier]

>> - git pull  =>  switches to 9.5.5, but several .el files are left unchanged.
>> - make autoloads  =>  this refreshes the autoloads, but the .elc files
>>   of those .el files which didn't change still won't be recompiled.
>
> Isn't it a bug in the elpa scripts then?
> If a macro definition is changed and the .elc file using that macro is
> not changed, it still needs to be re-compiled. Otherwise, all kinds of
> unexpected side effects may appear.

Yup.  But there's no option to automatically find those dependencies in
ELisp, and (IIRC from last time I looked at it, in many packages obeying
such dependencies would end up introducing circular dependencies in
the Makefile), so we'd have to depend on the package's author to provide
a working set of file dependencies.

Note that the same problem applies to Emacs's own ELisp files.
In Emacs we have `make bootstrap` to manually get out of such
a bad compilation.

>> PPS: Maybe instead of calling `org-assert-version` everywhere, the
>>      `org-autoloads.el` (i.e. the file that sets up the `load-path` and
>>      the autoloads) could look for traces of Org files in the
>>      `load-history` and signal an error if such files are found coming
>>      from a different directory.
>
> No, unfortunately.
>
> org-autoloads, when loaded from built-in Emacs version will not help
> to catch newer Org libraries being loaded after built-in Org version is
> loaded.

Hmm... after new-org-autoloads.el is loaded, the old-Org files will be
relegated to "late in the `load-path`" (i.e. after the directory that
holds the new-Org file) and should hence not be loaded any more (unless
someone goes through the trouble to explicitly load an old-Org files
with an absolute file name).

> Moreover, I consider loading personal forks of built-in Org libraries a
> valid use-case. Demanding all the org libraries to be loaded from the
> same directory will limit this possibility.

As long as they're loaded after new-org-autoloads.el, it would still be
fine since the test is only performed once when loading the
new-org-autoloads.el.


        Stefan

Re: org-assert-version considered harmful

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> - git pull  =>  switches to 9.5.5, but several .el files are left unchanged.
>>> - make autoloads  =>  this refreshes the autoloads, but the .elc files
>>>   of those .el files which didn't change still won't be recompiled.
>>
>> Isn't it a bug in the elpa scripts then?
>> If a macro definition is changed and the .elc file using that macro is
>> not changed, it still needs to be re-compiled. Otherwise, all kinds of
>> unexpected side effects may appear.
>
> Yup.  But there's no option to automatically find those dependencies in
> ELisp, and (IIRC from last time I looked at it, in many packages obeying
> such dependencies would end up introducing circular dependencies in
> the Makefile), so we'd have to depend on the package's author to provide
> a working set of file dependencies.

It would be nice to have such an option. At least, for the most critical
macros. Something similar to declare statements.

>>> PPS: Maybe instead of calling `org-assert-version` everywhere, the
>>>      `org-autoloads.el` (i.e. the file that sets up the `load-path` and
>>>      the autoloads) could look for traces of Org files in the
>>>      `load-history` and signal an error if such files are found coming
>>>      from a different directory.
>>
>> No, unfortunately.
>>
>> org-autoloads, when loaded from built-in Emacs version will not help
>> to catch newer Org libraries being loaded after built-in Org version is
>> loaded.
>
> Hmm... after new-org-autoloads.el is loaded, the old-Org files will be
> relegated to "late in the `load-path`" (i.e. after the directory that
> holds the new-Org file) and should hence not be loaded any more (unless
> someone goes through the trouble to explicitly load an old-Org files
> with an absolute file name).

I admit that I do not have sufficient knowledge about the autoload magic
Emacs uses when loading packages.

For reference, one simple way to trigger "mixed" state of Org is doing
something like:

1. emacs -Q
2. (require 'org)
3. Add the newer Org version to load-path
4. (require 'ob-python)

When and which version of org-autoloads.el will be loaded in such scenario?

-- 
Ihor Radchenko,
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: org-assert-version considered harmful

[Stefan Monnier]

>> Yup.  But there's no option to automatically find those dependencies in
>> ELisp, and (IIRC from last time I looked at it, in many packages obeying
>> such dependencies would end up introducing circular dependencies in
>> the Makefile), so we'd have to depend on the package's author to provide
>> a working set of file dependencies.
>
> It would be nice to have such an option.

Agreed.  The "last time" mentioned above, I looked at changing the
byte-compiler to keep track of the macros that were expanded so we can
auto-generate the dependencies.

> At least, for the most critical macros.  Something similar to
> declare statements.

But that also requires manual intervention :-(

>> Hmm... after new-org-autoloads.el is loaded, the old-Org files will be
>> relegated to "late in the `load-path`" (i.e. after the directory that
>> holds the new-Org file) and should hence not be loaded any more (unless
>> someone goes through the trouble to explicitly load an old-Org files
>> with an absolute file name).
>
> I admit that I do not have sufficient knowledge about the autoload magic
> Emacs uses when loading packages.
>
> For reference, one simple way to trigger "mixed" state of Org is doing
> something like:
>
> 1. emacs -Q
> 2. (require 'org)
> 3. Add the newer Org version to load-path
> 4. (require 'ob-python)
>
> When and which version of org-autoloads.el will be loaded in
> such scenario?

None :-(

In my book step 3 above is a mistake (even if moved to step 2).

The `org-autoloads.el` is the file that adds the dir to `load-path`
(and in a normal ELPA install, that's the file that `package.el` loads
for you at startup).
So step 3 above is replaced by (load ".../org-autoloads"), and that's
where the problem would be detected.

But admittedly, that won't help users who made the mistake of
manually adding to `load-path` :-(


        Stefan

Re: org-assert-version considered harmful

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> Yup.  But there's no option to automatically find those dependencies in
>>> ELisp, and (IIRC from last time I looked at it, in many packages obeying
>>> such dependencies would end up introducing circular dependencies in
>>> the Makefile), so we'd have to depend on the package's author to provide
>>> a working set of file dependencies.
>>
>> It would be nice to have such an option.
>
> Agreed.  The "last time" mentioned above, I looked at changing the
> byte-compiler to keep track of the macros that were expanded so we can
> auto-generate the dependencies.

Then, I am inclined towards easing the version check to (org-version)
instead of (org-git-version). It will be no worse than the existing
situation with re-compiling updated .el files.

>> For reference, one simple way to trigger "mixed" state of Org is doing
>> something like:
>>
>> 1. emacs -Q
>> 2. (require 'org)
>> 3. Add the newer Org version to load-path
>> 4. (require 'ob-python)
>>
>> When and which version of org-autoloads.el will be loaded in
>> such scenario?
>
> None :-(
>
> In my book step 3 above is a mistake (even if moved to step 2).

I am confused.
AFAIK, changing the load-path is a common way for users to install
packages manually.

We do recommend setting the load-path in
https://orgmode.org/org.html#Installation and
https://orgmode.org/manual/Feedback.html

Moreover, it is also the recommendation from Emacs manual section
"28.8 Libraries of Lisp Code for Emacs":

>>>    The default value of ‘load-path’ is a list of directories where the
>>> Lisp code for Emacs itself is stored.  If you have libraries of your own
>>> in another directory, you can add that directory to the load path.
>>> Unlike most other variables described in this manual, ‘load-path’ cannot
>>> be changed via the Customize interface (*note Easy Customization::), but
>>> you can add a directory to it by putting a line like this in your init
>>> file (*note Init File::):
>>> 
>>>      (add-to-list 'load-path "/path/to/my/lisp/library")

-- 
Ihor Radchenko,
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: org-assert-version considered harmful

[Stefan Monnier]

>> In my book step 3 above is a mistake (even if moved to step 2).
> I am confused.
> AFAIK, changing the load-path is a common way for users to install
> packages manually.

No, you're not confused, I just think that installing packages manually
(including messing with `load-path` and writing `(autoload ...)` in your
init file) is very last century :-)


        Stefan

Re: org-assert-version considered harmful

[Tim Cross]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> In my book step 3 above is a mistake (even if moved to step 2).
>> I am confused.
>> AFAIK, changing the load-path is a common way for users to install
>> packages manually.
>
> No, you're not confused, I just think that installing packages manually
> (including messing with `load-path` and writing `(autoload ...)` in your
> init file) is very last century :-)
>

but Emacs *is* last century! :-)

It is the fact we *can* manipulate load-path, autoloads and manually
control what is installed which makes it so powerful. See how far you
get when a core VS Code extension you rely on changes in a manner you
don't like and you want to revert to the previous version.

I know your comment was tongue in cheek, but I also do see some danger
in a future where we only interact with the well defined 'surface' layer
of software like Emacs and only a few hard core devs actually get into
crafting their init.el file.

It could be the reason we seem to be seeing an increase in the type of
issues which kicked this off is because fewer people are familiar with
manipulating load-path and autoloads, Less familiarity means less
familiarity with the common pitfalls and issues you may run into. 

Re: org-assert-version considered harmful

[Bastien]

Ihor Radchenko <yantar92@gmail.com> writes:

> Then, I am inclined towards easing the version check to (org-version)
> instead of (org-git-version).

FWIW strong +1 here.

-- 
 Bastien

Re: org-assert-version considered harmful

[Ihor Radchenko]

Bastien <bzg@gnu.org> writes:

> Ihor Radchenko <yantar92@gmail.com> writes:
>
>> Then, I am inclined towards easing the version check to (org-version)
>> instead of (org-git-version).
>
> FWIW strong +1 here.

There is one more concern we may need to solve prior to changing
org-git-version to org-version.

Currently, main and bugfix branches both have (org-version) ; => "9.5.5"
As a result, the assertion will not catch the important case when users
mix Org version installed via package.el and Org version installed from
git.

Should we use the next planned release version number on main branch as
a convention?

-- 
Ihor Radchenko,
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: org-assert-version considered harmful

[Timothy]

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

Hi Ihor,

> Should we use the next planned release version number on main branch as
> a convention?

For what it’s worth, in my own build of Org I set `org-release' to
`MAJOR.(MINOR+1).0', e.g. when the last tag is `9.5.5' I set `org-release' to `9.6.0'.
The `.0' suffix serves to differentiate this from the later `9.6' release, looking
at historical first-minor-version releases (`9.5', `9.4', `9.3', etc.) the patch
version seems to be omitted in each initial minor release.

I think it could make sense to apply this approach to the main branch.

All the best,
Timothy

Re: org-assert-version considered harmful

[Bastien]

Ihor Radchenko <yantar92@gmail.com> writes:

> Currently, main and bugfix branches both have (org-version) ; => "9.5.5"
> As a result, the assertion will not catch the important case when users
> mix Org version installed via package.el and Org version installed from
> git.
>
> Should we use the next planned release version number on main branch as
> a convention?

I'd rather use the value set in the ";; Version:" header.

It is "9.5.5" in the bugfix branch and "9.6-dev" on the main branch.

I'm not even sure we should keep `org-git-version' at all: if we need
to distinguish between pre-release states, it seems easy enough to set
the header as 9.6rc1, 9.6rc2, etc.

WDYT?

PS: I have a vague memory that Stefan suggested to look at how things
are done on bbdb.el:

https://git.savannah.gnu.org/cgit/emacs/elpa.git/tree/lisp/bbdb.el?h=externals/bbdb#n4750

If we can remove the complex Make machinery we have right now, I'd be
very happy.  One reason for this machinery was to avoid merge conflict
(thanks to getting rid of the Version: header), but we do have these
conflicts (now that the header is back) and they are easy to solve.

-- 
 Bastien

Re: org-assert-version considered harmful

[Ihor Radchenko]

Bastien <bzg@gnu.org> writes:

>> Should we use the next planned release version number on main branch as
>> a convention?
>
> I'd rather use the value set in the ";; Version:" header.
>
> It is "9.5.5" in the bugfix branch and "9.6-dev" on the main branch.

Makes sense.

> I'm not even sure we should keep `org-git-version' at all: if we need
> to distinguish between pre-release states, it seems easy enough to set
> the header as 9.6rc1, 9.6rc2, etc.
>
> WDYT?

org-git-version is very useful when people report bugs.
M-x org-submit-bug-report supplies org-git-version output for bug
subject. Thus, we can easily check which git commit their build
corresponds to.

> PS: I have a vague memory that Stefan suggested to look at how things
> are done on bbdb.el:
>
> https://git.savannah.gnu.org/cgit/emacs/elpa.git/tree/lisp/bbdb.el?h=externals/bbdb#n4750
>
> If we can remove the complex Make machinery we have right now, I'd be
> very happy.  One reason for this machinery was to avoid merge conflict
> (thanks to getting rid of the Version: header), but we do have these
> conflicts (now that the header is back) and they are easy to solve.

I am not very familiar with all the code paths our Makefile and
autoloads take from setting ORG-VERSION to generating the appropriate
Elisp constants.

However, I do note that mk/targets.mk contains the following:

ifneq ($(wildcard .git),)
  ORGVERSION ?= $(subst release_,,$(shell git describe --match release\* --abbrev=0 HEAD))
  ifeq ($(ORGVERSION),)
    # In elpa.git, there are no tags available.  Fall back to using
    # the org.el header.
    ORGVERSION := $(patsubst %-dev,%,$(shell $(BATCH) --eval "(require 'lisp-mnt)" \
      --visit lisp/org.el --eval '(princ (lm-header "version"))'))
    GITVERSION ?= $(ORGVERSION)-g$(shell git rev-parse --short=6 HEAD)
  else
    GITVERSION ?= $(shell git describe --match release\* --abbrev=6 HEAD)
  endif
  GITSTATUS  ?= $(shell git status -uno --porcelain)
else
 -include mk/version.mk
  GITVERSION ?= N/A
  ORGVERSION ?= N/A
endif

Note that we already have a way to parse Org version from lisp/org.el,
similar to what the commit you referenced does.
It is just that this code path is not used by default.

We can remove the current default and simply use

    ORGVERSION := $(patsubst %-dev,%,$(shell $(BATCH) --eval "(require 'lisp-mnt)" \
      --visit lisp/org.el --eval '(princ (lm-header "version"))'))
    GITVERSION ?= $(ORGVERSION)-g$(shell git rev-parse --short=6 HEAD)

all the time.

I do not know if more involved fix is required (because I am not
familiar enough with the relevant code).

-- 
Ihor Radchenko,
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: org-assert-version considered harmful

[Bastien]

Ihor Radchenko <yantar92@gmail.com> writes:

> org-git-version is very useful when people report bugs.
> M-x org-submit-bug-report supplies org-git-version output for bug
> subject. Thus, we can easily check which git commit their build
> corresponds to.

Let's keep `org-git-version'.  If we manage to release Org more often
(minor and major versions), I doubt keep `org-git-version' will remain
that useful in practise, though.  Let's revisit the topic then.

> I am not very familiar with all the code paths our Makefile and
> autoloads take from setting ORG-VERSION to generating the appropriate
> Elisp constants.
>
> However, I do note that mk/targets.mk contains the following:
>
> ifneq ($(wildcard .git),)
>   ORGVERSION ?= $(subst release_,,$(shell git describe --match release\* --abbrev=0 HEAD))
>   ifeq ($(ORGVERSION),)
>     # In elpa.git, there are no tags available.  Fall back to using
>     # the org.el header.
>     ORGVERSION := $(patsubst %-dev,%,$(shell $(BATCH) --eval "(require 'lisp-mnt)" \
>       --visit lisp/org.el --eval '(princ (lm-header "version"))'))
>     GITVERSION ?= $(ORGVERSION)-g$(shell git rev-parse --short=6 HEAD)
>   else
>     GITVERSION ?= $(shell git describe --match release\* --abbrev=6 HEAD)
>   endif
>   GITSTATUS  ?= $(shell git status -uno --porcelain)
> else
>  -include mk/version.mk
>   GITVERSION ?= N/A
>   ORGVERSION ?= N/A
> endif
>
> Note that we already have a way to parse Org version from lisp/org.el,
> similar to what the commit you referenced does.
> It is just that this code path is not used by default.

I'd favor using it by default.

When using Org from the main branch of the git repository,
M-x org-version RET should return this:

"Org mode version 9.6-dev (release_9.5.5-822-g0a6a56 @ [load-path])"

> We can remove the current default and simply use
>
>     ORGVERSION := $(patsubst %-dev,%,$(shell $(BATCH) --eval "(require 'lisp-mnt)" \
>       --visit lisp/org.el --eval '(princ (lm-header "version"))'))
>     GITVERSION ?= $(ORGVERSION)-g$(shell git rev-parse --short=6 HEAD)
>
> all the time.
>
> I do not know if more involved fix is required (because I am not
> familiar enough with the relevant code).

Can you provide a patch for the above suggestions?  I'll test and see
if more fixes are needed, even though I'm also not that familiar with
the code either.

-- 
 Bastien

Re: org-assert-version considered harmful

[Ihor Radchenko]

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

Bastien <bzg@gnu.org> writes:

> Ihor Radchenko <yantar92@gmail.com> writes:
>
>> org-git-version is very useful when people report bugs.
>> M-x org-submit-bug-report supplies org-git-version output for bug
>> subject. Thus, we can easily check which git commit their build
>> corresponds to.
>
> Let's keep `org-git-version'.  If we manage to release Org more often
> (minor and major versions), I doubt keep `org-git-version' will remain
> that useful in practise, though.  Let's revisit the topic then.

I think that it will still be useful. In particular, consider major new
feature development. We may take a longer delay between releases then.

>> Note that we already have a way to parse Org version from lisp/org.el,
>> similar to what the commit you referenced does.
>> It is just that this code path is not used by default.
>
> I'd favor using it by default.
>
> When using Org from the main branch of the git repository,
> M-x org-version RET should return this:
>
> "Org mode version 9.6-dev (release_9.5.5-822-g0a6a56 @ [load-path])"

The code I quoted explicitly removes the "-dev" part. Would you prefer
to keep it?

> Can you provide a patch for the above suggestions?  I'll test and see
> if more fixes are needed, even though I'm also not that familiar with
> the code either.

See the attached.
After the patch, org-version returns
Org mode version 9.6 (release_9.5.5-830-g77f9e1 @ [load-path])


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-mk-targets.mk-ORGVERSION-Prefer-lisp-org.el-version-.patch --]
[-- Type: text/x-patch, Size: 1811 bytes --]

From 77f9e16d7436ca629384e6574f2231e275ea8447 Mon Sep 17 00:00:00 2001
Message-Id: <77f9e16d7436ca629384e6574f2231e275ea8447.1664104208.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Sun, 25 Sep 2022 19:02:17 +0800
Subject: [PATCH] * mk/targets.mk (ORGVERSION): Prefer lisp/org.el version
 header

Do not use the latest Git tag.  Prefer the Version header in org.el.

The Git tag on main branch is only available for the latest release.
Before this commit, development Org version was indistinguishable from
the release version.

See https://orgmode.org/list/8735cfn44v.fsf@gnu.org
---
 mk/targets.mk | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/mk/targets.mk b/mk/targets.mk
index 5cba63e21..518635737 100644
--- a/mk/targets.mk
+++ b/mk/targets.mk
@@ -11,16 +11,10 @@ INSTSUB       = $(SUBDIRS:%=install-%)
 ORG_MAKE_DOC ?= info html pdf
 
 ifneq ($(wildcard .git),)
-  ORGVERSION ?= $(subst release_,,$(shell git describe --match release\* --abbrev=0 HEAD))
-  ifeq ($(ORGVERSION),)
-    # In elpa.git, there are no tags available.  Fall back to using
-    # the org.el header.
-    ORGVERSION := $(patsubst %-dev,%,$(shell $(BATCH) --eval "(require 'lisp-mnt)" \
-      --visit lisp/org.el --eval '(princ (lm-header "version"))'))
-    GITVERSION ?= $(ORGVERSION)-g$(shell git rev-parse --short=6 HEAD)
-  else
-    GITVERSION ?= $(shell git describe --match release\* --abbrev=6 HEAD)
-  endif
+  # Use the org.el header.
+  ORGVERSION := $(patsubst %-dev,%,$(shell $(BATCH) --eval "(require 'lisp-mnt)" \
+    --visit lisp/org.el --eval '(princ (lm-header "version"))'))
+  GITVERSION ?= $(shell git describe --match release\* --abbrev=6 HEAD)
   GITSTATUS  ?= $(shell git status -uno --porcelain)
 else
  -include mk/version.mk
-- 
2.35.1


[-- Attachment #3: Type: text/plain, Size: 206 bytes --]


-- 
Ihor Radchenko,
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: org-assert-version considered harmful

[Bastien]

Ihor Radchenko <yantar92@gmail.com> writes:

> I think that it will still be useful. In particular, consider major new
> feature development. We may take a longer delay between releases then.

Yes.

> The code I quoted explicitly removes the "-dev" part. Would you prefer
> to keep it?

Yes, let's keep it, otherwise (org-release) reads like a lie.

Why is it necessary to emit org-version.el?

We could have (defun org-version ...) and (defun org-git-version ...)
from within org.el, right?

Also, I don't think we need org-release: the info org-version provides
is enough to know if you are loading Org from a stable (ELPA) release
or from a local git repository.

WDYT?

> See the attached.

Tested and works fine, modulo the -dev part that we should keep.

Thanks!

-- 
 Bastien

Re: org-assert-version considered harmful

[Ihor Radchenko]

Bastien <bzg@gnu.org> writes:

>> The code I quoted explicitly removes the "-dev" part. Would you prefer
>> to keep it?
>
> Yes, let's keep it, otherwise (org-release) reads like a lie.
>
> Why is it necessary to emit org-version.el?
>
> We could have (defun org-version ...) and (defun org-git-version ...)
> from within org.el, right?

org-version is already defined in org.el (using org-git-version and org-release)
org-git-version requires running git.
org-release could be defined in org.el

> Also, I don't think we need org-release: the info org-version provides
> is enough to know if you are loading Org from a stable (ELPA) release
> or from a local git repository.
>
> WDYT?

They are not the same. org-git-version uses tags + HEAD. org-release
uses lisp/org.el (after the patch).

Also, they are used by Makefile to generate orgcard.tex
We need to be careful in this area.

This Makefile + Elisp usage is what makes me uncomfortable changing
things in this area.

>> See the attached.
>
> Tested and works fine, modulo the -dev part that we should keep.

Note that in
Org mode version 9.6 (release_9.5.5-830-g77f9e1 @ [load-path])

release_9.5.5 while version is 9.6
I feel like you missed this detail.

-- 
Ihor Radchenko,
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: org-assert-version considered harmful

[Bastien]

Ihor Radchenko <yantar92@gmail.com> writes:

> Also, they are used by Makefile to generate orgcard.tex
> We need to be careful in this area.

Yes...

> Note that in
> Org mode version 9.6 (release_9.5.5-830-g77f9e1 @ [load-path])
>
> release_9.5.5 while version is 9.6
> I feel like you missed this detail.

Yes I did :)

Still, "Org mode version 9.6-pre (...)" is more accurate IMO.

-- 
 Bastien

Re: org-assert-version considered harmful

[Ihor Radchenko]

Bastien <bzg@gnu.org> writes:

> Still, "Org mode version 9.6-pre (...)" is more accurate IMO.

Ok.
Pushed the patch + version change onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=c29d3e997d703f6bf14db559e351729cc25e4f26
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=c8e0a402df91e168e1ec263a617b4cec6eb29e2d

-- 
Ihor Radchenko,
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: org-assert-version considered harmful

[Ihor Radchenko]

Bastien <bzg@gnu.org> writes:

>> The code I quoted explicitly removes the "-dev" part. Would you prefer
>> to keep it?
>
> Yes, let's keep it, otherwise (org-release) reads like a lie.

Note that 9.6-dev is not a valid package version string.
See `version-to-list'.

One valid option is 9.6-pre

-- 
Ihor Radchenko,
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: org-assert-version considered harmful

[Bastien]

Ihor Radchenko <yantar92@gmail.com> writes:

> One valid option is 9.6-pre

Let's go for this one.  I've add a note about our conventions for the
;; Version header in https://orgmode.org/worg/org-maintenance.html

-- 
 Bastien

Re: org-assert-version considered harmful

[Ihor Radchenko]

Bastien <bzg@gnu.org> writes:

> Ihor Radchenko <yantar92@gmail.com> writes:
>
>> Then, I am inclined towards easing the version check to (org-version)
>> instead of (org-git-version).
>
> FWIW strong +1 here.

Done now.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=73f25bba8ffb9fe434486832c6acb98794dd2e69
Note that I used `org-release' macro. `org-version' would require
loading 'org.

-- 
Ihor Radchenko,
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: org-assert-version considered harmful

[Bastien]

Ihor Radchenko <yantar92@gmail.com> writes:

> Done now.

Thanks!

-- 
 Bastien

RE: org-assert-version considered harmful

[Cook, Malcolm]

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

Hello,

I found this recent thread researching why my strategy for staying abreast with org head had started failing with invalid-function "org-assert-version"

My strategy had been to build org initially with


` cd ~/.emacs.d && git clone https://git.savannah.gnu.org/git/emacs/org-mode.git &&   cd org-mode && make autoloads && make`

and ensure this clone of org was picked up in my "~/.emacs.d/org-mode/lisp by including the following in my .init.el very early (right after bootstrapping the package system and use-package):

(use-package
  :pin manual
  :load-path "~/.emacs.d/org-mode/lisp"
)

Then, when I occasionally wished to update org, I would

`cd ~/.emacs.d/org-mode && git pull && make autoloads && make`

Recently I started getting errors invalid-function "org-assert-version".

Upon cursory reading of this thread I guessed that I could fix them by adding a `make clean` to my update mantra.

It worked.

Am I advised to do otherwise?  Is there a best/better practice?

Thanks,

~Malcolm

[-- Attachment #2: Type: text/html, Size: 44232 bytes --]

Re: [External] : RE: org-assert-version considered harmful

[Daniel Ortmann]

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

Malcolm,
I also ran into troubles which are similar, apparently due to mixed 
org-mode versions; we've got to load org-mode before emacs tries to do 
it for us or we get mixed stuff.

My resolution was to load the org-mode path first in my init.el file and 
then require org:

    (add-to-list 'load-path "/home/dortmann/src/git-org-mode/lisp")
    (require 'org)

And then I build with something like this:

    dortmann@ddo-linux:src$ cd ~/src/git-org-contrib/ && git pull && cd
    ~/src/git-org-mode/ && git pull && make all && make autoloads && cd
    ~/src/git-emacs-master/ && git pull && make all && sudo -H make
    install && cd ..


Then only an occasional 'make bootstrap' is required in the emacs build dir.




On 10/31/22 09:11, Cook, Malcolm wrote:
>
> Hello,
>
> I found this recent thread researching why my strategy for staying 
> abreast with org head had started failing with invalid-function 
> "org-assert-version"
>
> My strategy had been to build org initially with
>
> ` cd ~/.emacs.d && git clone 
> https://git.savannah.gnu.org/git/emacs/org-mode.git 
> <https://urldefense.com/v3/__https://git.savannah.gnu.org/git/emacs/org-mode.git__;!!ACWV5N9M2RV99hQ!NJTKsXYcJrTb8d5ZN-S1HlVfATQrUIHtWtEz4qZmvjlGuMcS-6MG89rA3dDSBwxECGJw1YoNopf635M$> 
> &&cd org-mode && make autoloads && make`
>
> and ensure this clone of org was picked up in my 
> "~/.emacs.d/org-mode/lisp by including the following in my .init.el 
> very early (right after bootstrapping the package system and use-package):
>
> (use-package
>
> :pin manual
>
> :load-path "~/.emacs.d/org-mode/lisp"
>
> )
>
> Then, when I occasionally wished to update org, I would
>
> `cd ~/.emacs.d/org-mode && git pull && make autoloads && make`
>
> Recently I started getting errors invalid-function "org-assert-version".
>
> Upon cursory reading of this thread I guessed that I could fix them by 
> adding a `make clean` to my update mantra.
>
> It worked.
>
> Am I advised to do otherwise?Is there a best/better practice?
>
> Thanks,
>
> ~Malcolm
>

[-- Attachment #2: Type: text/html, Size: 45606 bytes --]

RE: [External] : RE: org-assert-version considered harmful

[Cook, Malcolm]

> Malcolm,
> I also ran into troubles which are similar, apparently due to mixed org-mode versions; we've got to load org-mode before emacs tries to do it for us or we get mixed stuff.

Thanks Daniel, yes, I understand the need to load org early, and am sure that my approach succeeds in doing so (without the need to re-make emacs-master).

Nonetheless, I recently found that that the addition of a  `make clean` on org-mode was necessary to delete the compiled .elc files in which (apparently?) used macros whose definition had changed in the interim.

Cheers
> 
> My resolution was to load the org-mode path first in my init.el file and then require org:
> (add-to-list 'load-path "/home/dortmann/src/git-org-mode/lisp")
> (require 'org)
> And then I build with something like this:
> dortmann@ddo-linux:src$ cd ~/src/git-org-contrib/ && git pull && cd ~/src/git-org-mode/ && git pull && make all && make autoloads && cd ~/src/git-emacs-master/ && git pull && make all && sudo -H make install && cd ..
> 
> Then only an occasional 'make bootstrap' is required in the emacs build dir.
> 
> 
> 
> On 10/31/22 09:11, Cook, Malcolm wrote:
> Hello,
>  
> I found this recent thread researching why my strategy for staying abreast with org head had started failing with invalid-function "org-assert-version"
>  
> My strategy had been to build org initially with
>  
>  
>        ` cd ~/.emacs.d && git clone https://urldefense.com/v3/__https://git.savannah.gnu.org/git/emacs/org-mode.git__;!!ACWV5N9M2RV99hQ!NJTKsXYcJrTb8d5ZN-S1HlVfATQrUIHtWtEz4qZmvjlGuMcS-6MG89rA3dDSBwxECGJw1YoNopf635M$ &&   cd org-mode && make autoloads && make` 
>  
> and ensure this clone of org was picked up in my "~/.emacs.d/org-mode/lisp by including the following in my .init.el very early (right after bootstrapping the package system and use-package):
>  
> (use-package 
>   :pin manual 
>   :load-path "~/.emacs.d/org-mode/lisp"
> )
>  
> Then, when I occasionally wished to update org, I would
>  
>        `cd ~/.emacs.d/org-mode && git pull && make autoloads && make`
>         
> Recently I started getting errors invalid-function "org-assert-version".
>  
> Upon cursory reading of this thread I guessed that I could fix them by adding a `make clean` to my update mantra.
>  
> It worked.
>  
> Am I advised to do otherwise?  Is there a best/better practice?
>  
> Thanks,
>  
> ~Malcolm
> 
> 

Re: org-assert-version considered harmful

[Tim Cross]

"Cook, Malcolm" <MEC@stowers.org> writes:

> Hello,
>
> I found this recent thread researching why my strategy for staying abreast with org head had started failing with invalid-function
> "org-assert-version"
>
> My strategy had been to build org initially with
>
> ` cd ~/.emacs.d && git clone https://git.savannah.gnu.org/git/emacs/org-mode.git &&   cd org-mode && make autoloads && make
> ` 
> and ensure this clone of org was picked up in my "~/.emacs.d/org-mode/lisp by including the following in my .init.el very early
> (right after bootstrapping the package system and use-package):
>
> (use-package 
>
>   :pin manual 
>
>   :load-path "~/.emacs.d/org-mode/lisp"
>
> )
>
> Then, when I occasionally wished to update org, I would
>
> `cd ~/.emacs.d/org-mode && git pull && make autoloads && make`
>
> Recently I started getting errors invalid-function "org-assert-version".
>
> Upon cursory reading of this thread I guessed that I could fix them by adding a `make clean` to my update mantra.
>
> It worked.
>
> Am I advised to do otherwise?  Is there a best/better practice?
>

I think it is good practice to always do make clean for any code you
build from sources yourself. There is a reason most Makefiles have a
'clean' target and when it comes to building software, starting from a
known clean state is critical. This can make the build slower, but for
small packages like org mode, the difference is insignificant. Always
safer to do make clean before make. Alternative approaches really only
necessary in larger and more complex source trees where there can be
significant time differences between full and incremental builds
(i.e. Emacs source tree has 4 different 'clean' targets; clean,
extraclean, distclean and bootstrap).

RE: org-assert-version considered harmful

[Ihor Radchenko]

"Cook, Malcolm" <MEC@stowers.org> writes:

> Then, when I occasionally wished to update org, I would
>
> `cd ~/.emacs.d/org-mode && git pull && make autoloads && make`
>
> Recently I started getting errors invalid-function "org-assert-version".
>
> Upon cursory reading of this thread I guessed that I could fix them by adding a `make clean` to my update mantra.

It should not be necessary and it does not happen on my side (as you can
imagine, I re-compile very often).

Could you provide more details?

-- 
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: org-assert-version considered harmful

[Cook, Malcolm]

> > Then, when I occasionally wished to update org, I would
> >
> > `cd ~/.emacs.d/org-mode && git pull && make autoloads && make`
> >
> > Recently I started getting errors invalid-function "org-assert-version".
> >
> > Upon cursory reading of this thread I guessed that I could fix them by adding a `make clean` to my update mantra.
> 
> It should not be necessary and it does not happen on my side (as you can
> imagine, I re-compile very often).

Perhap's my issue stems from the particular versions of org I was upgrading between and/or (earlier) poor management of multiple contending org versions (e.g. git head v. melpa v. system).

> 
> Could you provide more details?

At this point, I am not seeking to reproduce the issue, but rather to ensure that my current practice is "best" practice, given my aim of staying current with head (understanding and accepting that this may bring its own instabilities).

So the details I can provide are probably around how I 

	protect against multiple contending org versions
	obtain and build org sources
	load/require/use the built org sources

The very first thing in my init.el is intended to help protect against contending org versions:

```
(require 'cl-seq)
(delete (car (cl-member "lisp/org" load-path :test  #'string-match)) 
	;; "as extra level of precaution against getting the built-in
	;; org-mode, I ensure it never gets loaded" - kudos:
	;; https://lists.gnu.org/archive/html/emacs-orgmode/2022-02/msg00130.html
	load-path)
```

If I want to install the latest org from melpa, I never do this from within an active emacs session but rather from the (linux) command line as:

```
emacs -Q -batch -eval "(progn (require 'package) (package-initialize)  (package-refresh-contents) (package-install 'org))"
```

But more often, I strive to stay abreast of developments, and when I see an issue I care about discussed as being addressed with a source code change, I go get it

```
cd ~/.emacs.d/org-mode && git pull && make clean && make autoloads && make PERL5LIB=
```

And then relaunch emacs, where it gets picked up due to:

```
(use-package org ;org-plus-contrib			; instead of org-mode
  :pin manual 
  :load-path "~/.emacs.d/org-mode/lisp"
...
)
```

... which occurs very early in my init file (just after bootstrapping package system and latest use-package).

So, I've got (again) a working strategy.  

I'm really wondering if all this is needlessly complex.

Anyway, thanks for chiming in!

RE: org-assert-version considered harmful

[Ihor Radchenko]

"Cook, Malcolm" <MEC@stowers.org> writes:

>> It should not be necessary and it does not happen on my side (as you can
>> imagine, I re-compile very often).
>
> Perhap's my issue stems from the particular versions of org I was upgrading between and/or (earlier) poor management of multiple contending org versions (e.g. git head v. melpa v. system).

That might be possible. Because Emacs does not properly update macro
definitions in the already compiled files.

See https://orgmode.org/list/jwvsfkv5s7l.fsf-monnier+emacs@gnu.org

However, the current, more forgiving, version of org-assert-version
should only complain when upgrading to different Org version. make clean
is a good measure even during normal upgrades though. Because of the
Emacs limitation.

> ```
> cd ~/.emacs.d/org-mode && git pull && make clean && make autoloads && make PERL5LIB=
> ```
>
> And then relaunch emacs, where it gets picked up due to:
>
> ```
> (use-package org ;org-plus-contrib			; instead of org-mode
>   :pin manual 
>   :load-path "~/.emacs.d/org-mode/lisp"
> ...
> )
> ```
>
> ... which occurs very early in my init file (just after bootstrapping package system and latest use-package).
>
> So, I've got (again) a working strategy.  
>
> I'm really wondering if all this is needlessly complex.

The above should be safe.
Whatever straight.el does also work for me as long as I put Org loading
early in my init.el.

-- 
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: org-assert-version considered harmful

[Cook, Malcolm]

>> It should not be necessary and it does not happen on my side (as you can
> >> imagine, I re-compile very often).
> >
> > Perhap's my issue stems from the particular versions of org I was upgrading between and/or (earlier) poor management of multiple contending org versions (e.g. git head v. melpa v. system).
> 
> That might be possible. Because Emacs does not properly update macro
> definitions in the already compiled files.
> 
> See https://orgmode.org/list/jwvsfkv5s7l.fsf-monnier+emacs@gnu.org

That is exactly the message which suggested my workaround of inserting a `make clean` into my manta.  Thanks for the confirmation!
  
> 
> However, the current, more forgiving, version of org-assert-version
> should only complain when upgrading to different Org version. make clean
> is a good measure even during normal upgrades though. Because of the
> Emacs limitation.
> 
> > ```
> > cd ~/.emacs.d/org-mode && git pull && make clean && make autoloads && make PERL5LIB=
> > ```
> >
> > And then relaunch emacs, where it gets picked up due to:
> >
> > ```
> > (use-package org ;org-plus-contrib ; instead of org-mode
> > :pin manual 
> > :load-path "~/.emacs.d/org-mode/lisp"
> > ...
> > )
> > ```
> >
> > ... which occurs very early in my init file (just after bootstrapping package system and latest use-package).
> >
> > So, I've got (again) a working strategy. 
> >
> > I'm really wondering if all this is needlessly complex.
> 
> The above should be safe.
> Whatever straight.el does also work for me as long as I put Org loading
> early in my init.el.

Good to have confirmation here again.  Thanks Ihor!

Re: org-assert-version considered harmful

[Tom Gillespie]

Sorry to be so late chiming in here. I've only now encountered this
due to the 9.6 release. In short, org-assert-version is an absolute
disaster for me.

At the very least org-assert-version should be non-fatal by default.

Without going into too much detail, an orgstrap shebang block is
forced to use the system installed version of org because it is
intended to work in the absence of an init.el file, or before an
init.el file can ever be loaded.

This means that if a newer version of org is installed then no code
can ever run again after that package is visible on the load path
because loading the newer version of org will immediately cause an
error when something (e.g. ob-python) tries to require org-macs,
terminating the execution of the orgstrap block prematurely. There is
no simple workaround, and there is no guaranteed workaround aside from
going to great lengths to only ever use the builtin version of org.

I'm not going to write anything else at the moment because I've just
spent the last 3+ hours trying to deal with this and am in an
extremely uncharitable mood.

Tom

Re: org-assert-version considered harmful

[Ihor Radchenko]

Tom Gillespie <tgbugs@gmail.com> writes:

> Without going into too much detail, an orgstrap shebang block is
> forced to use the system installed version of org because it is
> intended to work in the absence of an init.el file, or before an
> init.el file can ever be loaded.

Once you load built-in Org partially, attempting to load libraries from
newer Org version is a recipe for a disaster (random failures due to
changes in the newer Org).

> This means that if a newer version of org is installed then no code
> can ever run again after that package is visible on the load path
> because loading the newer version of org will immediately cause an
> error when something (e.g. ob-python) tries to require org-macs,
> terminating the execution of the orgstrap block prematurely. There is
> no simple workaround, and there is no guaranteed workaround aside from
> going to great lengths to only ever use the builtin version of org.

If you absolutely need to load older version of Org without affecting
user's choice to use newer version, you may consider unloading Org
first. See `unload-feature'.

Ideally, Emacs itself should provide ways to deal with multiple package
versions.

-- 
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: org-assert-version considered harmful

[Tom Gillespie]

Hi Ihor,
    I have been able to use `unload-feature' (with some additional
hackery) to get things working. It is not a pretty sight (there are a
bunch of pitfalls and unexpected side-effects that are likely bugs
when using unload-feature), so yes, ideally Emacs would make it
possible to have multiple versions of the same package. Many thanks!
Tom

Re: org-assert-version considered harmful

[Stefan Monnier]

> when using unload-feature), so yes, ideally Emacs would make it
> possible to have multiple versions of the same package.

Having multiple versions *installed* has been supported since "for ever"
(AFAIK support for that was added very early in `package.el`s life,
before Emacs-24.1).

Having multiple versions loaded at the same time in an Emacs session?
I think we're very far from supporting that.

BTW, rather than unloading, `package.el` relies on forcibly "re"loading
from the new version the already loaded files from the old version.
It suffers from a different set of problem :-(
[ I suspect `defvar` is the main problem for that solution.  ]


        Stefan

Re: org-assert-version considered harmful

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> BTW, rather than unloading, `package.el` relies on forcibly "re"loading
> from the new version the already loaded files from the old version.
> It suffers from a different set of problem :-(
> [ I suspect `defvar` is the main problem for that solution.  ]

Could it be possible to force require use certain package version and
automatically re-load a package when older version is being loaded?

-- 
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: org-assert-version considered harmful

[Ihor Radchenko]

Ihor Radchenko <yantar92@posteo.net> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> BTW, rather than unloading, `package.el` relies on forcibly "re"loading
>> from the new version the already loaded files from the old version.
>> It suffers from a different set of problem :-(
>> [ I suspect `defvar` is the main problem for that solution.  ]
>
> Could it be possible to force require use certain package version and
> automatically re-load a package when older version is being loaded?

After trying several more approaches, I now came up with yet another
idea. Instead of fiddling with load internals, compilation, or
load-path, what about making sure that Org libraries include version
info directly?

Every library will have

(require 'org-foo-9.X "org-foo")
(require 'org-bar-9.X "org-bar")
...
(provide 'org-lib)
(provide 'org-lib-9.X)

Then, we will make sure that the right version of the library is always
loaded. Simply because (provide 'org-lib-9.X) will be unique for
different Org releases.

There might still be a problem where we solve cyclic dependencies by
using declare-function, but those places should be fixed anyway, sooner
or later.

WDYT?

-- 
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: org-assert-version considered harmful

[Stefan Monnier]

Ihor Radchenko [2023-08-16 11:08:15] wrote:
> Ihor Radchenko <yantar92@posteo.net> writes:
>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> BTW, rather than unloading, `package.el` relies on forcibly "re"loading
>>> from the new version the already loaded files from the old version.
>>> It suffers from a different set of problem :-(
>>> [ I suspect `defvar` is the main problem for that solution.  ]
>>
>> Could it be possible to force require use certain package version and
>> automatically re-load a package when older version is being loaded?
>
> After trying several more approaches,

[ Side note: did you keep notes about the various approaches you tried
  and their respective downsides?  E.g. I'm curious what were the
  problems linked to my proposal of using a `require-with-check` like
  the one below my sig.  ]

> I now came up with yet another idea.  Instead of fiddling with load
> internals, compilation, or load-path, what about making sure that Org
> libraries include version info directly?

That should work.  It implies a fair bit more churn in the code, tho,
but I guess you plan to automate it via some scripts?


        Stefan


(defun require-with-check (feature &optional filename noerror)
  "If FEATURE is not already loaded, load it from FILENAME.
This is like `require' except if FEATURE is already a member of the list
‘features’, then we check if this was provided by a different file than the
one that we would load now (presumably because `load-path' has been
changed since the file was loaded)."
  (let ((lh load-history)
        (res (require feature filename noerror)))
    ;; If the `feature' was not yet provided, `require' just loaded the right
    ;; file, so we're done.
    (if (not (eq lh load-history)) res
      ;; If `require' did nothing, we need to make sure that was warranted.
      (let ((fn (locate-file (or filename (symbol-name feature))
                             load-path (get-load-suffixes))))
        ;; If the right file was indeed loaded already, we're done.
        (if (assoc fn load-history) res
          (funcall (if noerror #'warn #'error)
                   "Feature provided by other file: %S" feature)
          res)))))

Re: org-assert-version considered harmful

[Ihor Radchenko]

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> After trying several more approaches,
>
> [ Side note: did you keep notes about the various approaches you tried
>   and their respective downsides?  E.g. I'm curious what were the
>   problems linked to my proposal of using a `require-with-check` like
>   the one below my sig.  ]

My attempt to use shadowcheck idea you proposed failed with some very
strange errors and I gave up.
See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=62762#311

Although, part of the problem was
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=65286 which does not seem
to be reproducible by others.

>> I now came up with yet another idea.  Instead of fiddling with load
>> internals, compilation, or load-path, what about making sure that Org
>> libraries include version info directly?
>
> That should work.  It implies a fair bit more churn in the code, tho,
> but I guess you plan to automate it via some scripts?

Yeah. It will require search-and-replace across Org between releases.
But, at least, it is the most reliable way I can see without falling
into subtle details of Emacs load system.

I did some automation in a form of =make test= barking when `provide' do
not match Org version:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-testing-lisp-test-org-version.el-New-test-library.patch --]
[-- Type: text/x-patch, Size: 2759 bytes --]

From 9caf9ca7207ecebed3499432828833187436940d Mon Sep 17 00:00:00 2001
Message-ID: <9caf9ca7207ecebed3499432828833187436940d.1692189593.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Wed, 16 Aug 2023 13:59:49 +0300
Subject: [PATCH] * testing/lisp/test-org-version.el: New test library

(test-org-version/provide): Test for all the versioned features to be
provided according to current Org version.
---
 testing/lisp/test-org-version.el | 54 ++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 testing/lisp/test-org-version.el

diff --git a/testing/lisp/test-org-version.el b/testing/lisp/test-org-version.el
new file mode 100644
index 000000000..3c12c8d46
--- /dev/null
+++ b/testing/lisp/test-org-version.el
@@ -0,0 +1,54 @@
+;;; test-org-version.el --- Test Org version consistency  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2023  Ihor Radchenko
+
+;; Author: Ihor Radchenko <yantar92@posteo.net>
+;; Keywords: internal
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; This file implements internal checks for Org versioning.
+
+;;; Code:
+
+(require 'org-version)
+
+(ert-deftest test-org-version/provide ()
+  "Test versioned `provide' calls in Org libraries."
+  (let* (org-library)
+    (dolist (org-library-file
+             (directory-files
+              (expand-file-name
+	       (concat org-test-dir "../lisp"))
+              t "\\.el$"))
+      (setq org-library
+            (file-name-nondirectory
+             (file-name-sans-extension org-library-file)))
+      (unless (member org-library '("org-loaddefs" "org-version"))
+        (with-temp-buffer
+          (insert-file-contents org-library-file)
+          (goto-char (point-max))
+          (should (re-search-backward
+                   (rx-to-string
+                    `(seq
+                      bol (0+ space)
+                      "(provide" (0+ space)
+                      "'" ,(concat org-library "-" (org-release))
+                      (0+ space) ")"))
+                   nil t)))))))
+
+(provide 'test-org-version)
+;;; test-org-version.el ends here
-- 
2.41.0


[-- Attachment #3: Type: text/plain, Size: 224 bytes --]


-- 
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: org-assert-version considered harmful

[Stefan Monnier]

> My attempt to use shadowcheck idea you proposed failed with some very
> strange errors and I gave up.
> See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=62762#311

For this one I can see the problem.  You define:

    (defmacro org-require-with-shadowcheck (feature)
      [...]
      `(eval-and-compile ...))

so it will behave like a function, except that it's also
executed during macroexpansion, so the (require 'org-element) within
`org-set-modules` will be eagerly executed while loading `org.el` :-(

You should define `org-require-with-shadowcheck` as a function (just
like `require`).

> Although, part of the problem was
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=65286 which does not seem
> to be reproducible by others.

Haven't tried to look into this one yet.


        Stefan

Re: org-assert-version considered harmful

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> My attempt to use shadowcheck idea you proposed failed with some very
>> strange errors and I gave up.
>> See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=62762#311
>
> For this one I can see the problem.  You define:
>
>     (defmacro org-require-with-shadowcheck (feature)
>       [...]
>       `(eval-and-compile ...))
>
> so it will behave like a function, except that it's also
> executed during macroexpansion, so the (require 'org-element) within
> `org-set-modules` will be eagerly executed while loading `org.el` :-(
>
> You should define `org-require-with-shadowcheck` as a function (just
> like `require`).

But then it will not run during byte-compilation.
And compilation will yield

[...]
org-datetree.el:278:14: Warning: the function ‘org-cut-subtree’ is not known to be defined.
org-datetree.el:264:22: Warning: the function ‘org-up-heading-safe’ is not known to be defined.
org-datetree.el:263:35: Warning: the function ‘org-back-to-heading’ is not known to be defined.
org-datetree.el:257:37: Warning: the function ‘org-time-string-to-time’ is not known to be defined.
org-datetree.el:237:6: Warning: the function ‘org-paste-subtree’ is not known to be defined.
[...]

and no single macro from other library will be expanded.

-- 
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: org-assert-version considered harmful

[Stefan Monnier]

>> For this one I can see the problem.  You define:
>>
>>     (defmacro org-require-with-shadowcheck (feature)
>>       [...]
>>       `(eval-and-compile ...))
>>
>> so it will behave like a function, except that it's also
>> executed during macroexpansion, so the (require 'org-element) within
>> `org-set-modules` will be eagerly executed while loading `org.el` :-(
>>
>> You should define `org-require-with-shadowcheck` as a function (just
>> like `require`).
>
> But then it will not run during byte-compilation.

Yeah, I was assuming that you had replaced all `require`s with
`org-require-with-shadowcheck`, but that's probably not what you'd done.

Not knowing what you had done (beside define
`org-require-with-shadowcheck`), it's hard to reproduce/investigate, tho.


        Stefan

Re: org-assert-version considered harmful

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> But then it will not run during byte-compilation.
>
> Yeah, I was assuming that you had replaced all `require`s with
> `org-require-with-shadowcheck`, but that's probably not what you'd done.

That's exactly what I have done.

> Not knowing what you had done (beside define
> `org-require-with-shadowcheck`), it's hard to reproduce/investigate, tho.

https://git.sr.ht/~yantar92/org-mode/tree/feature/shadowcheck

-- 
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: org-assert-version considered harmful

[Stefan Monnier]

>>> But then it will not run during byte-compilation.
>> Yeah, I was assuming that you had replaced all `require`s with
>> `org-require-with-shadowcheck`, but that's probably not what you'd done.
> That's exactly what I have done.

Ah.  The issue comes from the fact that `require` is treated specially
by the byte compiler, so if you define `org-require-with-shadowcheck` as
a function it indeed won't do quite the same as `require`.

The way `require` is treated specially by the byte-compiler only affects
top-level uses (making them behave as if they're wrapped inside
`eval-and-compile`).  So you want to use `eval-and-compile` only for
those `require`s that are at top-level.

Personally, I'd only change those `require`s that load `org-macs`, which
should be enough to cover almost all cases (and I wouldn't just
silently reload the file, but I'd also emit a warning explaining that
we detected a bad situation and using a workaround since that
workaround is not 100% reliable anyway).


        Stefan

Re: org-assert-version considered harmful

[Ihor Radchenko]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>>> But then it will not run during byte-compilation.
>>> Yeah, I was assuming that you had replaced all `require`s with
>>> `org-require-with-shadowcheck`, but that's probably not what you'd done.
>> That's exactly what I have done.
>
> Ah.  The issue comes from the fact that `require` is treated specially
> by the byte compiler, so if you define `org-require-with-shadowcheck` as
> a function it indeed won't do quite the same as `require`.
>
> The way `require` is treated specially by the byte-compiler only affects
> top-level uses (making them behave as if they're wrapped inside
> `eval-and-compile`).  So you want to use `eval-and-compile` only for
> those `require`s that are at top-level.

I see. Then, I can resolve the issue simply by
(put 'org-require-with-shadowcheck 'byte-hunk-handler 'byte-compile-file-form-require)

> Personally, I'd only change those `require`s that load `org-macs`, which
> should be enough to cover almost all cases (and I wouldn't just
> silently reload the file, but I'd also emit a warning explaining that
> we detected a bad situation and using a workaround since that
> workaround is not 100% reliable anyway).

I first want to try the most generous way to reveal any potential
pitfalls.

Like recursive load that is still happening when I do

1. emacs -Q
2. M-x org-version (built-in)
3. M-: (push "/path/to/git/version/of/org/lisp" load-path)
4. M-x org-mode
5. Observe recursive loading error

-- 
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: org-assert-version considered harmful

[Ihor Radchenko]

Ihor Radchenko <yantar92@posteo.net> writes:

> 1. emacs -Q
> 2. M-x org-version (built-in)
> 3. M-: (push "/path/to/git/version/of/org/lisp" load-path)
> 4. M-x org-mode
> 5. Observe recursive loading error

... which is also happening with the other approach using (provide
'org-foo-9.7-pre)

progn: Recursive load: "/home/yantar92/Git/org-mode/lisp/org-element.el", "/home/yantar92/Git/org-mode/lisp/org.el", "/home/yantar92/Git/org-mode/lisp/org-element.el", "/home/yantar92/Git/org-mode/lisp/org.el", "/home/yantar92/Git/org-mode/lisp/org-element.el", "/home/yantar92/Git/org-mode/lisp/org.el", "/home/yantar92/Git/org-mode/lisp/org-element.el", "/home/yantar92/Git/org-mode/lisp/org.el", "/home/yantar92/Git/org-mode/lisp/org-element.el

I should be missing something.

-- 
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: org-assert-version considered harmful

[Max Nikulin]

On 16/08/2023 18:08, Ihor Radchenko wrote:
> Every library will have
> 
> (require 'org-foo-9.X "org-foo")
> (require 'org-bar-9.X "org-bar")
> ...
> (provide 'org-lib)
> (provide 'org-lib-9.X)

Are you going to update patch version of particular library on each 
change of macro definitions?

Re: org-assert-version considered harmful

[Ihor Radchenko]

Max Nikulin <manikulin@gmail.com> writes:

> On 16/08/2023 18:08, Ihor Radchenko wrote:
>> Every library will have
>> 
>> (require 'org-foo-9.X "org-foo")
>> (require 'org-bar-9.X "org-bar")
>> ...
>> (provide 'org-lib)
>> (provide 'org-lib-9.X)
>
> Are you going to update patch version of particular library on each 
> change of macro definitions?

At least, each time we release a new non-bugfix Org version.
Maybe also each time we make breaking change in a macro. But that
should not normally happen on bugfix, only when we merge main into
bugfix. So, just during major/minor releases should be good enough.

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