24.0.91; package.el (org): Macros in tar packages & order of byte compilation

24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Jambunathan K]

There have been constant flow of issues from users who install daily org
tar balls from GNU ELPA.

The problem concerns itself with the defmacros that are introduced in
the *daily* tar but are *unavailable* in the *emacs* core. At the end of
the package installation these new macros never get recognized as macros
and gets compiled in as function calls. This triggers crashes at a later
stage.

Note that there are no such defmacro issues that get reported from users
who install org from git repo using conventional make and make install.

The crux of the issue is this:

1. While building via Makefile, there is an implicit dependency that is
*enforced* via make rules and files with macro definitions are compiled
ahead of their consumers.

2. While building from ELPA, the compilation order seems to be
alphabetical. So the files get compiled bass ackwards. For example,
org-macs.el gets compiled after org-agenda.el.

In summary, there needs to be a way to specify the order in which files
are compiled in a multifile tar.

A supplementary question to (2):

During the package compilation, when encountering (require
'some-org-library-with-macros) does the library get loaded from *within*
the tarball or from the *emacs core*.

I hope this description is sufficient. I can cite actual posts from
orgmode mailing list if additional information is needed.

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Jambunathan K]

This is one of the most frequently reported problems in Org mailing
list.  This is also one of the problems that the current maintainer
understands very poorly.

Anyways closing the bug as OP.

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Eli Zaretskii]

> From: Jambunathan K <kjambunathan@gmail.com>
> Date: Thu, 24 Nov 2011 17:42:38 +0530
> Cc: emacs-orgmode@gnu.org, Stelian Iancu <stelian.iancu@gmail.com>
> 
> 1. While building via Makefile, there is an implicit dependency that is
> *enforced* via make rules and files with macro definitions are compiled
> ahead of their consumers.

That's not true: there are no dependencies defined in lisp/Makefile.in
in the Emacs source tree for Lisp files, with a very few exceptions.
Org Mode files certainly have no dependency rules in lisp/Makefile.in.
So the question why the problem does not happen while compiling Org in
Emacs remains.

> 2. While building from ELPA, the compilation order seems to be
> alphabetical. So the files get compiled bass ackwards. For example,
> org-macs.el gets compiled after org-agenda.el.
> 
> In summary, there needs to be a way to specify the order in which files
> are compiled in a multifile tar.

This was discussed several time, in the context of recompiling
multiple Lisp files while building Emacs, and the decision till now
was to ignore the issue.  While at least in principle one could write
a Lisp program that would analyze the various `require' and `load'
calls (possibly as side effect of byte compilation, like GCC does),
and generate Makefile rules with correct prerequisites, this is a
non-trivial project.

One simple band-aid is to remove all the *.elc files before
byte-compiling after resync.  This prolongs the compilation, but the
results are predictably correct.

