Reverting *Locate* buffers.

Reverting *Locate* buffers.

[Luc Teirlinck]

There have been in the past several complaints about the fact that
trying to revert a *Locate* buffers first asks whether you want to
update the locate database.

If you answer yes, then in my setup and I would guess in most setups,
you get an error unless you are running as root, because you need to
be root to update the locate database.  As a result, the buffer does
not get reverted.

If you answer no, the *Locate* buffer does not get reverted either,
whereas you might just have wanted to revert to undo manual changes
to the buffer and whereas the locate database might have been updated
by cron, or by the user, in the meantime anyway.

A few days ago, I got a request to make this behavior customizable,
which was CC-ed to emacs-pretesters instead of the more appropriate
emacs-devel.

The patch below introduces an option allowing the user to avoid the
question.  The default remains to ask the question.  But, even for the
default behavior there is a change: if the user answers no, the buffer
gets reverted without updating the database.

If there are no objections, I will install the patch below:

===File ~/locate-diff=======================================
*** locate.el	15 Mar 2006 19:31:47 -0600	1.36
--- locate.el	25 Jun 2006 20:49:16 -0500	
***************
*** 191,196 ****
--- 191,204 ----
    :group 'locate
    :version "22.1")
  
+ (defcustom locate-update-when-revert t
+   "This option affects how the *Locate* buffer gets reverted.
+ If non-nil, offer to update the locate database when reverting that buffer.
+ If nil, reverting does not update the locate database."
+   :type 'boolean
+   :group 'locate
+   :version "22.1")
+ 
  (defcustom locate-update-command "updatedb"
    "The executable program used to update the locate database."
    :type 'string
***************
*** 557,568 ****
  
  ;; From Stephen Eglen <stephen@cns.ed.ac.uk>
  (defun locate-update (ignore1 ignore2)
!   "Update the locate database.
! Database is updated using the shell command in `locate-update-command'."
    (let ((str (car locate-history-list)))
!     (cond ((yes-or-no-p "Update locate database (may take a few seconds)? ")
! 	   (shell-command locate-update-command)
! 	   (locate str)))))
  
  ;;; Modified three functions from `dired.el':
  ;;;   dired-find-directory,
--- 565,578 ----
  
  ;; From Stephen Eglen <stephen@cns.ed.ac.uk>
  (defun locate-update (ignore1 ignore2)
!   "Revert the *Locate* buffer.
! If `locate-update-when-revert' is non-nil, offer to update the
! locate database using the shell command in `locate-update-command'."
    (let ((str (car locate-history-list)))
!     (and locate-update-when-revert
! 	 (yes-or-no-p "Update locate database (may take a few seconds)? ")
! 	 (shell-command locate-update-command))
!     (locate str)))
  
  ;;; Modified three functions from `dired.el':
  ;;;   dired-find-directory,
============================================================

Re: Reverting *Locate* buffers.

[David Kastrup]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> There have been in the past several complaints about the fact that
> trying to revert a *Locate* buffers first asks whether you want to
> update the locate database.
>
> If you answer yes, then in my setup and I would guess in most
> setups, you get an error unless you are running as root, because you
> need to be root to update the locate database.  As a result, the
> buffer does not get reverted.

In this case, one option would be to try
(let ((default-directory "/sudo::")) ...
or (depending on the setup of the system)
(let ((default-directory "/su::")) ...
around rerunning the database.

This could be one customization option (and a failed regeneration as
normal user might point out the availability), I think.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Reverting *Locate* buffers.

[Peter Breton]

That sounds fine to me.

I've never actually used the revert feature.. someone
else sent me in a patch for it, and it seemed harmless
enough so I added it. I find the idea of reverting the
buffer by rebuilding the database to be a bit odd; I
would just rerun the search, and have an additional
function to rebuild the database. However, if people
are happy with the existing feature, then it still
seems harmless to me :)

Having an additional warning when not running as root
also sounds like a good idea.

Peter

> There have been in the past several complaints about
> the fact that
> trying to revert a *Locate* buffers first asks
> whether you want to
> update the locate database.
> 
> If you answer yes, then in my setup and I would
> guess in most setups,
> you get an error unless you are running as root,
> because you need to
> be root to update the locate database.  As a result,
> the buffer does
> not get reverted.
> 
> If you answer no, the *Locate* buffer does not get
> reverted either,
> whereas you might just have wanted to revert to undo
> manual changes
> to the buffer and whereas the locate database might
> have been updated
> by cron, or by the user, in the meantime anyway.
> 
> A few days ago, I got a request to make this
> behavior customizable,
> which was CC-ed to emacs-pretesters instead of the
> more appropriate
> emacs-devel.
> 
> The patch below introduces an option allowing the
> user to avoid the
> question.  The default remains to ask the question. 
> But, even for the
> default behavior there is a change: if the user
> answers no, the buffer
> gets reverted without updating the database.
> 
> If there are no objections, I will install the patch
> below:
> 
> ===File
> ~/locate-diff=======================================
> *** locate.el	15 Mar 2006 19:31:47 -0600	1.36
> --- locate.el	25 Jun 2006 20:49:16 -0500	
> ***************
> *** 191,196 ****
> --- 191,204 ----
>     :group 'locate
>     :version "22.1")
>   
> + (defcustom locate-update-when-revert t
> +   "This option affects how the *Locate* buffer
> gets reverted.
> + If non-nil, offer to update the locate database
> when reverting that buffer.
> + If nil, reverting does not update the locate
> database."
> +   :type 'boolean
> +   :group 'locate
> +   :version "22.1")
> + 
>   (defcustom locate-update-command "updatedb"
>     "The executable program used to update the
> locate database."
>     :type 'string
> ***************
> *** 557,568 ****
>   
>   ;; From Stephen Eglen <stephen@cns.ed.ac.uk>
>   (defun locate-update (ignore1 ignore2)
> !   "Update the locate database.
> ! Database is updated using the shell command in
> `locate-update-command'."
>     (let ((str (car locate-history-list)))
> !     (cond ((yes-or-no-p "Update locate database
> (may take a few seconds)? ")
> ! 	   (shell-command locate-update-command)
> ! 	   (locate str)))))
>   
>   ;;; Modified three functions from `dired.el':
>   ;;;   dired-find-directory,
> --- 565,578 ----
>   
>   ;; From Stephen Eglen <stephen@cns.ed.ac.uk>
>   (defun locate-update (ignore1 ignore2)
> !   "Revert the *Locate* buffer.
> ! If `locate-update-when-revert' is non-nil, offer
> to update the
> ! locate database using the shell command in
> `locate-update-command'."
>     (let ((str (car locate-history-list)))
> !     (and locate-update-when-revert
> ! 	 (yes-or-no-p "Update locate database (may take a
> few seconds)? ")
> ! 	 (shell-command locate-update-command))
> !     (locate str)))
>   
>   ;;; Modified three functions from `dired.el':
>   ;;;   dired-find-directory,
>
============================================================
> 
> 

Re: Reverting *Locate* buffers.

[Luc Teirlinck]

Peter Breton wrote:

   I've never actually used the revert feature.. someone
   else sent me in a patch for it, and it seemed harmless
   enough so I added it. I find the idea of reverting the
   buffer by rebuilding the database to be a bit odd; I
   would just rerun the search, and have an additional
   function to rebuild the database. However, if people
   are happy with the existing feature, then it still
   seems harmless to me :)

What about just setting the standard value of `locate-update-when-revert'
to nil?  Then the default behavior will make sense for most people and
people who prefer the old behavior just have to customize an option
to get it back.

   Having an additional warning when not running as root
   also sounds like a good idea.

The problem is that there is not that much space in the minibuffer.
But the docstring of `locate-update-when-revert' can mention the
problem.  Since one would need need to explicitly set this non-nil to
get asked the question, presumably people will read the docstring
before doing so.

The following patches and NEWS entry implement this.  I can install if
we all agree on this.

===File ~/NEWS-diff=========================================
*** NEWS	25 Jun 2006 09:51:35 -0500	1.1367
--- NEWS	26 Jun 2006 20:32:40 -0500	
***************
*** 3494,3499 ****
--- 3494,3508 ----
  (defun PP (data) (insert (format "%S\n" data)))
  (ewoc-create 'PP "start\n\n" "\n" t)
  
+ ** Locate changes
+ 
+ ---
+ *** By default, reverting the *Locate* buffer now just runs the last
+ `locate' command back over again without offering to update the locate
+ database (which normally only works if you have root privileges).  If
+ you prefer the old behavior, set the new customizable option
+ `locate-update-when-revert' to t.
+ 
  \f
  * Changes in Emacs 22.1 on non-free operating systems
  
============================================================

===File ~/locate-diff=======================================
*** locate.el	15 Mar 2006 19:31:47 -0600	1.36
--- locate.el	26 Jun 2006 20:11:34 -0500	
***************
*** 191,196 ****
--- 191,205 ----
    :group 'locate
    :version "22.1")
  
+ (defcustom locate-update-when-revert nil
+   "This option affects how the *Locate* buffer gets reverted.
+ If non-nil, offer to update the locate database when reverting that buffer.
+ \(Normally, you need to have root privileges for this to work.)
+ If nil, reverting does not update the locate database."
+   :type 'boolean
+   :group 'locate
+   :version "22.1")
+ 
  (defcustom locate-update-command "updatedb"
    "The executable program used to update the locate database."
    :type 'string
***************
*** 557,568 ****
  
  ;; From Stephen Eglen <stephen@cns.ed.ac.uk>
  (defun locate-update (ignore1 ignore2)
!   "Update the locate database.
! Database is updated using the shell command in `locate-update-command'."
    (let ((str (car locate-history-list)))
!     (cond ((yes-or-no-p "Update locate database (may take a few seconds)? ")
! 	   (shell-command locate-update-command)
! 	   (locate str)))))
  
  ;;; Modified three functions from `dired.el':
  ;;;   dired-find-directory,
--- 566,579 ----
  
  ;; From Stephen Eglen <stephen@cns.ed.ac.uk>
  (defun locate-update (ignore1 ignore2)
!   "Revert the *Locate* buffer.
! If `locate-update-when-revert' is non-nil, offer to update the
! locate database using the shell command in `locate-update-command'."
    (let ((str (car locate-history-list)))
!     (and locate-update-when-revert
! 	 (yes-or-no-p "Update locate database (may take a few seconds)? ")
! 	 (shell-command locate-update-command))
!     (locate str)))
  
  ;;; Modified three functions from `dired.el':
  ;;;   dired-find-directory,
============================================================

Re: Reverting *Locate* buffers.

[Luc Teirlinck]

David Kastrup wrote:

   In this case, one option would be to try
   (let ((default-directory "/sudo::")) ...
   or (depending on the setup of the system)
   (let ((default-directory "/su::")) ...
   around rerunning the database.

Using the latter, I got the following error message while trying to
revert the *Locate* buffer (cut and pasted from *Messages*):

Loading tramp... [4 times]
expand-file-name: Recursive load: "/usr/local/share/emacs/22.0.50/lisp/net/tramp.elc", "/usr/local/share/emacs/22.0.50/lisp/net/tramp.elc", "/usr/local/share/emacs/22.0.50/lisp/net/tramp.elc", "/usr/local/share/emacs/22.0.50/lisp/net/tramp.elc", "/usr/local/share/emacs/22.0.50/lisp/net/tramp.elc"

The *Locate* buffer was not reverted.

Sincerely,

Luc.

Re: Reverting *Locate* buffers.

[T. V. Raman]

While on the error message regarding tramp, I've found that tramp
being around causes *a lot* of recursive expand file calls ---
try running edebug on anything that calls expand-file-name to see
what I mean.

Basically, tramp should only fire on the  "head" of the path
being expanded -- at present it appears to fire for *each* path
component. 

>>>>> "Luc" == Luc Teirlinck <teirllm@dms.auburn.edu> writes:
    Luc> David Kastrup wrote: In this case, one option would be
    Luc> to try (let ((default-directory "/sudo::")) ...  or
    Luc> (depending on the setup of the system) (let
    Luc> ((default-directory "/su::")) ...  around rerunning the
    Luc> database.
    Luc> 
    Luc> Using the latter, I got the following error message
    Luc> while trying to revert the *Locate* buffer (cut and
    Luc> pasted from *Messages*):
    Luc> 
    Luc> Loading tramp... [4 times] expand-file-name: Recursive
    Luc> load:
    Luc> "/usr/local/share/emacs/22.0.50/lisp/net/tramp.elc",
    Luc> "/usr/local/share/emacs/22.0.50/lisp/net/tramp.elc",
    Luc> "/usr/local/share/emacs/22.0.50/lisp/net/tramp.elc",
    Luc> "/usr/local/share/emacs/22.0.50/lisp/net/tramp.elc",
    Luc> "/usr/local/share/emacs/22.0.50/lisp/net/tramp.elc"
    Luc> 
    Luc> The *Locate* buffer was not reverted.
    Luc> 
    Luc> Sincerely,
    Luc> 
    Luc> Luc.

-- 
Best Regards,
--raman

      
Email:  raman@users.sf.net
WWW:    http://emacspeak.sf.net/raman/
AIM:    emacspeak       GTalk: tv.raman.tv@gmail.com
PGP:    http://emacspeak.sf.net/raman/raman-almaden.asc
Google: tv+raman 
IRC:    irc://irc.freenode.net/#emacs

Re: Reverting *Locate* buffers.

[Luc Teirlinck]

T. V. Raman wrote:

   While on the error message regarding tramp, I've found that tramp
   being around causes *a lot* of recursive expand file calls ---
   try running edebug on anything that calls expand-file-name to see
   what I mean.

   Basically, tramp should only fire on the  "head" of the path
   being expanded -- at present it appears to fire for *each* path
   component. 

Just as a general question to everybody interested (not just to the
person in the "To" field):

Where do we stand now in this thread?  Obviously, I can not implement
David's suggestion as long as there is this error message (assuming it
would not give other problems, even without that message, which I can
not test).

Do I just commit my previously sent patch, or do we have the feeling
that there are bugs in Tramp which should be reported?  Note that
Tramp is independently maintained by tramp-devel, which I have CC-ed,
rather than by emacs-devel.  I believe that the people maintaining
Tramp also read emacs-devel, but I am not sure that they have followed
this thread.

Sincerely,

Luc.

Re: Reverting *Locate* buffers.

[David Kastrup]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> T. V. Raman wrote:
>
>    While on the error message regarding tramp, I've found that tramp
>    being around causes *a lot* of recursive expand file calls ---
>    try running edebug on anything that calls expand-file-name to see
>    what I mean.
>
>    Basically, tramp should only fire on the  "head" of the path
>    being expanded -- at present it appears to fire for *each* path
>    component. 
>
> Just as a general question to everybody interested (not just to the
> person in the "To" field):
>
> Where do we stand now in this thread?  Obviously, I can not implement
> David's suggestion as long as there is this error message (assuming it
> would not give other problems, even without that message, which I can
> not test).
>
> Do I just commit my previously sent patch, or do we have the feeling
> that there are bugs in Tramp which should be reported?

Sounds like a bug to me.  I think that my suggestion should be able to
work and it is not really out of the ordinary with regard to using
tramp.  That being said, using "cd" to change into root is certainly
not something to be done as a default setting, and it is actually not
possible to know whether the "sudo" or "su" method would need to get
used for a particular user on a particular system (if locate _is_
already running in some remote but non-root context, one would
probably even want to use a multihop method from tramp, but this is so
crazy that we really need not worry about it now).  But the feature is
so obscure that it needs to announce itself for customization, so
failure of updatedb should be detected and the variable to try for
customization in order to place updatedb in a tramp-governed announced
after the error message.

Note that this is a "wow feature": people will be pleasantly surprised
if it is there, but not really expect it.  So there is no large
incentive to get it going if other difficulties rather than a tramp
bug seem to make that infeasible.

> Note that Tramp is independently maintained by tramp-devel, which I
> have CC-ed, rather than by emacs-devel.  I believe that the people
> maintaining Tramp also read emacs-devel, but I am not sure that they
> have followed this thread.

Let's wait for their comment.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Reverting *Locate* buffers.

[Michael Albinus]

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

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Where do we stand now in this thread?  Obviously, I can not implement
> David's suggestion as long as there is this error message (assuming it
> would not give other problems, even without that message, which I can
> not test).

After playing a while, I could reproduce the same error here. In my
environment, I needed to recompile everything (make bootfast) and
start emacs without any preloaded Tramp (emacs -q). This should be the
default for most of the people except me ...

> Do I just commit my previously sent patch, or do we have the feeling
> that there are bugs in Tramp which should be reported?

There is a bug in Tramp. And you did sufficient reporting already,
thank you.

I don't believe I'll catch the bug tonight; it is some nasty
autoloading cycle I guess. But I'll do it next days.

When Tramp is preloaded (require 'tramp), your patch works fine. I've
enhanced it a little bit according to David's recommendations: a
custom option locate-update-path should provide enough freedom for
calling updatedb sufficiently. From my point of view you could commit
your patch in this (or your) version.

> Sincerely,
>
> Luc.

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 2285 bytes --]

*** /home/albinus/src/emacs/lisp/locate.el.~1.36.~	Sat Mar 18 17:17:04 2006
--- /home/albinus/src/emacs/lisp/locate.el	Thu Jun 29 23:06:57 2006
***************
*** 191,196 ****
--- 191,215 ----
    :group 'locate
    :version "22.1")
  
+ (defcustom locate-update-when-revert nil
+   "This option affects how the *Locate* buffer gets reverted.
+ If non-nil, offer to update the locate database when reverting that buffer.
+ \(Normally, you need to have root privileges for this to work.)
+ If nil, reverting does not update the locate database."
+   :type 'boolean
+   :group 'locate
+   :version "22.1")
+ 
+ (defcustom locate-update-path "/su::"
+   "The default directory from where `locate-update-command' is called.
+ Usually, root permissions are required running the command.  This
+ can be achieved by setting this option to \"/su::\" or \"/sudo::\".
+ If your current user permissions are sufficient to run the command,
+ you shall set this option to \"/\"."
+   :type 'string
+   :group 'locate
+   :version "22.1")
+ 
  (defcustom locate-update-command "updatedb"
    "The executable program used to update the locate database."
    :type 'string
***************
*** 557,568 ****
  
  ;; From Stephen Eglen <stephen@cns.ed.ac.uk>
  (defun locate-update (ignore1 ignore2)
!   "Update the locate database.
! Database is updated using the shell command in `locate-update-command'."
    (let ((str (car locate-history-list)))
!     (cond ((yes-or-no-p "Update locate database (may take a few seconds)? ")
! 	   (shell-command locate-update-command)
! 	   (locate str)))))
  
  ;;; Modified three functions from `dired.el':
  ;;;   dired-find-directory,
--- 576,590 ----
  
  ;; From Stephen Eglen <stephen@cns.ed.ac.uk>
  (defun locate-update (ignore1 ignore2)
!   "Revert the *Locate* buffer.
! If `locate-update-when-revert' is non-nil, offer to update the
! locate database using the shell command in `locate-update-command'."
    (let ((str (car locate-history-list)))
!     (and locate-update-when-revert
! 	 (yes-or-no-p "Update locate database (may take a few seconds)? ")
! 	 (let ((default-directory locate-update-path))
! 	   (shell-command locate-update-command)))
!     (locate str)))
  
  ;;; Modified three functions from `dired.el':
  ;;;   dired-find-directory,

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

_______________________________________________
Tramp-devel mailing list
Tramp-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/tramp-devel

Re: Reverting *Locate* buffers.

[Michael Albinus]

"T. V. Raman" <raman@users.sf.net> writes:

> While on the error message regarding tramp, I've found that tramp
> being around causes *a lot* of recursive expand file calls ---
> try running edebug on anything that calls expand-file-name to see
> what I mean.
>
> Basically, tramp should only fire on the  "head" of the path
> being expanded -- at present it appears to fire for *each* path
> component. 

That happens in `file-truename'. Every path component must be checked
for being a symlink. During this check, other primitive file name
operations are called, which call then `expand-file-name'.

In Tramp 2.1, several file name operations cache their result;
therefore much less calls are necessary.

Best regards, Michael.

Re: Reverting *Locate* buffers.

[Luc Teirlinck]

Michael Albinus wrote:

   When Tramp is preloaded (require 'tramp), your patch works fine

Not quite.  If you use your suggested version of the patch, then do
`emacs -q', then load tramp, then set locate-update-when-revert to t,
then do `M-x locate' RET emacs, then do `C-x 1' to have the *Locate*
buffer filling the entire frame, then run g, answer yes and give your
password, then instead of having one updated *Locate* buffer filling
the entire frame, the frame gets split into two windows, each showing
the same updated *Locate* buffer.  I find this quite surprising.  Is
this explainable by anything that Tramp does?  This bug is
inconvenient enough that I would rather stick with my original version
without David's suggestion if there would be no easy way around it.

This bug does not occur if locate-update-when-revert is nil.

Sincerely,

Luc.

Re: Reverting *Locate* buffers.

[Michael Albinus]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Michael Albinus wrote:
>
>    When Tramp is preloaded (require 'tramp), your patch works fine
>
> Not quite.  If you use your suggested version of the patch, then do
> `emacs -q', then load tramp, then set locate-update-when-revert to t,
> then do `M-x locate' RET emacs, then do `C-x 1' to have the *Locate*
> buffer filling the entire frame, then run g, answer yes and give your
> password, then instead of having one updated *Locate* buffer filling
> the entire frame, the frame gets split into two windows, each showing
> the same updated *Locate* buffer.  I find this quite surprising.  Is
> this explainable by anything that Tramp does?

It's a bug in Tramp. tramp-handle-shell-command didn't reset the
current buffer at the end. I've fixed that.

> Sincerely,
>
> Luc.

Best regards, Michael.

Re: Reverting *Locate* buffers.

[Michael Albinus]

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

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Just as a general question to everybody interested (not just to the
> person in the "To" field):
>
> Where do we stand now in this thread?  Obviously, I can not implement
> David's suggestion as long as there is this error message (assuming it
> would not give other problems, even without that message, which I can
> not test).

I believe I understand what happens when the "Recursive load" message
for Tramp appears. We have the following situation:

- Tramp is not loaded yet.
- default-directory is set to "/su::".
- shell-command is called.

The Tramp handler of shell-command must be applied. Therefore, due to
the autoload definitions, Tramp must be loaded. Loading tramp.el
itself causes new remote file operations (due to default-directory).
And so on.

The solution would be to set temporarily default-directory to "/" or
whatover local during loading of Tramp. But I have no idea how to say
it, I don't know of any before-load-hook where I could apply such a
setting. Does anybody has an idea how to do this?

For your patch, it would work with the small change that you arrange
the loading of Tramp yourself (a working version of the patch is
appended). But I don't like this solution too much, I'ld prefer that
Tramp could handle it itself.

> Sincerely,
>
> Luc.

Best regards, Michael.


[-- Attachment #2: Type: text/plain, Size: 2306 bytes --]

*** /home/albinus/src/emacs/lisp/locate.el	Sun Jul  2 21:46:24 2006
--- /home/albinus/src/emacs/lisp/locate.el.~1.36.~	Sat Mar 18 17:17:04 2006
***************
*** 191,215 ****
    :group 'locate
    :version "22.1")
  
- (defcustom locate-update-when-revert nil
-   "This option affects how the *Locate* buffer gets reverted.
- If non-nil, offer to update the locate database when reverting that buffer.
- \(Normally, you need to have root privileges for this to work.)
- If nil, reverting does not update the locate database."
-   :type 'boolean
-   :group 'locate
-   :version "22.1")
- 
- (defcustom locate-update-path "/su::"
-   "The default directory from where `locate-update-command' is called.
- Usually, root permissions are required running the command.  This
- can be achieved by setting this option to \"/su::\" or \"/sudo::\".
- If your current user permissions are sufficient to run the command,
- you shall set this option to \"/\"."
-   :type 'string
-   :group 'locate
-   :version "22.1")
- 
  (defcustom locate-update-command "updatedb"
    "The executable program used to update the locate database."
    :type 'string
--- 191,196 ----
***************
*** 576,591 ****
  
  ;; From Stephen Eglen <stephen@cns.ed.ac.uk>
  (defun locate-update (ignore1 ignore2)
!   "Revert the *Locate* buffer.
! If `locate-update-when-revert' is non-nil, offer to update the
! locate database using the shell command in `locate-update-command'."
    (let ((str (car locate-history-list)))
!     (and locate-update-when-revert
! 	 (yes-or-no-p "Update locate database (may take a few seconds)? ")
! 	 (require 'tramp)
! 	 (let ((default-directory locate-update-path))
! 	   (shell-command locate-update-command)))
!     (locate str)))
  
  ;;; Modified three functions from `dired.el':
  ;;;   dired-find-directory,
--- 557,568 ----
  
  ;; From Stephen Eglen <stephen@cns.ed.ac.uk>
  (defun locate-update (ignore1 ignore2)
!   "Update the locate database.
! Database is updated using the shell command in `locate-update-command'."
    (let ((str (car locate-history-list)))
!     (cond ((yes-or-no-p "Update locate database (may take a few seconds)? ")
! 	   (shell-command locate-update-command)
! 	   (locate str)))))
  
  ;;; Modified three functions from `dired.el':
  ;;;   dired-find-directory,

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

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: Reverting *Locate* buffers.

[Luc Teirlinck]

Michael Albinus wrote:

   The solution would be to set temporarily default-directory to "/" or
   whatover local during loading of Tramp. But I have no idea how to say
   it

I did not think hard about the problem and hence I may not understand
it, but is I guess that there must be some reason why just saving the
default directory at the start of tramp, setting it to "/" and setting
it back to the saved value at the end of tramp does not work?

Anyway, assuming that the problem really is hard to solve, I propose
the following patch.  The main difference with what you proposed is
that the default value of `locate-update-path' is "/" and that Tramp
does not get loaded if `locate-update-path' has that default value.

There are two reasons for this.  It makes it easier to revert to the
old default and that default does not load Tramp (it does not need
to).  Secondly, I would be very hesitant about asking for a root
password without the user having explicitly asked for that behavior
(through setting `locate-update-path').  If the user has no authority
to be root and inadvertently gives his own password at the prompt,
then mail may be sent to the "appropriate authorities", possibly
embarrassing the user.

I propose the following patch and if nobody objects, I will install
it.  if the recursive load problem would be solved some other way,
then obviously, the requiring of Tramp could be undone.

===File ~/locate-diff=======================================
*** locate.el	15 Mar 2006 19:31:47 -0600	1.36
--- locate.el	02 Jul 2006 17:35:17 -0500	
***************
*** 191,201 ****
--- 191,222 ----
    :group 'locate
    :version "22.1")
  
+ (defcustom locate-update-when-revert nil
+   "This option affects how the *Locate* buffer gets reverted.
+ If non-nil, offer to update the locate database when reverting that buffer.
+ \(Normally, you need to have root privileges for this to work.  See the
+ option `locate-update-path'.)
+ If nil, reverting does not update the locate database."
+   :type 'boolean
+   :group 'locate
+   :version "22.1")
+ 
  (defcustom locate-update-command "updatedb"
    "The executable program used to update the locate database."
    :type 'string
    :group 'locate)
  
+ (defcustom locate-update-path "/"
+   "The default directory from where `locate-update-command' is called.
+ Usually, root permissions are required to run that command.  This
+ can be achieved by setting this option to \"/su::\" or \"/sudo::\"
+ \(if you have the appropriate authority).  If your current user
+ permissions are sufficient to run the command, you can set this
+ option to \"/\"."
+   :type 'string
+   :group 'locate
+   :version "22.1")
+ 
  (defcustom locate-prompt-for-command nil
    "If non-nil, the `locate' command prompts for a command to run.
  Otherwise, that behavior is invoked via a prefix argument."
***************
*** 557,568 ****
  
  ;; From Stephen Eglen <stephen@cns.ed.ac.uk>
  (defun locate-update (ignore1 ignore2)
!   "Update the locate database.
! Database is updated using the shell command in `locate-update-command'."
    (let ((str (car locate-history-list)))
!     (cond ((yes-or-no-p "Update locate database (may take a few seconds)? ")
! 	   (shell-command locate-update-command)
! 	   (locate str)))))
  
  ;;; Modified three functions from `dired.el':
  ;;;   dired-find-directory,
--- 578,595 ----
  
  ;; From Stephen Eglen <stephen@cns.ed.ac.uk>
  (defun locate-update (ignore1 ignore2)
!   "Revert the *Locate* buffer.
! If `locate-update-when-revert' is non-nil, offer to update the
! locate database using the shell command in `locate-update-command'."
    (let ((str (car locate-history-list)))
!     (and locate-update-when-revert
! 	 (yes-or-no-p "Update locate database (may take a few seconds)? ")
! 	 (progn
! 	   (unless (equal locate-update-path "/")
! 	     (require 'tramp))
! 	   (let ((default-directory locate-update-path))
! 	     (shell-command locate-update-command))))
!     (locate str)))
  
  ;;; Modified three functions from `dired.el':
  ;;;   dired-find-directory,
============================================================

Re: Reverting *Locate* buffers.

[Michael Albinus]

Luc Teirlinck <teirllm@dms.auburn.edu> writes:

> Michael Albinus wrote:
>
>    The solution would be to set temporarily default-directory to "/" or
>    whatover local during loading of Tramp. But I have no idea how to say
>    it
>
> I did not think hard about the problem and hence I may not understand
> it, but is I guess that there must be some reason why just saving the
> default directory at the start of tramp, setting it to "/" and setting
> it back to the saved value at the end of tramp does not work?

The problem is that default directory must not be a remote filename
when (auto)loading tramp.el. Any counter measure "at the start of
tramp" (i.e. inside tramp.el) is too late.

> Anyway, assuming that the problem really is hard to solve, I propose
> the following patch.  The main difference with what you proposed is
> that the default value of `locate-update-path' is "/" and that Tramp
> does not get loaded if `locate-update-path' has that default value.
>
> There are two reasons for this.  It makes it easier to revert to the
> old default and that default does not load Tramp (it does not need
> to).  Secondly, I would be very hesitant about asking for a root
> password without the user having explicitly asked for that behavior
> (through setting `locate-update-path').  If the user has no authority
> to be root and inadvertently gives his own password at the prompt,
> then mail may be sent to the "appropriate authorities", possibly
> embarrassing the user.

That's OK for me.

> I propose the following patch and if nobody objects, I will install
> it.  if the recursive load problem would be solved some other way,
> then obviously, the requiring of Tramp could be undone.

It's really too hot these days. One obvious solution avoiding to
require Tramp explicitely coould be this:

;; From Stephen Eglen <stephen@cns.ed.ac.uk>
(defun locate-update (ignore1 ignore2)
  "Revert the *Locate* buffer.
If `locate-update-when-revert' is non-nil, offer to update the
locate database using the shell command in `locate-update-command'."
  (let ((str (car locate-history-list)))
    (and locate-update-when-revert
	 (yes-or-no-p "Update locate database (may take a few seconds)? ")
         ;; `expand-file-name' is used in order to autoload Tramp if
         ;; necessary.  It cannot be loaded when `default-directory'
         ;; is remote.
	 (let ((default-directory (expand-file-name locate-update-path)))
	   (shell-command locate-update-command)))
    (locate str)))

Best regards, Michael.

Re: Reverting *Locate* buffers.

[Richard Stallman]

    The solution would be to set temporarily default-directory to "/" or
    whatover local during loading of Tramp. But I have no idea how to say
    it,

Does

(let ((default-directory "/"))
  (require 'tramp))

do the job?

Re: Reverting *Locate* buffers.

[Michael Albinus]

Richard Stallman <rms@gnu.org> writes:

>     The solution would be to set temporarily default-directory to "/" or
>     whatover local during loading of Tramp. But I have no idea how to say
>     it,
>
> Does
>
> (let ((default-directory "/"))
>   (require 'tramp))
>
> do the job?

Of course. But this would be given to the _consumers_ of Tramp,
contradicting the concept of autoload. And sometimes they shouldn't
even know that Tramp is required.

Best regards, Michael.

Re: Reverting *Locate* buffers.

[David Kastrup]

Michael Albinus <michael.albinus@gmx.de> writes:

> Richard Stallman <rms@gnu.org> writes:
>
>>     The solution would be to set temporarily default-directory to "/" or
>>     whatover local during loading of Tramp. But I have no idea how to say
>>     it,
>>
>> Does
>>
>> (let ((default-directory "/"))
>>   (require 'tramp))
>>
>> do the job?
>
> Of course. But this would be given to the _consumers_ of Tramp,
> contradicting the concept of autoload. And sometimes they shouldn't
> even know that Tramp is required.

Maybe `autoload' should locally bind default-directory to some default
value.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: Reverting *Locate* buffers.

[Richard Stallman]

    The problem is that default directory must not be a remote filename
    when (auto)loading tramp.el. Any counter measure "at the start of
    tramp" (i.e. inside tramp.el) is too late.

The value of default-directory does not normally affect the loading of
a file.  How does it affect the loading of tramp.el?

Maybe you can change that (whatever it is) not to look at
default-directory any more.

Re: Reverting *Locate* buffers.

[Luc Teirlinck]

Michael Albinus wrote:

   It's really too hot these days. One obvious solution avoiding to
   require Tramp explicitely coould be this:

I have installed the latest version of my patch with this one change
(using expand-filename instead of requiring Tramp explicitly).

Sincerely,

Luc.

Re: Reverting *Locate* buffers.

[Stefan Monnier]

> The Tramp handler of shell-command must be applied. Therefore, due to
> the autoload definitions, Tramp must be loaded. Loading tramp.el
> itself causes new remote file operations (due to default-directory).
> And so on.

Maybe a backtrace could help decide where the default-directory should be
temporarily rebound to / ?


        Stefan