Minor VCS Info things

Minor VCS Info things

[Peter Stephenson]

Some minor changes for VCS_Info I'd like to suggest, if they're
uncontroversial.

vcs_comm needs to be local in all top-level functions, otherwise you get
errors trying to set elements in it when it doesn't exist from
e.g. vcs_info_printsys.

I think it's better to sanitize all options with "emulate" rather than
second guess which ones are needed, because there are lots and trying to
guess can easily cause problems (compare the completion code).  If there
are options that need to be kept they can be handled specially.  The
special problem in completion that stopped us from using "emulate" was
that completion's own behaviour is controlled by options and they're not
easily characterised by their emulation behaviour; I don't think we
should have that problem in an add-on non-interactive function suite,
but we can see.

It's confusing that if you run "vcs_info_printsys" to see what is
available it shows nothing if you haven't yet run "vcs_info" or
"vcs_info_setsys".  I think it should run vcs_info_setsys in that case.

The Perforce detector doesn't work very well.  $P4CONFIG is not tied to
being in the Perforce workspace: you can have a config file outside a
workspace to set a default client etc., and you don't need one at all if
the client etc. are set via environment variables.  The only good way of
detecting whether you're in a workspace is to ask the server.
Unfortunately this can hang (though not permanently) if the server is
not available, so I haven't sent a patch.  It could be fixed by adding
an explicit style to use the server and also a global (per server:port
pair) flag that the server didn't respond, so that it only hangs (for a
second or two) the first time.

Index: Functions/VCS_Info/vcs_info_lastmsg
===================================================================
RCS file: /cvsroot/zsh/zsh/Functions/VCS_Info/vcs_info_lastmsg,v
retrieving revision 1.1
diff -u -r1.1 vcs_info_lastmsg
--- Functions/VCS_Info/vcs_info_lastmsg	19 Sep 2008 12:58:52 -0000	1.1
+++ Functions/VCS_Info/vcs_info_lastmsg	22 Sep 2008 13:55:25 -0000
@@ -2,7 +2,8 @@
 ## Written by Frank Terbeck <ft@bewatermyfriend.org>
 ## Distributed under the same BSD-ish license as zsh itself.
 
-setopt localoptions NO_shwordsplit
+emulate -L zsh
+
 local -i i
 local -ix maxexports
 
Index: Functions/VCS_Info/vcs_info_printsys
===================================================================
RCS file: /cvsroot/zsh/zsh/Functions/VCS_Info/vcs_info_printsys,v
retrieving revision 1.1
diff -u -r1.1 vcs_info_printsys
--- Functions/VCS_Info/vcs_info_printsys	19 Sep 2008 12:58:52 -0000	1.1
+++ Functions/VCS_Info/vcs_info_printsys	22 Sep 2008 13:55:25 -0000
@@ -2,13 +2,20 @@
 ## Written by Frank Terbeck <ft@bewatermyfriend.org>
 ## Distributed under the same BSD-ish license as zsh itself.
 
-setopt localoptions noksharrays extendedglob NO_shwordsplit
+emulate -L zsh
+setopt extendedglob
+
 local sys
 local -a disabled enabled