> A supplementary question to (2):
> 
> During the package compilation, when encountering (require
> 'some-org-library-with-macros) does the library get loaded from *within*
> the tarball or from the *emacs core*.

Whichever is found first along load-path, I think.  See `openp'.

Re: bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Jambunathan K]

> Org Mode files certainly have no dependency rules in lisp/Makefile.in.

The Makefile - in devel repo of Orgmode - does define rules. Read on ...

> So the question why the problem does not happen while compiling Org in
> Emacs remains.

I believe the way the files are compiled makes a substantial difference.

When compiling with makefiles:

The compilation happens with *minimal* emacs and in batch mode.

--8<---------------cut here---------------start------------->8---
BATCH=$(EMACS) -batch -q -no-site-file -eval                             	
  "(setq load-path (cons (expand-file-name \"./lisp/\") 
  (cons \"$(lispdir)\" load-path)))"
--8<---------------cut here---------------end--------------->8---

As can be seen above, any (require 'something) of macro files in the
compiled elisp file has to be loaded from the development version
itself.

Furthermore there are dependencies like this in the Makefile:

--8<---------------cut here---------------start------------->8---
lisp/org.elc:		lisp/org-macs.el lisp/org-compat.el lisp/org-faces.el
lisp/org-agenda.elc:	lisp/org.el
--8<---------------cut here---------------end--------------->8---

(I believe removing the dependencies from the Makefiles will still do
the right thing because of the require directives in the compiled files
will load the development version and not the system version)

When compiling with package manager, the compilation happens from within
a running Emacs session and very likely the "old" Org files are already
loaded in to the runtime "inadvertently" by the user either by looking
at the org agenda for the day or may be by just viewing an Org file or
by the plain old (require 'org-whatever) out of habit in .emacs. 

While reporting macro issues, users never say whether they were already
running Org when they were trying to fetch and compile a new Org. They
think it is immaterial. I believe it matters

If "old" org and hence "old" org-macs is already loaded in the
environment when the package is installed, any subsequent (require
'something) will be essentially no-ops. (Can you confirm this?)

What ideally should happen is that during package compilation, a require
should *forcibly* load from the compiled package and not merely check
for availability of a feature symbol.

>> 2. While building from ELPA, the compilation order seems to be
>> alphabetical. So the files get compiled bass ackwards. For example,
>> org-macs.el gets compiled after org-agenda.el.
>> 
>> In summary, there needs to be a way to specify the order in which files
>> are compiled in a multifile tar.
>
> This was discussed several time, in the context of recompiling
> multiple Lisp files while building Emacs, and the decision till now
> was to ignore the issue.  While at least in principle one could write
> a Lisp program that would analyze the various `require' and `load'
> calls (possibly as side effect of byte compilation, like GCC does),
> and generate Makefile rules with correct prerequisites, this is a
> non-trivial project.

I have tried giving an explanation in the earlier paragraph.

> One simple band-aid is to remove all the *.elc files before
> byte-compiling after resync.  This prolongs the compilation, but the
> results are predictably correct.

We are talking of "automatic" compilation by package manager. What you
say applies to hand compilation via makefiles.

>> A supplementary question to (2):
>> 
>> During the package compilation, when encountering (require
>> 'some-org-library-with-macros) does the library get loaded from *within*
>> the tarball or from the *emacs core*.
>
> Whichever is found first along load-path, I think.  See `openp'.

What does package manager do?
-- 

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Glenn Morris]

Jambunathan K wrote:

> When compiling with package manager, the compilation happens from within
> a running Emacs session and very likely the "old" Org files are already
> loaded in to the runtime "inadvertently" by the user either by looking
> at the org agenda for the day or may be by just viewing an Org file or
> by the plain old (require 'org-whatever) out of habit in .emacs. 

There's your problem. The only way to reliably compile, especially
something where an old version might already be loaded, is to use a
fresh Emacs instance. There's no reason the "package manager" could not
spawn a separate Emacs in batch-mode as a sub-job to do the compilation.

cc-mode tries to have some voodoo to get around this, but please, please
don't go down that road.

I guess nobody ever expected the package manager to be used to load a
different version of something that was already in Emacs.

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Glenn Morris]

Glenn Morris wrote:

> fresh Emacs instance. There's no reason the "package manager" could not
> spawn a separate Emacs in batch-mode as a sub-job to do the compilation.

Very lightly tested version:

*** lisp/emacs-lisp/package.el	2011-11-20 03:48:53 +0000
--- lisp/emacs-lisp/package.el	2011-11-24 19:48:49 +0000
***************
*** 595,600 ****
--- 595,612 ----
  	(error "Package does not untar cleanly into directory %s/" dir))))
    (tar-untar-buffer))
  
+ (defun package-compile (directory)
+   "Compile the Lisp files in DIRECTORY."
+   (with-current-buffer (get-buffer-create "*package-compile*")
+     (goto-char (point-max))
+     (pop-to-buffer (current-buffer))
+     (or (zerop (call-process "emacs" nil t t "--batch" "--eval"
+ 			     (format
+ 			      "(progn (setq load-path (cons \"%s\" load-path))
+                                 (batch-byte-recompile-directory 0))" directory)
+ 			     directory))
+ 	(error "Compiling the package gave an error"))))
+ 
  (defun package-unpack (name version)
    (let* ((dirname (concat (symbol-name name) "-" version))
  	 (pkg-dir (expand-file-name dirname package-user-dir)))
***************
*** 603,610 ****
      (let* ((default-directory (file-name-as-directory package-user-dir)))
        (package-untar-buffer dirname)
        (package-generate-autoloads (symbol-name name) pkg-dir)
!       (let ((load-path (cons pkg-dir load-path)))
! 	(byte-recompile-directory pkg-dir 0 t)))))
  
  (defun package--write-file-no-coding (file-name)
    (let ((buffer-file-coding-system 'no-conversion))
--- 615,621 ----
      (let* ((default-directory (file-name-as-directory package-user-dir)))
        (package-untar-buffer dirname)
        (package-generate-autoloads (symbol-name name) pkg-dir)
!       (package-compile pkg-dir))))
  
  (defun package--write-file-no-coding (file-name)
    (let ((buffer-file-coding-system 'no-conversion))
***************
*** 645,652 ****
  	 pkg-file
  	 nil nil nil 'excl))
        (package-generate-autoloads file-name pkg-dir)
!       (let ((load-path (cons pkg-dir load-path)))
! 	(byte-recompile-directory pkg-dir 0 t)))))
  
  (defmacro package--with-work-buffer (location file &rest body)
    "Run BODY in a buffer containing the contents of FILE at LOCATION.
--- 656,662 ----
  	 pkg-file
  	 nil nil nil 'excl))
        (package-generate-autoloads file-name pkg-dir)
!       (package-compile pkg-dir))))
  
  (defmacro package--with-work-buffer (location file &rest body)
    "Run BODY in a buffer containing the contents of FILE at LOCATION.

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Stelian Iancu]

On Thu, Nov 24, 2011 at 20:09, Glenn Morris <rgm@gnu.org> wrote:
> Jambunathan K wrote:
>
>> When compiling with package manager, the compilation happens from within
>> a running Emacs session and very likely the "old" Org files are already
>> loaded in to the runtime "inadvertently" by the user either by looking
>> at the org agenda for the day or may be by just viewing an Org file or
>> by the plain old (require 'org-whatever) out of habit in .emacs.
>
> There's your problem. The only way to reliably compile, especially
> something where an old version might already be loaded, is to use a
> fresh Emacs instance. There's no reason the "package manager" could not
> spawn a separate Emacs in batch-mode as a sub-job to do the compilation.
>
> cc-mode tries to have some voodoo to get around this, but please, please
> don't go down that road.
>
> I guess nobody ever expected the package manager to be used to load a
> different version of something that was already in Emacs.
>

I am sorry to be asking a stupid question, but then, wouldn't restart
Emacs fix the issue and have the new compiled org files loaded? In my
case, that didn't seem to happen either (even though load-library
showed org-compat to be from ELPA).

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Glenn Morris]

Stelian Iancu wrote:

> I am sorry to be asking a stupid question, but then, wouldn't restart
> Emacs fix the issue and have the new compiled org files loaded?

No, because the files get compiled with a mix of old and new code
loaded, so the compiled files are probably messed up. Restarting Emacs
would not help with that. (You'll definitely need to restart Emacs if
you had one version of Org loaded and want to switch to another.)

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Jambunathan K]

Stelian

> I am sorry to be asking a stupid question, but then, wouldn't restart
> Emacs fix the issue and have the new compiled org files loaded? In my
> case, that didn't seem to happen either (even though load-library
> showed org-compat to be from ELPA).

locate-library doesn't show what is *already* loaded. It will only shows
what *will* be loaded. 

The most important thing during package compilation is this: Make sure
there is no running instance of Org in any form. If this condition is
not satisfied then to-be-installed files will be compiled with old
definitions of macro. This is not what we want. We want the new files to
be compiled with new macro definitions.

Do things work for you when you start a *minimal* Emacs and *then* do
M-x list-packages -> install? You don't have to apply Glenn's patch to
get the desired behaviour (I am assuming here that you are probably not
that comfortable working with patches).

Jambunathan K.
-- 

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Stelian Iancu]

On Fri, Nov 25, 2011 at 04:31, Jambunathan K <kjambunathan@gmail.com> wrote:
>
> Stelian
>
>> I am sorry to be asking a stupid question, but then, wouldn't restart
>> Emacs fix the issue and have the new compiled org files loaded? In my
>> case, that didn't seem to happen either (even though load-library
>> showed org-compat to be from ELPA).
>
> locate-library doesn't show what is *already* loaded. It will only shows
> what *will* be loaded.
>
> The most important thing during package compilation is this: Make sure
> there is no running instance of Org in any form. If this condition is
> not satisfied then to-be-installed files will be compiled with old
> definitions of macro. This is not what we want. We want the new files to
> be compiled with new macro definitions.
>
> Do things work for you when you start a *minimal* Emacs and *then* do
> M-x list-packages -> install? You don't have to apply Glenn's patch to
> get the desired behaviour (I am assuming here that you are probably not
> that comfortable working with patches).
>

I will try with a minimal Emacs first. You're right, I don't know my
way around patches that well :-). I am on OSX and I suppose I could
use something like homebrew for including the patch and building.

> Jambunathan K.
> --
>

Re: bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Jambunathan K]

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


Stelian

> I don't know my way around patches that well :-).

OK. I am attaching the patched up version for you to try.

1. Put it somewhere that is NOT in your load-path.
2. M-x load-file RET /path/to/patched-up-package.el RET
3. Proceed with usual installation.
4. Once you are happy with the patch you can copy it to load-path 
5. Remember to delete the patch once you upgrade your Emacs


[-- Attachment #2: package.el --]
[-- Type: application/emacs-lisp, Size: 67254 bytes --]

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, 10125@debbugs.gnu.org,  stelian.iancu@gmail.com
> Date: Thu, 24 Nov 2011 14:09:38 -0500
> 
> I guess nobody ever expected the package manager to be used to load a
> different version of something that was already in Emacs.

Indeed.  Because doing this is asking for trouble: Emacs loads
packages in a certain order to avoid several subtle problems; doing
that in a different order defeats that.

I think package.el should test with featurep whether a version of a
package is already loaded, and refuse to load it into a running
session, or at least display a warning to that effect, suggesting to
restart Emacs.

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Jambunathan K]

Glenn

Glenn Morris <rgm@gnu.org> writes:
> Glenn Morris wrote:
>
>> fresh Emacs instance. There's no reason the "package manager" could not
>> spawn a separate Emacs in batch-mode as a sub-job to do the
>> compilation.

I will let Stelian or any aggrieved future parties to test the patch. 

[...context switch...]

I am wondering whether a knob to "not compile" installed packages is a
good option in general. Can the autoloads be still generated even if the
files themselves are not compiled?

Jambunathan K.

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Chong Yidong]

Glenn Morris <rgm@gnu.org> writes:

> Glenn Morris wrote:
>
>> fresh Emacs instance. There's no reason the "package manager" could not
>> spawn a separate Emacs in batch-mode as a sub-job to do the compilation.
>
> Very lightly tested version:

This uses the Emacs executable on the exec path, which might not be the
correct one.

I'm wary of making such a fundamental change to package.el without first
exploring possible package-specific work-arounds.  Could problematic
packages use an eval-when-compile form to forcibly load the correct
version of the required file?

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Stefan Monnier]

>>> fresh Emacs instance. There's no reason the "package manager" could not
>>> spawn a separate Emacs in batch-mode as a sub-job to do the compilation.
>> Very lightly tested version:
> This uses the Emacs executable on the exec path, which might not be the
> correct one.
> I'm wary of making such a fundamental change to package.el without first
> exploring possible package-specific work-arounds.

Yes, this should not a candidate for 24.1.

> Could problematic packages use an eval-when-compile form to forcibly
> load the correct version of the required file?

I think it's a sufficiently general problem that we might want to
provide some special support for a good solution (tho again, I doubt
there's something simple&safe enough to be a candidate for 24.1).

E.g. we could add to bytecomp.el the ability to force `require' to
reload a package if it's not already loaded from the file that
locate-library returns.


        Stefan

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Glenn Morris]

Stefan Monnier wrote:

> E.g. we could add to bytecomp.el the ability to force `require' to
> reload a package if it's not already loaded from the file that
> locate-library returns.

That will probably work fine most of the time, but what if a package is
restructed so that the feature names are different? Or a feature is
removed? Simply starting a fresh Emacs seems fine to me. Though there is
the issue of should it be a `-Q' one or not.

BTW, I was also thinking that rather than simply byte-recompiling,
package.el ought to check for a Makefile in the package directory, and
if it finds one, call make to build the package according to the rules
the package has defined. Though you would have to trust the package not
to do anything nasty.

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Glenn Morris]

