Re: Bug#512308: zsh: out-of-date completions for svn revert

Re: Bug#512308: zsh: out-of-date completions for svn revert

[Clint Adams]

On Mon, Jan 19, 2009 at 03:55:20PM +0100, Vincent Lefevre wrote:
> Once completions have been done, information for "svn revert"
> can become out-of-date. Let's see the problem on an example,
> in an empty working copy:
> 
> $ touch blah1
> $ svn add blah1
> $ svn revert [TAB]
> 
> This one completes to "svn revert blah1" as expected. Type [Enter].
> 
> $ touch blah2
> $ svn add blah2
> $ svn revert [TAB]
> 
> This one completes to "svn revert blah1" again, while it should have
> completed to "svn revert blah2".

Is there a timestamp that the 'svn add' will touch so we could compare it
with the cache age?

Re: Bug#512308: zsh: out-of-date completions for svn revert

[Vincent Lefevre]

On 2009-01-19 15:06:02 +0000, Clint Adams wrote:
> Is there a timestamp that the 'svn add' will touch so we could
> compare it with the cache age?

.svn/entries is modified.

-- 
Vincent Lefèvre <vincent@vinc17.org> - Web: <http://www.vinc17.org/>
100% accessible validated (X)HTML - Blog: <http://www.vinc17.org/blog/>
Work: CR INRIA - computer arithmetic / Arenaire project (LIP, ENS-Lyon)

Re: Bug#512308: zsh: out-of-date completions for svn revert

[Clint Adams]

On Tue, Jan 20, 2009 at 02:28:37AM +0100, Vincent Lefevre wrote:
> .svn/entries is modified.

If _subversion were modified to use the caching layer, it could
invalidate the cache if .svn/entries were newer.  Otherwise,
I can only imagine something like recording $EPOCHSECONDS when
the array is set and then statting .svn/entries, or doing away
with the cache altogether.

I would suggest that somebody else work on this, as I still hate
svn.

PATCH: Re: Bug#512308: zsh: out-of-date completions for svn revert

[Greg Klanderman]

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


>>>>> Clint Adams <schizo@debian.org> writes:

> On Mon, Jan 19, 2009 at 03:55:20PM +0100, Vincent Lefevre wrote:
>> Once completions have been done, information for "svn revert"
>> can become out-of-date. Let's see the problem on an example,
>> in an empty working copy:
>> 
>> $ touch blah1
>> $ svn add blah1
>> $ svn revert [TAB]
>> 
>> This one completes to "svn revert blah1" as expected. Type [Enter].
>> 
>> $ touch blah2
>> $ svn add blah2
>> $ svn revert [TAB]
>> 
>> This one completes to "svn revert blah1" again, while it should have
>> completed to "svn revert blah2".

> Is there a timestamp that the 'svn add' will touch so we could compare it
> with the cache age?

Here's a patch to fix this svn completion problem reported back in
January [workers #26367].. things were even worse than it appeared
because the key used for caching 'svn status' output for a directory
was the relative directory path, which would usually be '.' so if you
cd'd around it would have exhibited worse problems.  It's now caching
based on the directory device and inode, and using the mtime of the
.svn/entries file to invalidate a cached entry.

cheers,
Greg


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

Index: Completion/Unix/Command/_subversion
===================================================================
RCS file: /cvsroot/zsh/zsh/Completion/Unix/Command/_subversion,v
retrieving revision 1.26
diff -u -r1.26 _subversion
--- Completion/Unix/Command/_subversion	17 May 2009 18:23:10 -0000	1.26
+++ Completion/Unix/Command/_subversion	18 May 2009 02:19:15 -0000
@@ -22,7 +22,7 @@
     ;;
     args)
       local cmd args usage
-      typeset -gHA _cache_svn_status
+      typeset -gHA _cache_svn_status _cache_svn_mtime
 
       cmd="${${(k)_svn_cmds[(R)*:$words[1]:*]}:-${(k)_svn_cmds[(i):$words[1]:]}}"
       if (( $#cmd )); then
@@ -193,11 +193,16 @@
   local dir=$REPLY:h
   local pat="${1:-([ADMR~]|?M)}"
 
-  if (( ! $+_cache_svn_status[$dir] )); then
-    _cache_svn_status[$dir]="$(_call_program files svn status -N $dir)"
+  zmodload -F zsh/stat b:zstat 2>/dev/null
+  local key="$(zstat +device $dir):$(zstat +inode $dir)"
+  local mtime="$(zstat +mtime $dir/.svn/entries)"
+
+  if (( ! $+_cache_svn_status[$key] || _cache_svn_mtime[$key] != mtime )); then
+    _cache_svn_status[$key]="$(_call_program files svn status -N $dir)"
+    _cache_svn_mtime[$key]="$mtime"
   fi
 
-  (( ${(M)#${(f)_cache_svn_status[$dir]}:#(#s)${~pat}*$REPLY} ))
+  (( ${(M)#${(f)_cache_svn_status[$key]}:#(#s)${~pat}*$REPLY} ))
 }
 
 (( $+functions[_svn_urls] )) ||

Re: PATCH: Re: Bug#512308: zsh: out-of-date completions for svn revert

[Vincent Lefevre]

On 2009-05-17 22:34:01 -0400, Greg Klanderman wrote:
> Here's a patch to fix this svn completion problem reported back in
> January [workers #26367].. things were even worse than it appeared
> because the key used for caching 'svn status' output for a directory
> was the relative directory path, which would usually be '.' so if you
> cd'd around it would have exhibited worse problems.  It's now caching
> based on the directory device and inode, and using the mtime of the
> .svn/entries file to invalidate a cached entry.

Thanks! I've done some tests and it seems to work well.

-- 
Vincent Lefèvre <vincent@vinc17.org> - Web: <http://www.vinc17.org/>
100% accessible validated (X)HTML - Blog: <http://www.vinc17.org/blog/>
Work: CR INRIA - computer arithmetic / Arenaire project (LIP, ENS-Lyon)

Re: PATCH: Re: Bug#512308: zsh: out-of-date completions for svn revert

[Greg Klanderman]

>>>>> Vincent Lefevre <vincent@vinc17.org> writes:

> Thanks! I've done some tests and it seems to work well.

Thank you for testing it Vincent!

Peter, could you commit this, or let me know if there's
something you'd like me to address differently?

thanks,
Greg