+local -Ax vcs_comm
 
 zstyle -a ":vcs_info:-init-:${1:-default}:-all-" "enable" enabled
 (( ${#enabled} == 0 )) && enabled=( all )
 
+if (( ${+VCS_INFO_backends} == 0 )); then
+  vcs_info_setsys
+fi
+
 if [[ -n ${(M)enabled:#(#i)all} ]] ; then
     enabled=( ${VCS_INFO_backends} )
     zstyle -a ":vcs_info:-init-:${1:-default}:-all-" "disable" disabled
Index: Functions/VCS_Info/vcs_info_setsys
===================================================================
RCS file: /cvsroot/zsh/zsh/Functions/VCS_Info/vcs_info_setsys,v
retrieving revision 1.1
diff -u -r1.1 vcs_info_setsys
--- Functions/VCS_Info/vcs_info_setsys	19 Sep 2008 12:58:52 -0000	1.1
+++ Functions/VCS_Info/vcs_info_setsys	22 Sep 2008 13:55:25 -0000
@@ -2,7 +2,9 @@
 ## Written by Frank Terbeck <ft@bewatermyfriend.org>
 ## Distributed under the same BSD-ish license as zsh itself.
 
-setopt localoptions noksharrays extendedglob typeset_silent NO_shwordsplit
+emulate -L zsh
+setopt extendedglob typeset_silent
+
 local sys
 typeset -g VCS_INFO_backends
 


-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: Minor VCS Info things

[Frank Terbeck]

Peter Stephenson <pws@csr.com>:
> Some minor changes for VCS_Info I'd like to suggest, if they're
> uncontroversial.
[...]

I think all of these suggestions are the right thing to do.

> The Perforce detector doesn't work very well.  $P4CONFIG is not tied to
> being in the Perforce workspace: you can have a config file outside a
> workspace to set a default client etc., and you don't need one at all if
> the client etc. are set via environment variables.  The only good way of
> detecting whether you're in a workspace is to ask the server.
> Unfortunately this can hang (though not permanently) if the server is
> not available, so I haven't sent a patch.  It could be fixed by adding
> an explicit style to use the server and also a global (per server:port
> pair) flag that the server didn't respond, so that it only hangs (for a
> second or two) the first time.

I'm not familiar with perforce myself. I think it's safe to trust
people who actually use a system. The explicit style idea sounds nice
enough.

Whatever works for perforce users, we should do.

Regards, Frank

-- 
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
                                                  -- RFC 1925

Re: Minor VCS Info things

[Phil Pennock]

On 2008-09-22 at 15:11 +0100, Peter Stephenson wrote:
> The Perforce detector doesn't work very well.

This is my fault; as I noted to Frank when I supplied it, I've only ever
used P4 in this one environment, which is *very* heavily customised, and
frankly don't know what's normal and what isn't.  I offered something
minimal which works for me.

I am in no way wedded to the existing P4 support, but I simply don't
know enough to provide better.

-Phil

Re: Minor VCS Info things

[Peter Stephenson]

On Mon, 22 Sep 2008 15:11:02 +0100
Peter Stephenson <pws@csr.com> wrote:
> The Perforce detector doesn't work very well.  $P4CONFIG is not tied to
> being in the Perforce workspace: you can have a config file outside a
> workspace to set a default client etc., and you don't need one at all if
> the client etc. are set via environment variables.  The only good way of
> detecting whether you're in a workspace is to ask the server.
> Unfortunately this can hang (though not permanently) if the server is
> not available, so I haven't sent a patch.  It could be fixed by adding
> an explicit style to use the server and also a global (per server:port
> pair) flag that the server didn't respond, so that it only hangs (for a
> second or two) the first time.

I've done this.  It seems to work OK.

Index: Doc/Zsh/contrib.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/contrib.yo,v
retrieving revision 1.88
diff -u -r1.88 contrib.yo
--- Doc/Zsh/contrib.yo	22 Sep 2008 17:22:48 -0000	1.88
+++ Doc/Zsh/contrib.yo	23 Sep 2008 15:55:51 -0000
@@ -502,6 +502,21 @@
 repositories (checked in the var(-init-) context, too). Only used if
 tt(enable) contains tt(ALL).
 )
+kindex(use-server)
+item(tt(use-server))(
+This is used by the Perforce backend (tt(p4)) to decide if it should
+contact the Perforce server to find out if a directory is managed
+by Perforce.  This is the only reliable way of doing this, but runs
+the risk of a delay if the server name cannot be found.  If the
+server (more specifically, the var(host)tt(:)var(port) pair describing the
+server) cannot be contacted its name is put into the associative array
+tt(vcs_info_p4_dead_servers) and not contacted again during the session
+until it is removed by hand.  If you do not set this style, the tt(p4)
+backend is only usable if you have set the environment variable
+tt(P4CONFIG) to a file name and have corresponding files in the root
+directories of each Perforce client.  See comments in the function
+tt(VCS_INFO_detect_p4) for more detail.
+)
 kindex(use-simple)
 item(tt(use-simple))(
 If there are two different ways of gathering
Index: Functions/VCS_Info/Backends/VCS_INFO_detect_p4
===================================================================
RCS file: /cvsroot/zsh/zsh/Functions/VCS_Info/Backends/VCS_INFO_detect_p4,v
retrieving revision 1.1
diff -u -r1.1 VCS_INFO_detect_p4
--- Functions/VCS_Info/Backends/VCS_INFO_detect_p4	19 Sep 2008 12:58:53 -0000	1.1
+++ Functions/VCS_Info/Backends/VCS_INFO_detect_p4	23 Sep 2008 15:55:51 -0000
@@ -2,8 +2,76 @@
 ## perforce support by: Phil Pennock
 ## Distributed under the same BSD-ish license as zsh itself.
 
-[[ -n ${P4CONFIG} ]] || return 1
-VCS_INFO_check_com p4 || return 1
-vcs_comm[detect_need_file]="${P4CONFIG}"
-VCS_INFO_bydir_detect .
-return $?
+# If user-server is true in the :vcs_info:p4:... context, contact the
+# server to decide whether the directory is handled by Perforce.  This can
+# cause a delay if the network times out, in particular if looking up the
+# server name failed.  Hence this is not the default.  If a timeout
+# occurred, the server:port pair is added to the associative array
+# vcs_info_p4_dead_servers and the server is never contacted again.  The
+# array must be edited by hand to remove it.
+#
+# If user-server is false or not set, the function looks to see if there is
+# a file $P4CONFIG somewhere above in the hierarchy.  This is far from
+# foolproof; in fact it relies on you using the particular working practice
+# of having such files in all client root directories and nowhere above.
+
+
+VCS_INFO_p4_get_server() {
+  emulate -L zsh
+  setopt extendedglob
+
+  local -a settings
+  settings=(${(f)"$(p4 set)"})
+  serverport=${${settings[(r)P4PORT=*]##P4PORT=}%% *}
+  case $serverport in
+    (''|:)
+    serverport=perforce:1666
+    ;;
+
+    (:*)
+    serverport=perforce${serverport}
+    ;;
+
+    (*:)
+    serverport=${serverport}1666
+    ;;
+
+    (<->)
+    serverport=perforce:${serverport}
+    ;;
+  esac
+}
+
+
+VCS_INFO_detect_p4() {
+  local serverport p4where
+
+  if zstyle -t ":vcs_info:p4:${usercontext}:${rrn}" use-server; then
+    # Use "p4 where" to decide whether the path is under the
+    # client workspace.
+    if (( ${#vcs_info_p4_dead_servers} )); then
+      # See if the server is in the list of defunct servers
+      VCS_INFO_p4_get_server
+      [[ -n $vcs_info_p4_dead_servers[$serverport] ]] && return 1
+    fi
+    if p4where="$(p4 where 2>&1)"; then
+      return 0
+    fi
+    if [[ $p4where = *"Connect to server failed"* ]]; then
+      # If the connection failed, mark the server as defunct.
+      # Otherwise it worked but we weren't within a client.
+      typeset -gA vcs_info_p4_dead_servers
+      [[ -z $serverport ]] && VCS_INFO_p4_get_server
+      vcs_info_p4_dead_servers[$serverport]=1
+    fi
+    return 1
+  else
+    [[ -n ${P4CONFIG} ]] || return 1
+    VCS_INFO_check_com p4 || return 1
+    vcs_comm[detect_need_file]="${P4CONFIG}"
+    VCS_INFO_bydir_detect .
+    return $?
+  fi
+}
+
+VCS_INFO_detect_p4 "$@"

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: Minor VCS Info things

[Phil Pennock]

On 2008-09-23 at 17:02 +0100, Peter Stephenson wrote:
> I've done this.  It seems to work OK.

As a side note: there's a scenario I have at $work where use-server
won't work but the simple case will: multiple P4 servers and the file
named by $P4CONFIG, in the root directory of the client, setting P4PORT
to the correct server.

I don't know of a way to deal with this for the use-server case, since
environment variables can't be auto-set by directory (although I could
use precmd() to mangle that in, just for this, I suppose).

So ...

> Index: Doc/Zsh/contrib.yo
[...]
> +until it is removed by hand.  If you do not set this style, the tt(p4)
> +backend is only usable if you have set the environment variable
> +tt(P4CONFIG) to a file name and have corresponding files in the root
> +directories of each Perforce client.  See comments in the function
> +tt(VCS_INFO_detect_p4) for more detail.

Suggest adding before the "See comments[...]" sentence:

  Using the tt(P4CONFIG) method may function better in environments with
  multiple P4 servers where tt(P4PORT) is set in the file named by
  tt(P4CONFIG).

-Phil

Re: Minor VCS Info things

[Peter Stephenson]

Phil Pennock wrote:
> On 2008-09-23 at 17:02 +0100, Peter Stephenson wrote:
> > I've done this.  It seems to work OK.
> 
> As a side note: there's a scenario I have at $work where use-server
> won't work but the simple case will: multiple P4 servers and the file
> named by $P4CONFIG, in the root directory of the client, setting P4PORT
> to the correct server.

I don't see why this shouldn't work.  The only access to the server from
the functions is via standard Perforce commands, which will search for
$P4CONFIG.  That's why I used "p4 set" to probe the server name: in
recent versions of Perforce it will provide the settings that the client
would use to talk to the server (and not simply print out the
environment variables).  Even if that doesn't work, "p4 where" will
contact the right server by the usual rules; the only problem would be
that, if that failed, the wrong server would be recorded as inactive.

The only case not covered is if you need some global options to "p4".
You can fix that by writing your own p4 front-end function.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070

Re: Minor VCS Info things

[Phil Pennock]

On 2008-09-24 at 09:46 +0100, Peter Stephenson wrote:
> I don't see why this shouldn't work.  The only access to the server from
> the functions is via standard Perforce commands, which will search for
> $P4CONFIG.

*doh*  Sometimes I'm so blond.  Sorry.

-Phil