Glenn Morris wrote:

> the package has defined. Though you would have to trust the package not
> to do anything nasty.

Of course, you already have to trust it since byte-compiling can run
arbitrary code.

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Glenn Morris]

Chong Yidong wrote:

> This uses the Emacs executable on the exec path, which might not be the
> correct one.

Yes; I wondered if (car command-line-args) was a reliable way to find
the actual name of the running Emacs binary?

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Glenn Morris]

Glenn Morris wrote:

>> This uses the Emacs executable on the exec path, which might not be the
>> correct one.
>
> Yes; I wondered if (car command-line-args) was a reliable way to find
> the actual name of the running Emacs binary?

Turns out I was looking for invocation-directory and invocation-name.

bug#10125: 24.0.91; package.el (org): Macros in tar packages & order of byte compilation

[Glenn Morris]

Eli Zaretskii wrote:

> I think package.el should test with featurep whether a version of a
> package is already loaded, and refuse to load it into a running
> session, or at least display a warning to that effect, suggesting to
> restart Emacs.

Yes, that might be better.

bug#10125: RFE: require and load-path-shadowing

[Achim Gratz]

Stefan Monnier writes:
>    I guess we could fork Emacs early on and keep this second process
>    around as a "process from which to generate new clean slates".

I've been thinking about something like this for a while… if it worked
at least as well as starting a new Emacs instance on all platforms, I'd
favor this approach.

