mh-e/mh-acros.el advices `require' incorrectly

[Kenichi Handa]

This is in the latest CVS.

The origianl `require' returns FEATURE even if it is already
provided, but the adviced `require' returns nil in such a
case. So, once mh-acros.el is loaded, a code depending on
the return value of `require' stops working.

Ex:
(require 'mh-e)
(require 'enriched) => enriched
(require 'enriched) => nil

How about this change?

*** mh-acros.el 13 Jan 2006 10:27:43 +0900 1.13
--- mh-acros.el 13 Jan 2006 16:45:04 +0900
***************
*** 152,161 ****

(defadvice require (around mh-prefer-el activate)
"Modify `require' to load uncompiled MH-E files."
! (or (featurep (ad-get-arg 0))
! (and (string-match "^mh-" (symbol-name (ad-get-arg 0)))
! (load (format "%s.el" (ad-get-arg 0)) t t))
! ad-do-it))

(defmacro mh-assoc-ignore-case (key alist)
"Check if KEY is present in ALIST while ignoring case to do the comparison.
--- 152,162 ----

(defadvice require (around mh-prefer-el activate)
"Modify `require' to load uncompiled MH-E files."
! (if (or (featurep (ad-get-arg 0))
! (and (string-match "^mh-" (symbol-name (ad-get-arg 0)))
! (load (format "%s.el" (ad-get-arg 0)) t t))
! ad-do-it)
! (setq ad-return-value (ad-get-arg 0))))

(defmacro mh-assoc-ignore-case (key alist)
"Check if KEY is present in ALIST while ignoring case to do the comparison.

---
Kenichi Handa
***@m17n.org

[Bill Wohler]

Post by Kenichi Handa
This is in the latest CVS.
The origianl `require' returns FEATURE even if it is already
provided, but the adviced `require' returns nil in such a
case. So, once mh-acros.el is loaded, a code depending on
the return value of `require' stops working.
(require 'mh-e)
(require 'enriched) => enriched
(require 'enriched) => nil
How about this change?
*** mh-acros.el 13 Jan 2006 10:27:43 +0900 1.13
--- mh-acros.el 13 Jan 2006 16:45:04 +0900
***************
*** 152,161 ****
(defadvice require (around mh-prefer-el activate)
"Modify `require' to load uncompiled MH-E files."
! (or (featurep (ad-get-arg 0))
! (and (string-match "^mh-" (symbol-name (ad-get-arg 0)))
! (load (format "%s.el" (ad-get-arg 0)) t t))
! ad-do-it))
(defmacro mh-assoc-ignore-case (key alist)
"Check if KEY is present in ALIST while ignoring case to do the comparison.
--- 152,162 ----
(defadvice require (around mh-prefer-el activate)
"Modify `require' to load uncompiled MH-E files."
! (if (or (featurep (ad-get-arg 0))
! (and (string-match "^mh-" (symbol-name (ad-get-arg 0)))
! (load (format "%s.el" (ad-get-arg 0)) t t))
! ad-do-it)
! (setq ad-return-value (ad-get-arg 0))))
(defmacro mh-assoc-ignore-case (key alist)
"Check if KEY is present in ALIST while ignoring case to do the comparison.

Satyaki,

What do you (and others) think about this?

[Drew Adams]

Post by Kenichi Handa
The origianl `require' returns FEATURE even if it is already
provided, but the adviced `require' returns nil in such a
case. So, once mh-acros.el is loaded, a code depending on
the return value of `require' stops working.
How about this change?

What do you (and others) think about this?

Besides the fact that we shouldn't use defadvice in vanilla Emacs code, I
don't think we should change the return value of `require' at all. It has
acted this way for a very long time, and there is undoubtedly code that
depends on this behavior (at least some external libraries). Besides, we
certainly should not make such a change just before a release.

[Bill Wohler]

Post by Bill Wohler

Post by Kenichi Handa
The origianl `require' returns FEATURE even if it is already
provided, but the adviced `require' returns nil in such a
case. So, once mh-acros.el is loaded, a code depending on
the return value of `require' stops working.
How about this change?

What do you (and others) think about this?
Besides the fact that we shouldn't use defadvice in vanilla Emacs code, I
don't think we should change the return value of `require' at all. It has
acted this way for a very long time, and there is undoubtedly code that
depends on this behavior (at least some external libraries). Besides, we
certainly should not make such a change just before a release.

Unless you load MH-E, require does not change. If I understand
correctly, Kanichi's patch addresses your concern (about the return
value of require changing). Here's the ChangeLog for that change:

2004-08-23 Satyaki Das <***@theforce.stanford.edu>

* mh-acros.el (mh-require-cl): Remove unneeded autoloads.
(require): Add an advice to the function so that at compile time
the uncompiled file is loaded. This avoids compilation problems
when built in the Emacs tree.

Satyaki, just out of curiosity, do you remember what the problems were?
Would we be able to remove the require defadvice if we removed the
circular dependencies?

[Satyaki Das]

Post by Bill Wohler

Post by Bill Wohler

Post by Kenichi Handa
The origianl `require' returns FEATURE even if it is already
provided, but the adviced `require' returns nil in such a
case. So, once mh-acros.el is loaded, a code depending on
the return value of `require' stops working.
How about this change?

What do you (and others) think about this?
Besides the fact that we shouldn't use defadvice in vanilla Emacs code, I
don't think we should change the return value of `require' at all. It has
acted this way for a very long time, and there is undoubtedly code that
depends on this behavior (at least some external libraries). Besides, we
certainly should not make such a change just before a release.

Unless you load MH-E, require does not change.

Even if you load MH-E, require isn't adviced since mh-acros.el is
never loaded at run time unless you load it yourself.

Post by Bill Wohler
If I understand
correctly, Kanichi's patch addresses your concern (about the return

Kenichi's patch won't cause any problems for MH-E.

Post by Bill Wohler
* mh-acros.el (mh-require-cl): Remove unneeded autoloads.
(require): Add an advice to the function so that at compile time
the uncompiled file is loaded. This avoids compilation problems
when built in the Emacs tree.
Satyaki, just out of curiosity, do you remember what the problems were?
Would we be able to remove the require defadvice if we removed the
circular dependencies?

That advice forces the load of src .el file. Suppose you have three
files a.el, b.el and c.el and the compiled files a.elc, b.elc and
c.elc. Suppose that there is a macro foo in c.el that is used in
a.el. Now let us make a change to macro foo. Then if we recompile
all the files in the directory then they are compiled in the lexical
order. So when a.el is getting compiled, require will load the c.elc
and get the old definition of the macro. The advice above avoids
this.

A better solution would be to use Stefan's change in bytecomp.el. If
it were checked in, we can drop the advice to require and it will make
things nicer elsewhere too.

Satyaki

[Bill Wohler]

Post by Satyaki Das

Post by Bill Wohler

Post by Bill Wohler

Post by Kenichi Handa
The origianl `require' returns FEATURE even if it is already
provided, but the adviced `require' returns nil in such a
case. So, once mh-acros.el is loaded, a code depending on
the return value of `require' stops working.
How about this change?

What do you (and others) think about this?
Besides the fact that we shouldn't use defadvice in vanilla Emacs code, I
don't think we should change the return value of `require' at all. It has
acted this way for a very long time, and there is undoubtedly code that
depends on this behavior (at least some external libraries). Besides, we
certainly should not make such a change just before a release.

Unless you load MH-E, require does not change.

Even if you load MH-E, require isn't adviced since mh-acros.el is
never loaded at run time unless you load it yourself.

Post by Bill Wohler
If I understand
correctly, Kanichi's patch addresses your concern (about the return

Kenichi's patch won't cause any problems for MH-E.

This means you can check in your patch, Kenichi ;-).

Thanks very much for the explanation, Satyaki.

[Kenichi Handa]

Post by Bill Wohler

Post by Drew Adams
Besides the fact that we shouldn't use defadvice in vanilla Emacs code, I
don't think we should change the return value of `require' at all. It has
acted this way for a very long time, and there is undoubtedly code that
depends on this behavior (at least some external libraries). Besides, we
certainly should not make such a change just before a release.

Unless you load MH-E, require does not change.

Unfortunately, even if we don't load MH-E explicitly, now,
just compiling some file in emacs/lisp/mh-e/*.el change the
behaviour of `require'.

Post by Bill Wohler
If I understand correctly, Kanichi's patch addresses your
concern (about the return value of require changing).

Yes.

---
Kenichi Handa
***@m17n.org

[Richard M. Stallman]

Unless you load MH-E, require does not change. If I understand
correctly, Kanichi's patch addresses your concern (about the return
value of require changing).

MH-E should not advise `require'; nothing should.
Another solution is necessary.

Which files are trying to force loading in source form?

[Bill Wohler]

Post by Richard M. Stallman
MH-E should not advise `require'; nothing should.
Another solution is necessary.

We only had the problem our workaround addresses when compiling in the
Emacs tree. We do not have any problems when compiling in our own
environment because we remove all MH-E .elc files before compiling.

What do people think about my updating lisp/Makefile.in to remove
lisp/mh-e/*.elc if and only if lisp/mh-e/*.el changes?

[Andreas Schwab]

Post by Bill Wohler

Post by Richard M. Stallman
MH-E should not advise `require'; nothing should.
Another solution is necessary.

We only had the problem our workaround addresses when compiling in the
Emacs tree. We do not have any problems when compiling in our own
environment because we remove all MH-E .elc files before compiling.

That's what `make bootstrap' does.

Andreas.

[Bill Wohler]

Post by Andreas Schwab

Post by Bill Wohler

Post by Richard M. Stallman
MH-E should not advise `require'; nothing should.
Another solution is necessary.

We only had the problem our workaround addresses when compiling in the
Emacs tree. We do not have any problems when compiling in our own
environment because we remove all MH-E .elc files before compiling.

That's what `make bootstrap' does.

You can't expect everyone to run make bootstrap when they change one
lisp file.

[Stefan Monnier]

Post by Bill Wohler
You can't expect everyone to run make bootstrap when they change one
lisp file.

Yet, that's how we've worked for many years now.
In practice the problems you describe are infrequent and managed by hand or
by `make bootstrap'.


Stefan

[Richard M. Stallman]

What do people think about my updating lisp/Makefile.in to remove
lisp/mh-e/*.elc if and only if lisp/mh-e/*.el changes?

That won't help with M-x byte-recompile. The right solution is to
make the graph of requires acyclic, and put the macro file at the
start of the alphabet. Given the way the software in fact works,
that should be totally reliable.

[Bill Wohler]

Post by Bill Wohler
What do people think about my updating lisp/Makefile.in to remove
lisp/mh-e/*.elc if and only if lisp/mh-e/*.el changes?
That won't help with M-x byte-recompile. The right solution is to
make the graph of requires acyclic, and put the macro file at the
start of the alphabet. Given the way the software in fact works,
that should be totally reliable.

Thank you. I think I'm going to go ahead and bite the bullet and do just
that. I've appended a proposal I sent to the MH-E developers today.
While I don't expect you to understand MH-E, perhaps you can let me know
whether your gut says "good idea" or "bad idea" and why.

(mh-customize.el holds all of our defcustom and defface calls.
mh-loaddefs.el is the internal MH-E analog of loaddefs.el.)

1. Remove all (require 'mh-*) lines, move the provides in
mh-customize.el and mh-e.el to the end and start from scratch ;-).

2. Make mh-utils.el a pure "utility" package. I'm thinking chunks could
go into mh-xface.el and mh-scan.el (taking stuff from mh-e.el too).
We would then create files for each of the modes. We now already have
mh-search.el, so I'd propose mh-show.el and mh-folder.el, maybe
renaming mh-comp.el to mh-letter.el for consistency. Move
mh-modify to mh-funcs.el where it belongs.

3. Make mh-e.el a lean, mean entry point. Make it require mh-acros,
mh-customize, *and nothing else* within MH-E. Have it define pretty
much only mh-rmail, mh-smail, and mh-version (functions with the
;;;###autoload cookie) as well as a few other generic globals such as
mh-xemacs-flag. The frequently-used commands would go into
mh-folder.el and the rest would go into mh-funcs.el.

4. Make mh-customize require mh-loaddefs to get the function definitions
it needs *and nothing else* within MH-E.

5. Everything else then requires mh-e.el (implicitly getting
mh-customize and mh-loaddefs) *and nothing else* within MH-E (except
for perhaps variable-only files like mh-buffers.el and mh-scan.el).

6. Limit scope of variables to just the file in which it is defined.
If necessary, provide access to variables via functions that are
found in mh-loaddefs.el.

7. Move all macros into mh-acros.el so that we can remove the defadvice
of require in mh-acros.el.

[Richard M. Stallman]

I don't understand 100% of the plan, but it looks like a good plan
overall.

2. Make mh-utils.el a pure "utility" package. I'm thinking chunks could
go into mh-xface.el and mh-scan.el (taking stuff from mh-e.el too).
We would then create files for each of the modes. We now already have
mh-search.el, so I'd propose mh-show.el and mh-folder.el, maybe
renaming mh-comp.el to mh-letter.el for consistency. Move
mh-modify to mh-funcs.el where it belongs.

6. Limit scope of variables to just the file in which it is defined.
If necessary, provide access to variables via functions that are
found in mh-loaddefs.el.

I think these two are be necessary to solve the present problem, and
could substantially increase the amount of change. It might be better
not to do these changes now.

[Bill Wohler]

Post by Richard M. Stallman
I don't understand 100% of the plan, but it looks like a good plan
overall.

Thanks for the feedback.

Post by Richard M. Stallman
2. Make mh-utils.el a pure "utility" package. I'm thinking chunks could
go into mh-xface.el and mh-scan.el (taking stuff from mh-e.el too).
We would then create files for each of the modes. We now already have
mh-search.el, so I'd propose mh-show.el and mh-folder.el, maybe
renaming mh-comp.el to mh-letter.el for consistency. Move
mh-modify to mh-funcs.el where it belongs.
6. Limit scope of variables to just the file in which it is defined.
If necessary, provide access to variables via functions that are
found in mh-loaddefs.el.
I think these two are be necessary to solve the present problem, and
could substantially increase the amount of change. It might be better
not to do these changes now.

Maybe. I'm investigating it right now since it seems that every day we
uncover a new problem associated with the current mess. It's extremely
brittle. The changes will either work or they won't. If they do, I think
it would actually add to the stability of the release to check in the
changes. I'll let you know how it goes.

[David Kastrup]

Post by Bill Wohler
Maybe. I'm investigating it right now since it seems that every day
we uncover a new problem associated with the current mess. It's
extremely brittle. The changes will either work or they won't. If
they do, I think it would actually add to the stability of the
release to check in the changes. I'll let you know how it goes.

You are aware that this sounds like smoldering duct tape all over?
I am not at all acquainted with the involved code, but if you feel it
is best described with those words...

[Bill Wohler]

Post by David Kastrup

Post by Bill Wohler
Maybe. I'm investigating it right now since it seems that every day
we uncover a new problem associated with the current mess. It's
extremely brittle. The changes will either work or they won't. If
they do, I think it would actually add to the stability of the
release to check in the changes. I'll let you know how it goes.

You are aware that this sounds like smoldering duct tape all over?
I am not at all acquainted with the involved code, but if you feel it
is best described with those words...

The code has roots from the early 80's so it's long past the smoldering
point ;-). But the bones are strong; it just needs a renovation ;-).

I usually describe it in these words: http://mh-e.sourceforge.net/

[Bill Wohler]

Post by Bill Wohler

Post by Richard M. Stallman
I don't understand 100% of the plan, but it looks like a good plan
overall.

Thanks for the feedback.

Post by Richard M. Stallman
2. Make mh-utils.el a pure "utility" package. I'm thinking chunks could
go into mh-xface.el and mh-scan.el (taking stuff from mh-e.el too).
We would then create files for each of the modes. We now already have
mh-search.el, so I'd propose mh-show.el and mh-folder.el, maybe
renaming mh-comp.el to mh-letter.el for consistency. Move
mh-modify to mh-funcs.el where it belongs.
6. Limit scope of variables to just the file in which it is defined.
If necessary, provide access to variables via functions that are
found in mh-loaddefs.el.
I think these two are be necessary to solve the present problem, and
could substantially increase the amount of change. It might be better
not to do these changes now.

Maybe. I'm investigating it right now since it seems that every day we
uncover a new problem associated with the current mess. It's extremely
brittle. The changes will either work or they won't. If they do, I think
it would actually add to the stability of the release to check in the
changes. I'll let you know how it goes.

Hi Richard,

I finished the reorganization of MH-E over a week ago, and I'm extremely
pleased with the results. The developers and a few users have been smoke
testing a tarball of my work directory and there are no known issues
(the only issue revealed by the smoke test was a small Emacs 21 issue
which was quickly fixed).

I'm sorry this didn't come up sooner in the release cycle, but I'll go
ahead and check in the changes since I'm definitely going to use them
for the MH-E 8.0 release, the MH-E package isn't a critical part of
Emacs, and I think the Emacs 22 release will be more stable with few, if
any, risks.

Highlights:

1. Circular dependencies are gone.

2. defadvice of require is gone; shared macros have been moved to
mh-acros.el. This means that you'll never get stale macro definitions
making the defadvice unnecessary.

3. I was able to delete this code *within*
mh-gnus-article-highlight-citation:

;; Requiring gnus-cite should have been sufficient. However for Emacs21.1,
;; recursive-load-depth-limit is only 10, so an error occurs. Also it may be
;; better to have an autoload at top-level (though that won't work because
;; of recursive-load-depth-limit). That gets rid of a compiler warning as
;; well.
(unless mh-xemacs-flag
(require 'gnus-art)
(require 'gnus-cite))

4. I was able to delete this code in mh-utils.el:

(eval-and-compile
(defvar recursive-load-depth-limit)
(if (and (boundp 'recursive-load-depth-limit)
(integerp recursive-load-depth-limit)
(< recursive-load-depth-limit 50))
(setq recursive-load-depth-limit 50)))

5. I was able to delete eval-and-compile tricks as in the following
around mh-show-font-lock-keywords:

(eval-and-compile
;; Otherwise byte-compilation fails on
;; `mh-show-font-lock-keywords-with-cite'

6. XEmacs now compiles with just three warnings (from XEmacs macros
which I can't do anything about). It used to compile with *hundreds*
of warnings and a handful of errors too. (Note that MH-E has always
compiled clean in GNU Emacs--it is a developer requirement.)

While the diffs will be large, they mostly represent functions moving
from one file to another. The functional groupings are now much more
cohesive and consistent and will be easier to maintain.

Developers should be aware that three files will be deleted so it might
be a good idea to remove lisp/mh-e/*.elc. The entry points are in
different files too so users of CVS MH-E will have to run "cd lisp; make
autoloads". You can accomplish both, of course, with "make bootstrap".
Nine new files were added.

Post by Bill Wohler
1. Remove all (require 'mh-*) lines, move the provides in
mh-customize.el and mh-e.el to the end and start from scratch ;-).

Done.

Post by Bill Wohler
2. Make mh-utils.el a pure "utility" package. I'm thinking chunks could
go into mh-xface.el and mh-scan.el (taking stuff from mh-e.el too).
We would then create files for each of the modes. We now already have
mh-search.el, so I'd propose mh-show.el and mh-folder.el, maybe
renaming mh-comp.el to mh-letter.el for consistency. Move
mh-modify to mh-funcs.el where it belongs.

Done. While I moved mh-letter-mode into mh-letter.el, It still made
sense to keep mh-comp.el. I also pulled the limiting code out of
mh-seq.el into mh-limit.el and the threading code out of mh-seq.el into
mh-thread.el. In addition, I created mh-compat.el (for compatibility
defsubsts) and mh-tool-bar.el for the tool bar creation code.

Post by Bill Wohler
3. Make mh-e.el a lean, mean entry point. Make it require mh-acros,
mh-customize, *and nothing else* within MH-E. Have it define pretty
much only mh-rmail, mh-smail, and mh-version (functions with the
;;;###autoload cookie) as well as a few other generic globals such as
mh-xemacs-flag. The frequently-used commands would go into
mh-folder.el and the rest would go into mh-funcs.el.

I made mh-e.el lean and mean, but not exactly as I described. I moved
nearly all of the code into other more logical files, pulled in the
defcustoms from mh-customize.el, and then just enough code to get it to
compile, including mh-init.el and mh-exec.el (removing those three files
as a result). The only entry point left in mh-e.el is mh-version. I
moved the other entry points into their corresponding files (for
example, mh-smail into mh-comp.el and mh-rmail into mh-folder.el). The
result eliminates dependencies in mh-e.el, and reduces the amount of
code that needs to be loaded when running, say, just mh-smail.

Post by Bill Wohler
4. Make mh-customize require mh-loaddefs to get the function definitions
it needs *and nothing else* within MH-E.

The content of mh-customize.el was incorporated into mh-e.el and
the file was removed.

Post by Bill Wohler
5. Everything else then requires mh-e.el (implicitly getting
mh-customize and mh-loaddefs) *and nothing else* within MH-E (except
for perhaps variable-only files like mh-buffers.el and mh-scan.el).

Done.

Post by Bill Wohler
6. Limit scope of variables to just the file in which it is defined.
If necessary, provide access to variables via functions that are
found in mh-loaddefs.el.

I did this in a couple of places where it was very clearly the right
thing to do, but left the vast majority of globals there were previously
spread across mh-e.el and mh-utils.el in mh-e.el.

Post by Bill Wohler
7. Move all macros into mh-acros.el so that we can remove the defadvice
of require in mh-acros.el.

Done, except that I didn't move all macros, just those that were used
in more than one file.

[Richard M. Stallman]

Thanks for making this clean. Certainly please install it.

[Bill Wohler]

Post by Richard M. Stallman
Thanks for making this clean. Certainly please install it.

Thank you. Done.

[Stefan Monnier]

Post by Kenichi Handa
*** mh-acros.el 13 Jan 2006 10:27:43 +0900 1.13
--- mh-acros.el 13 Jan 2006 16:45:04 +0900
***************
*** 152,161 ****
(defadvice require (around mh-prefer-el activate)
"Modify `require' to load uncompiled MH-E files."
! (or (featurep (ad-get-arg 0))
! (and (string-match "^mh-" (symbol-name (ad-get-arg 0)))
! (load (format "%s.el" (ad-get-arg 0)) t t))
! ad-do-it))

For what it's worth:
We usually try to avoid defadvice within Emacs and I think for
good reasons. Now, some elisp packages do you use defadvice, but they only
only do it when it's absolutely necessary. The above advice seem to be just
a minor convenience hack to making recompiling occasionally easier.

I'd rather try and come up with a generic fix for the problem.
I personally use the patch below for now.


Stefan


--- orig/lisp/emacs-lisp/bytecomp.el
+++ mod/lisp/emacs-lisp/bytecomp.el
@@ -1640,6 +1640,12 @@
;; Force logging of the file name for each file compiled.
(setq byte-compile-last-logged-file nil)
(let ((byte-compile-current-file filename)
+ ;; Prefer source files over compiled files. This is so that when
+ ;; several files are changed and recompiled, each new file is
+ ;; properly recompiled with the new macros in the other new files.
+ (load-suffixes (sort (copy-sequence load-suffixes)
+ (lambda (s1 s2) (and (string-match "\\.elc\\b" s2)
+ (string-match "\\.el\\b" s1)))))
(set-auto-coding-for-load t)
target-file input-buffer output-buffer
byte-compile-dest-file)

[Bill Wohler]

Post by Stefan Monnier

Post by Kenichi Handa
*** mh-acros.el 13 Jan 2006 10:27:43 +0900 1.13
--- mh-acros.el 13 Jan 2006 16:45:04 +0900
***************
*** 152,161 ****
(defadvice require (around mh-prefer-el activate)
"Modify `require' to load uncompiled MH-E files."
! (or (featurep (ad-get-arg 0))
! (and (string-match "^mh-" (symbol-name (ad-get-arg 0)))
! (load (format "%s.el" (ad-get-arg 0)) t t))
! ad-do-it))

We usually try to avoid defadvice within Emacs and I think for
good reasons. Now, some elisp packages do you use defadvice, but they only
only do it when it's absolutely necessary. The above advice seem to be just
a minor convenience hack to making recompiling occasionally easier.
I'd rather try and come up with a generic fix for the problem.
I personally use the patch below for now.
Stefan
--- orig/lisp/emacs-lisp/bytecomp.el
+++ mod/lisp/emacs-lisp/bytecomp.el
@@ -1640,6 +1640,12 @@
;; Force logging of the file name for each file compiled.
(setq byte-compile-last-logged-file nil)
(let ((byte-compile-current-file filename)
+ ;; Prefer source files over compiled files. This is so that when
+ ;; several files are changed and recompiled, each new file is
+ ;; properly recompiled with the new macros in the other new files.
+ (load-suffixes (sort (copy-sequence load-suffixes)
+ (lambda (s1 s2) (and (string-match "\\.elc\\b" s2)
+ (string-match "\\.el\\b" s1)))))
(set-auto-coding-for-load t)
target-file input-buffer output-buffer
byte-compile-dest-file)

Is there a problem with your committing this patch? Then we can get
rid of the defadvice and make everyone happy ;-).

[Richard M. Stallman]

--- orig/lisp/emacs-lisp/bytecomp.el
+++ mod/lisp/emacs-lisp/bytecomp.el
@@ -1640,6 +1640,12 @@
;; Force logging of the file name for each file compiled.
(setq byte-compile-last-logged-file nil)
(let ((byte-compile-current-file filename)
+ ;; Prefer source files over compiled files. This is so that when
+ ;; several files are changed and recompiled, each new file is
+ ;; properly recompiled with the new macros in the other new files.
+ (load-suffixes (sort (copy-sequence load-suffixes)
+ (lambda (s1 s2) (and (string-match "\\.elc\\b" s2)
+ (string-match "\\.el\\b" s1)))))
(set-auto-coding-for-load t)
target-file input-buffer output-buffer
byte-compile-dest-file)

That creates an inconsistency. If it were a reliable solution to the
problem, that could justify the inconsistency. But it isn't reliable,
because if the file require'd was already loaded in bytecode outside
the compiler, this won't reload it.

Thus, if we want a reliable solution, it has to be something else.

For the same reason, the advice in mh-e.el is not a good solution
(even disregarding that it is a bad thing for parts of Emacs to
advise other parts).

[Stefan Monnier]

Post by Stefan Monnier
--- orig/lisp/emacs-lisp/bytecomp.el
+++ mod/lisp/emacs-lisp/bytecomp.el
@@ -1640,6 +1640,12 @@
;; Force logging of the file name for each file compiled.
(setq byte-compile-last-logged-file nil)
(let ((byte-compile-current-file filename)
+ ;; Prefer source files over compiled files. This is so that when
+ ;; several files are changed and recompiled, each new file is
+ ;; properly recompiled with the new macros in the other new files.
+ (load-suffixes (sort (copy-sequence load-suffixes)
+ (lambda (s1 s2) (and (string-match "\\.elc\\b" s2)
+ (string-match "\\.el\\b" s1)))))
(set-auto-coding-for-load t)
target-file input-buffer output-buffer
byte-compile-dest-file)
That creates an inconsistency. If it were a reliable solution to the
problem, that could justify the inconsistency. But it isn't reliable,
because if the file require'd was already loaded in bytecode outside
the compiler, this won't reload it.

Indeed, it's just a minor convenience hack.

Post by Stefan Monnier
For the same reason, the advice in mh-e.el is not a good solution
(even disregarding that it is a bad thing for parts of Emacs to
advise other parts).

100% agreement.


Stefan

[Richard M. Stallman]

The origianl `require' returns FEATURE even if it is already
provided, but the adviced `require' returns nil in such a
case.

Packages in Emacs are not supposed to advise other parts of Emacs.
So really the right thing to do is get rid of this advising.
It is not clean for `require' to work differently in a certain
part of Emacs.

Bill, what was the reason for this?

[Bill Wohler]

Post by Kenichi Handa
The origianl `require' returns FEATURE even if it is already
provided, but the adviced `require' returns nil in such a
case.
Packages in Emacs are not supposed to advise other parts of Emacs.
So really the right thing to do is get rid of this advising.
It is not clean for `require' to work differently in a certain
part of Emacs.
Bill, what was the reason for this?

I trust you've seen Satyaki's and Stefan's messages by now. I'm happy to
remove the defadvice as soon as the problem it works around has been
solved. Perhaps we can install Stefan's patch or one like it.

[Bill Wohler]

Post by Bill Wohler
Developers should be aware that three files will be deleted so it might
be a good idea to remove lisp/mh-e/*.elc. The entry points are in
different files too so users of CVS MH-E will have to run "cd lisp; make
autoloads". You can accomplish both, of course, with "make bootstrap".
Nine new files were added.

I forgot to point out that because lisp/Makefile.in changed, you first
have to run ./configure.

Apologies for the inconvenience.