Re: Failing to advice `require'

[Barry Margolin]

In article <80zk5ou2r0.fsf@somewhere.org>,
 "Sebastien Vauban" <wxhgmqzgwmuf@spammotel.com> wrote:

> Hi Barry,
> 
> Thanks for helping me...
> 
> Barry Margolin wrote:
> > In article <80r4r18ttc.fsf@somewhere.org>,
> >  "Sebastien Vauban" <wxhgmqzgwmuf@spammotel.com> wrote:
> >> 
> >> I tried to advice the require function, in order to get:
> >> 
> >> - performance information (load time)
> >> - call graph of `require'
> >> 
> >> with the following code:
> >> 
> >>   (defvar my/require-depth 0)
> >> 
> >>   (defadvice require (around require-around activate)
> >>     "Leave a trace of packages being loaded."
> >>     (let ((feature (ad-get-arg 0))
> >>           (filename (ad-get-arg 1))
> >>           (noerror (ad-get-arg 2))
> >
> > You never use filename or noerror, why do you bother binding them?
> 
> Right.
> 
> >>           (prefix (concat (make-string (* 2 my/require-depth) ? ) "+-> 
> >>           ")))
> >>       (setq my/require-depth (1+ my/require-depth))
> >
> > You should bind my/require-depth in your 'let'. Then you don't need to 
> > restore it at the end, it will happen automatically (including if 
> > there's an error that aborts out).
> 
> Not sure to understand, here, because I still want a global variable holding
> the depth of the require calls. I'm not playing with a local variable, but
> setting back and forth the value of the global var. Do I miss something?

You can use 'let' to temporarily bind a global variable.  This is a 
feature of dynamic scoping.

> 
> >>       (cond ((featurep feature)
> >>              (message "(info) %sRequiring `%s'... already loaded"
> >>                       prefix feature)
> >>              )
> >>             (t
> >>              (let ((my/time-start))
> >>                (message "(info) %sRequiring `%s'..." prefix feature)
> >>                (setq my/time-start (float-time))
> >>                ad-do-it
> >>                (message "(info) %sRequiring `%s'... %s (loaded in %.2f s)"
> >>                         prefix feature
> >>                         (locate-library (symbol-name feature))
> >>                         (- (float-time) my/time-start))
> >>                )))
> >>       (setq my/require-depth (1- my/require-depth))))
> >> 
> >> It works quite well, except for the following case:
> >> 
> >> ;; (info)  +-> Requiring `eieio'... already loaded [2 times]
> >> ;; byte-code: eieio not found in `load-path' or gnus-fallback-lib/ 
> >> directory.
> >> 
> >> Guess what? `eieio' is in my default load path, in my GNU Emacs 24.1.1
> >> (i386-mingw-nt5.1.2600) of 2012-06-02 on MARVIN on Windows XP.
> >> 
> >> Do you have any idea why it's failing?
> >
> > The real require returns the feature name, or nil if the package is not
> > found and noerror is set, yours returns various other things. You need to
> > ensure that you return the correct thing;
> 
> I did not think at that, yes...
> 
> > 'prog1' is useful for this:
> >
> > (prog1 ad-do-it
> >        (message ...))
> 
> Hence my new version of the code:
> 
> --8<---------------cut here---------------start------------->8---
>          (defadvice require (around require-around activate)
>            "Leave a trace of packages being loaded."
>            (let ((feature (ad-get-arg 0))
>                  (prefix (concat (make-string (* 2 my/require-depth) ? ) "+-> 
>                  ")))
>              (setq my/require-depth (1+ my/require-depth))
>              (cond ((featurep feature)
>                     (message "(info) %sRequiring `%s'... already loaded"
>                              prefix feature)
>                     )
>                    (t
>                     (let ((my/time-start))
>                       (message "(info) %sRequiring `%s'..." prefix feature)
>                       (setq my/time-start (float-time))
>                       (prog1
>                           ad-do-it
>                         (message "(info) %sRequiring `%s'... %s (loaded in 
>                         %.2f s)"
>                                  prefix feature
>                                  (locate-library (symbol-name feature))
>                                  (- (float-time) my/time-start)))
>                       )))
>              (setq my/require-depth (1- my/require-depth))))
> --8<---------------cut here---------------end--------------->8---
> 
> Is the above what you suggested me to do?

You're still not returning the feature name in the case where the 
feature is already loaded.

> 
> However, with the above, I still have the same problem (here a little bit 
> more
> trace):

I don't really know if the above issue is related to the error you're 
getting.  Maybe you should enable debugging.

-- 
Barry Margolin, barmar@alum.mit.edu
Arlington, MA
*** PLEASE post questions in newsgroups, not directly to me ***