> - outdated .elc file taking precedence over the new .el file can do
>   the same.

Yes, but you get a warning and can already arrange for this (by binding
the appropriate variables) to be no problem in practise.  See the way
org-reload works in current master (of the Org repo).

> - bytecompiling a file affects the running session by side-effects such
>   as requiring packages.

If that problem was finally solved that would be very welcome.

> I suggested a quick&dirty solution:
>> > E.g. we could add to bytecomp.el the ability to force `require' to
>> > reload a package if it's not already loaded from the file that
>> > locate-library returns.
>
> I still think it's not a bad option.

Would an advice work in this situation (given that require is a
primitive)?  If yes, I'd like to give it a try over the weekend.  If
not, I don't really see why require, more specifically the part that
checks features needs to be a primitive, so maybe it could be moved
partly to elisp.

> Of course, we'd still get trouble when the loading is not performed via
> `require' but via autoload (maybe we could try and attack this problem
> by allowing `autoload' to override an already existing definition, but
> that could be delicate).

That I'd like to split off from the discussion about require.

> I don't see why that would introduce a difficulty.

As long as the package is properly namespaced, why not allow for
removing all definitions pertaining to that entire namespace (features,
autoloads, definitions, …)?


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf Blofeld V1.15B11:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

bug#10125: RFE: require and load-path-shadowing

[Stefan Monnier]

>> I guess we could fork Emacs early on and keep this second process
>> around as a "process from which to generate new clean slates".
> I've been thinking about something like this for a while… if it worked
> at least as well as starting a new Emacs instance on all platforms, I'd
> favor this approach.

IIUC "fork" is not really an option for w32.

>> I suggested a quick&dirty solution:
>>> > E.g. we could add to bytecomp.el the ability to force `require' to
>>> > reload a package if it's not already loaded from the file that
>>> > locate-library returns.
>> I still think it's not a bad option.
> Would an advice work in this situation (given that require is a
> primitive)?

Yes, this subroutine is never directly called from C, so placing an
advice should work just fine.

> If yes, I'd like to give it a try over the weekend.  If not, I don't
> really see why require, more specifically the part that checks
> features needs to be a primitive, so maybe it could be moved partly
> to elisp.

