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

[Satyaki Das]

On 1/13/06, Bill Wohler <address@hidden> wrote:
> Drew Adams <address@hidden> wrote:
>
> >     > 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.

>                                                                         If I 
> understand
> correctly, Kanichi's patch addresses your concern (about the return
> value of require changing). Here's the ChangeLog for that change:

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

>
>     2004-08-23  Satyaki Das  <address@hidden>
>
>         * 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