AFAICT the only part of `require' which can't be written in Elisp right
now is the part that handles Vautoload_queue because that variable is
not exposed to Elisp (IIRC this variable is used to undo the effects of
a partially loaded file when the load bumps into an error midway
through; FWIW I'm not convinced this feature works reliably nowadays).

>> I don't see why that would introduce a difficulty.
> As long as the package is properly namespaced, why not allow for
> removing all definitions pertaining to that entire namespace (features,
> autoloads, definitions, …)?

We could try that, as well, but it would only work for those packages
that are "properly namespaced" (and there's no way to detect that
AFAIK).
Along the same lines, we could try to use unload-feature.


        Stefan

bug#10125: RFE: require and load-path-shadowing

[Achim Gratz]

Stefan Monnier writes:
>>> I guess we could fork Emacs early on and keep this second process
>>> around as a "process from which to generate new clean slates".
>> I've been thinking about something like this for a while… if it worked
>> at least as well as starting a new Emacs instance on all platforms, I'd
>> favor this approach.
>
> IIUC "fork" is not really an option for w32.

For the intended application spawn should work as well?

> Yes, this subroutine is never directly called from C, so placing an
> advice should work just fine.

OK, I'll give it a try.

>> As long as the package is properly namespaced, why not allow for
>> removing all definitions pertaining to that entire namespace (features,
>> autoloads, definitions, …)?
>
> We could try that, as well, but it would only work for those packages
> that are "properly namespaced" (and there's no way to detect that
> AFAIK).

True, but a package might indicate if it is "one of those" and get the
appropriate treatment from package manager.

> Along the same lines, we could try to use unload-feature.

I thought this was potentially dangerous, but reading the docstring
again maybe not.  Let me try that as well.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Q+, Q and microQ:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

bug#10125: RFE: require and load-path-shadowing

[Stefan Monnier]

>>>> I guess we could fork Emacs early on and keep this second process
>>>> around as a "process from which to generate new clean slates".
>>> I've been thinking about something like this for a while… if it worked
>>> at least as well as starting a new Emacs instance on all platforms, I'd
>>> favor this approach.
>> IIUC "fork" is not really an option for w32.
> For the intended application spawn should work as well?

Could be: depends on the precise semantics of spawn, which I don't know.

>> Along the same lines, we could try to use unload-feature.
> I thought this was potentially dangerous, but reading the docstring
> again maybe not.  Let me try that as well.

It's fundamentally tricky just in the same way as your proposed
"namespace cleanup": if you undefine a function that's still registered
on some hook, process filter, ... you may get subsequent errors, some of
which may render Emacs completely unusable.
So it's risky to call unload-feature on a random package, but it's not
too hard for a package to make sure it survives unload-feature.
Tho currently, there are some significant shortcomings (IIRC there are
cases where the package's autoloads aren't re-instated, for example).


        Stefan

bug#10125: RFE: require and load-path-shadowing

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Fri, 11 Jan 2013 17:52:33 -0500
> Cc: 10125@debbugs.gnu.org
> 
> >>>> I guess we could fork Emacs early on and keep this second process
> >>>> around as a "process from which to generate new clean slates".
> >>> I've been thinking about something like this for a while… if it worked
> >>> at least as well as starting a new Emacs instance on all platforms, I'd
> >>> favor this approach.
> >> IIUC "fork" is not really an option for w32.
> > For the intended application spawn should work as well?
> 
> Could be: depends on the precise semantics of spawn, which I don't know.

I'm barely following this thread, so please tell what semantics do you
need, and I will then try figuring out whether spawn does what you
need.

bug#10125: RFE: require and load-path-shadowing

[Achim Gratz]

Achim Gratz writes:
> Stefan Monnier writes:
>> Yes, this subroutine is never directly called from C, so placing an
>> advice should work just fine.
>
> OK, I'll give it a try.

Here's what I've come up with:

--8<---------------cut here---------------start------------->8---
;; some parts of Org might already have been used from a different
;; place, try to reload these parts from the current load-path
(defadvice require (before org-require-reload-when-shadowed
			   (feature &optional filename noerror)
			   activate compile preactivate)
  "Check whether a required feature has been shadowed by changing
`load-path' after it has been loaded and reload that feature from
current load-path in this case."
  (when (featurep feature)
    (let ((feature-name (or filename (symbol-name feature))))
      (when (string-match "^\\(org\\|ob\\)[.-]" feature-name)
	(let ((feature-lib  (file-name-directory (or (locate-library feature-name) "")))
	      (feature-dir  (file-name-directory (feature-file feature))))
	  ;(message "require-reload-when shadowed %s\n\t%s\n\t%s" feature-name feature-lib feature-dir)
	  (when (not (string= feature-lib feature-dir))
	    (message "Reloading %s" feature-name)
	    (unload-feature feature t)))))))
(require 'org-macs)
(require 'org-compat)
(require 'org-entities)
(require 'org-faces)
(require 'org-list)
(require 'org-pcomplete)
(require 'org-src)
(require 'org-footnote)
;; babel
(require 'ob)
(eval-and-compile (ad-unadvise 'require))
--8<---------------cut here---------------end--------------->8---

This takes care of a situation when Org is activated after some parts of
it have already been loaded (maybe via autoload) and the load-path has
been changed inbetween.  This uses unload-feature although it would
probably work to just (setq features (delq feature features)).

For package manager, a slightly more general version of the above (not
conditionalized on the feature starting with "org" or "ob") could be
used around the package compilation.  I haven't yet tried this, though.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldUserWavetables

bug#10125: RFE: require and load-path-shadowing

[Bastien]

Achim Gratz <Stromeko@nexgo.de> writes:

> Achim Gratz writes:
>> Stefan Monnier writes:
>>> Yes, this subroutine is never directly called from C, so placing an
>>> advice should work just fine.

I thought coding conventions prevented advising primitives?
I does not look clean to advise `require' here, just for Org.

>   "Check whether a required feature has been shadowed by changing
> `load-path' after it has been loaded and reload that feature from
> current load-path in this case."

I don't understand why we need this.  

This is at best a workaround to fix dirty installations.

I updated Org's installation process yesterday:

  http://orgmode.org/org.html#Installation

I made it clear that the user, if she wants to install Org on top of
the pre-bundled version, must choose between on of the three methods.
For each method, I explain how to set up the load-path (if needed) and
org.el will load the correct org-loaddefs.el, resetting autoloads
correctly.

In org.el, no Org package is required before org-loaddefs.el is loaded.

> This takes care of a situation when Org is activated after some parts of
> it have already been loaded (maybe via autoload) and the load-path has
> been changed inbetween.

I don't see why this should happen.  Setting the correct load-paths is
the first thing user should do in their .emacs.el.

-- 
 Bastien

bug#10125: RFE: require and load-path-shadowing

[Stefan Monnier]

>>>> Yes, this subroutine is never directly called from C, so placing an
>>>> advice should work just fine.
> I thought coding conventions prevented advising primitives?

Nothing prevents advising primitive functions (advising special forms
is another matter altogether).  But primitive functions can be called
from C directly (rather than via looking up the symbols' function cell)
in which case the advice will be ignored.

> I does not look clean to advise `require' here, just for Org.

It's just an experiment (AFAIK).

>> "Check whether a required feature has been shadowed by changing
>> `load-path' after it has been loaded and reload that feature from
>> current load-path in this case."
> I don't understand why we need this.

For the case where Org is installed via package.el rather than by
manually downloading and following some installation instructions.

> For package manager, a slightly more general version of the above (not
> conditionalized on the feature starting with "org" or "ob") could be
> used around the package compilation.  I haven't yet tried this, though.

Maybe the better way to do it is for package.el to compare the set of
files of the new package, with the set of currently loaded `features'
and unload the intersection.


        Stefan

bug#10125: RFE: require and load-path-shadowing

[Bastien]

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

> It's just an experiment (AFAIK).

Okay.

>>> "Check whether a required feature has been shadowed by changing
>>> `load-path' after it has been loaded and reload that feature from
>>> current load-path in this case."
>> I don't understand why we need this.
>
> For the case where Org is installed via package.el rather than by
> manually downloading and following some installation instructions.

In that case, isn't it enough to call (package-initialize) before
any Org configuration?

-- 
 Bastien

bug#10125: RFE: require and load-path-shadowing

[Stefan Monnier]

> In that case, isn't it enough to call (package-initialize) before
> any Org configuration?

Here's the scenario:

start Emacs
use Org
use package.el to install a newer version of Org


        Stefan

bug#10125: RFE: require and load-path-shadowing

[Bastien]

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

>> In that case, isn't it enough to call (package-initialize) before
>> any Org configuration?
>
> Here's the scenario:
>
> start Emacs
> use Org
> use package.el to install a newer version of Org

I see, thanks.

-- 
 Bastien

bug#10125: RFE: require and load-path-shadowing

[Achim Gratz]

Eli Zaretskii writes:
>> Could be: depends on the precise semantics of spawn, which I don't know.
>
> I'm barely following this thread, so please tell what semantics do you
> need, and I will then try figuring out whether spawn does what you
> need.

The idea IIUC is to fork/spawn Emacs after it has initialized to be able
to use a "pristine" instance to spawn off again for byte compilation.

emacs-pristine +--- emacs-user
               |
               +--- emacs-bytecompile-1
               |
               +--- emacs-bytecompile-2
               |
               +--- emacs-bytecompile-3

Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

bug#10125: RFE: require and load-path-shadowing

[Eli Zaretskii]

> From: Achim Gratz <Stromeko@nexgo.de>
> Date: Sat, 12 Jan 2013 11:20:58 +0100
> 
> Eli Zaretskii writes:
> >> Could be: depends on the precise semantics of spawn, which I don't know.
> >
> > I'm barely following this thread, so please tell what semantics do you
> > need, and I will then try figuring out whether spawn does what you
> > need.
> 
> The idea IIUC is to fork/spawn Emacs after it has initialized to be able
> to use a "pristine" instance to spawn off again for byte compilation.
> 
> emacs-pristine +--- emacs-user
>                |
>                +--- emacs-bytecompile-1
>                |
>                +--- emacs-bytecompile-2
>                |
>                +--- emacs-bytecompile-3

How is this different from invoking emacs-bytecompile-N instances in
parallel to invoking emacs-pristine?  IOW, do you expect the
byte-compile instances to be different in any way from a fresh Emacs
session invoked from the shell as "emacs -Q"?

bug#10125: RFE: require and load-path-shadowing

[Stefan Monnier]

> IOW, do you expect the byte-compile instances to be different in any
> way from a fresh Emacs session invoked from the shell as "emacs -Q"?

Yes, because the current Emacs may be a different executable than the
one the shell would run in response to "emacs -Q".


        Stefan

bug#10125: RFE: require and load-path-shadowing

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Achim Gratz <Stromeko@nexgo.de>,  10125@debbugs.gnu.org
> Date: Sat, 12 Jan 2013 08:28:29 -0500
> 
> > IOW, do you expect the byte-compile instances to be different in any
> > way from a fresh Emacs session invoked from the shell as "emacs -Q"?
> 
> Yes, because the current Emacs may be a different executable than the
> one the shell would run in response to "emacs -Q".

And if we make sure the same executable is run (easy on Windows)?  Are
there any other differences?

bug#10125: RFE: require and load-path-shadowing

[Eli Zaretskii]

> From: Achim Gratz <Stromeko@nexgo.de>
> Date: Sat, 12 Jan 2013 18:01:36 +0100
> 
> Eli Zaretskii writes:
> >> > IOW, do you expect the byte-compile instances to be different in any
> >> > way from a fresh Emacs session invoked from the shell as "emacs -Q"?
> >> 
> >> Yes, because the current Emacs may be a different executable than the
> >> one the shell would run in response to "emacs -Q".
> >
> > And if we make sure the same executable is run (easy on Windows)?  Are
> > there any other differences?
> 
> There may have been changes in the way Emacs starts up in the meantime,
> since we're considering the case where new packages get installed.
> Starting Emacs with "-Q" shouldn't be affected by this, but there's no
> guarantee (yes, this is a fringe case).

Sorry, I don't follow: new packages can only affect "emacs -Q" if you
re-dump Emacs in between.  Am I missing something?

bug#10125: RFE: require and load-path-shadowing

[Glenn Morris]

Stefan Monnier wrote:

>> IOW, do you expect the byte-compile instances to be different in any
>> way from a fresh Emacs session invoked from the shell as "emacs -Q"?
>
> Yes, because the current Emacs may be a different executable than the
> one the shell would run in response to "emacs -Q".

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10125#50

   Turns out I was looking for invocation-directory and invocation-name.

If you want to worry about wacky things like a new Emacs having been
installed on top of the old in the meantime, more power to you.

bug#10125: RFE: require and load-path-shadowing

[Kevin Rodgers]

On 1/15/13 12:34 PM, Achim Gratz wrote:
> Achim Gratz writes:
>> +               (concat invocation-directory invocation-name)
>
> Better make that
> +               (expand-file-name invocation-name invocation-directory)

FWIW here's what I use:

(defun run-emacs (command)
   "Run the Emacs COMMAND in the background via `shell-command'."
   (interactive
    (let ((program (expand-file-name invocation-name invocation-directory)))
      (list (read-string "Emacs command: "
			(cons (concat program
				      (if (cdr command-line-args)
					  (mapconcat 'identity
						     (cdr command-line-args)
						     " ")
					" -Q")
				      " &")
			      (1+ (length program)))))))
   (shell-command command))

-- 
Kevin Rodgers
Denver, Colorado, USA

bug#10125: RFE: require and load-path-shadowing

[Andreas Schwab]

Kevin Rodgers <kevin.d.rodgers@gmail.com> writes:

> (defun run-emacs (command)
>   "Run the Emacs COMMAND in the background via `shell-command'."
>   (interactive
>    (let ((program (expand-file-name invocation-name invocation-directory)))
>      (list (read-string "Emacs command: "
> 			(cons (concat program
> 				      (if (cdr command-line-args)
> 					  (mapconcat 'identity
> 						     (cdr command-line-args)

You need to use shell-quote-argument to properly shell-quote each
argument.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#10125: RFE: require and load-path-shadowing

[Kevin Rodgers]

On 1/16/13 3:06 AM, Andreas Schwab wrote:
> Kevin Rodgers<kevin.d.rodgers@gmail.com>  writes:
>
>> (defun run-emacs (command)
>>    "Run the Emacs COMMAND in the background via `shell-command'."
>>    (interactive
>>     (let ((program (expand-file-name invocation-name invocation-directory)))
>>       (list (read-string "Emacs command: "
>> 			(cons (concat program
>> 				      (if (cdr command-line-args)
>> 					  (mapconcat 'identity
>> 						     (cdr command-line-args)
>
> You need to use shell-quote-argument to properly shell-quote each
> argument.

Thanks, Andreas!

-- 
Kevin Rodgers
Denver, Colorado, USA

bug#10125: RFE: require and load-path-shadowing

[Achim Gratz]

Stefan Monnier writes:
>> In that case, isn't it enough to call (package-initialize) before
>> any Org configuration?
>
> Here's the scenario:
>
> start Emacs
> use Org
> use package.el to install a newer version of Org

Exactly.  Thanks for putting it so succinctly.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf Blofeld V1.15B11:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

bug#10125: RFE: require and load-path-shadowing

[Achim Gratz]

Eli Zaretskii writes:
>> > IOW, do you expect the byte-compile instances to be different in any
>> > way from a fresh Emacs session invoked from the shell as "emacs -Q"?
>> 
>> Yes, because the current Emacs may be a different executable than the
>> one the shell would run in response to "emacs -Q".
>
> And if we make sure the same executable is run (easy on Windows)?  Are
> there any other differences?

There may have been changes in the way Emacs starts up in the meantime,
since we're considering the case where new packages get installed.
Starting Emacs with "-Q" shouldn't be affected by this, but there's no
guarantee (yes, this is a fringe case).


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada

bug#10125: RFE: require and load-path-shadowing

[Achim Gratz]

Glenn Morris writes:
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10125#50
>
>    Turns out I was looking for invocation-directory and invocation-name.

Indeed, thanks.  So using your earlier patch as a template, this should
be good for emacs-24:

--8<---------------cut here---------------start------------->8---
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 6059f03..df194b8 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -632,11 +632,20 @@ untar into a directory named DIR; otherwise, signal an error."
   "Generate autoloads and do byte-compilation for package named NAME.
 PKG-DIR is the name of the package directory."
   (package-generate-autoloads name pkg-dir)
-  (let ((load-path (cons pkg-dir load-path)))
-    ;; We must load the autoloads file before byte compiling, in
-    ;; case there are magic cookies to set up non-trivial paths.
-    (load (expand-file-name (concat name "-autoloads") pkg-dir) nil t)
-    (byte-recompile-directory pkg-dir 0 t)))
+  (with-current-buffer (get-buffer-create "*package-compile*")
+    (goto-char (point-max))
+    (pop-to-buffer (current-buffer))
+    (or (zerop (call-process
+               (concat invocation-directory invocation-name)
+               nil t t "--batch" "-Q" "--eval"
+               (format
+                "(let ((pkg-dir \"%s\")(name \"%s\"))
+                    (progn (setq load-path (cons pkg-dir load-path))
+                    (load (expand-file-name (concat name \"-autoloads\") pkg-dir) nil t)
+                    (batch-byte-recompile-directory 0)))"
+                pkg-dir name)
+               pkg-dir))
+       (error "Compiling the package gave an error"))))
 
 (defun package--write-file-no-coding (file-name)
   (let ((buffer-file-coding-system 'no-conversion))
--8<---------------cut here---------------end--------------->8---

I've confirmed that Emacs 24.2 fails to install current Org from ELPA
without that patch when trying to install after having opened an Org
file.  It installs Org correctly in that same situation with the patch
that does the package compilation in an external Emacs process.


Regards
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf rackAttack:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

bug#10125: RFE: require and load-path-shadowing

[Achim Gratz]

Eli Zaretskii writes:
> Sorry, I don't follow: new packages can only affect "emacs -Q" if you
> re-dump Emacs in between.  Am I missing something?

No, for package manager "emacs -Q" should work well enough.  The
fork/spawn thing might still be useful to make this more efficient or
(if you allow to create such fork points programmatically) to enable
Emacs to easily backtrack to some earlier state.  But that should be
discussed in emacs-devel under a different headline.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Terratec KOMPLEXER:
http://Synth.Stromeko.net/Downloads.html#KomplexerWaves

bug#10125: RFE: require and load-path-shadowing

[Achim Gratz]

Achim Gratz writes:
> +               (concat invocation-directory invocation-name)

Better make that 
+               (expand-file-name invocation-name invocation-directory)

Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Terratec KOMPLEXER:
http://Synth.Stromeko.net/Downloads.html#KomplexerWaves

bug#10125: RFE: require and load-path-shadowing

[Jambunathan K]

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

> - outdated .elc file taking precedence over the new .el file can do
>   the same.

I find this warning quite useful.

Consider this, all happening because of what is in .emacs.

1. Something in .emacs causes org-x to be loaded from Vanilla Emacs.

2. Load path changed to point to a git checkout.

3. User does something which loads org-y.  org-y comes from (2).  The
   chain of events ends up with requiring an org-x.  org-x WASN'T
   RE-LOADED because the symbol is provided.  Ofcourse, it is provided
   from the wrong place.

In case of (3), a warning that says "I refuse to have mutiple
personality disorder".   A warning during Emacs initing could be
missed.  So if there is a command line option to trigger an abrupt-end,
we could advise the user to enable it and say "You have got a problem in
your init file.  Not the Org-mode that is distributed."

This will serve to remove endless amount of confusion.

What I am saying, even if we couldn't solve the problem cleanly, a
simple warning that says "Something fishy" will greatly reduce the user
complaints in the Org list.

I am not sure how much of what I typed makes sense.  But I type
nevertheless, people read it or ignore it.
-- 

bug#10125: RFE: require and load-path-shadowing

[Stefan Monnier]

>> There is currently a problem with package manager when a package is
>> installed from a package archive,that package is already installed
>> either in Emacs core or site-lisp, and when (parts of) said package have
>> already been loaded when ELPA tries to install: the byte-compiler will
>> use the already loaded definitions rather than the new ones from the
>> package to install.
> I guess this would be http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10125

Indeed, and as I mentioned back then I think it's a general enough
problem that we should try and think up a good solution.

Currently, we have two proposals:

1- run a separate Emacs instance: this gives you a clean slate, and lets
   you compile in parallel, but runs into the difficulty of figuring out
   exactly which clean slate to use.
   I guess we could fork Emacs early on and keep this second process
   around as a "process from which to generate new clean slates".

2- improve bytecomp.el to try and better isolate the compiled file from
   the previously loaded packages.

I don't see a clear winner, but since I'm biased in favor of the second
(not sure why, to tell you the truth), I'll add a few points related to
it.  The current behavior of bytecomp.el leads to various related
problems:
- outdated but already loaded packages can lead to mis-compilation.
  for the bootstrap we try to workaround this with
  byte-compile-refresh-preloaded, tho it only takes care of some
  particular cases.
- outdated .elc file taking precedence over the new .el file can do
  the same.
- bytecompiling a file affects the running session by side-effects such
  as requiring packages.
- if a package calls `byte-compile' during its own compilation, this
  sub-compilation will tend to complain about undeclared variables
  because it doesn't know about the vars that have been defvar'd in the
  outer compilation.  That's one of the main reasons for cc-bytecomp's
  hideous gymnastics.

Maybe we should (similarly to the fork idea above) keep a "clean
obarray", and run byte-compilations in a fresh copy of this
clean obarray.

I suggested a quick&dirty solution:
> > E.g. we could add to bytecomp.el the ability to force `require' to
> > reload a package if it's not already loaded from the file that
> > locate-library returns.

I still think it's not a bad option.

Of course, we'd still get trouble when the loading is not performed via
`require' but via autoload (maybe we could try and attack this problem
by allowing `autoload' to override an already existing definition, but
that could be delicate).

> That will probably work fine most of the time, but what if a package is
> restructed so that the feature names are different? Or a feature is
> removed?

I don't see why that would introduce a difficulty.


        Stefan