{"thread":{"id":"44788","subject":"[PATCH] git-p4: do not pass '-r 0' to p4 commands","startedAt":"2016-12-29T09:06:53Z","lastAt":"2016-12-29T10:02:17Z","messageCount":3,"participants":["Igor Kushnir","Luke Diamand","Lars Schneider"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"308483","messageId":"20161229090533.4717-1-igorkuo@gmail.com","threadId":"44788","inReplyTo":null,"subject":"[PATCH] git-p4: do not pass '-r 0' to p4 commands","fromName":"Igor Kushnir","fromEmail":"igorkuo@gmail.com","sentAt":"2016-12-29T09:05:33Z","receivedAt":"2016-12-29T09:06:53Z","isPatch":true,"sender":{"key":"igorkuo@gmail.com","avatar":null},"body":"git-p4 crashes when used with a very old p4 client version\nthat does not support the '-r <number>' option in its commands.\n\nAllow making git-p4 work with old p4 clients by setting git-p4.retries to 0.\n\nAlternatively git-p4.retries could be made opt-in.\nBut since only very old, barely maintained p4 versions don't support\nthe '-r' option, the setting-retries-to-0 workaround would do.\n\nThe \"-r retries\" option is present in Perforce 2012.2 Command Reference,\nbut absent from Perforce 2012.1 Command Reference.\n\nSigned-off-by: Igor Kushnir <igorkuo@gmail.com>\n---\n Documentation/git-p4.txt | 1 +\n git-p4.py                | 4 +++-\n 2 files changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\nindex bae862ddc..f4f1be5be 100644\n--- a/Documentation/git-p4.txt\n+++ b/Documentation/git-p4.txt\n@@ -479,6 +479,7 @@ git-p4.client::\n git-p4.retries::\n \tSpecifies the number of times to retry a p4 command (notably,\n \t'p4 sync') if the network times out. The default value is 3.\n+\t'-r 0' is not passed to p4 commands if this option is set to 0.\n \n Clone and sync variables\n ~~~~~~~~~~~~~~~~~~~~~~~~\ndiff --git a/git-p4.py b/git-p4.py\nindex 22e3f57e7..e5a9e1cce 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -83,7 +83,9 @@ def p4_build_cmd(cmd):\n     if retries is None:\n         # Perform 3 retries by default\n         retries = 3\n-    real_cmd += [\"-r\", str(retries)]\n+    if retries != 0:\n+        # Provide a way to not pass this option by setting git-p4.retries to 0\n+        real_cmd += [\"-r\", str(retries)]\n \n     if isinstance(cmd,basestring):\n         real_cmd = ' '.join(real_cmd) + ' ' + cmd\n-- \n2.11.0\n\n"},{"id":"308484","messageId":"CAE5ih78HgKMOgXC82eTC3j-ehLmPXmNd0jQPJLcZebFXhWxOLQ@mail.gmail.com","threadId":"44788","inReplyTo":"20161229090533.4717-1-igorkuo@gmail.com","subject":"Re: [PATCH] git-p4: do not pass '-r 0' to p4 commands","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2016-12-29T09:20:27Z","receivedAt":"2016-12-29T09:27:45Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 29 December 2016 at 09:05, Igor Kushnir <igorkuo@gmail.com> wrote:\n> git-p4 crashes when used with a very old p4 client version\n> that does not support the '-r <number>' option in its commands.\n>\n> Allow making git-p4 work with old p4 clients by setting git-p4.retries to 0.\n>\n> Alternatively git-p4.retries could be made opt-in.\n> But since only very old, barely maintained p4 versions don't support\n> the '-r' option, the setting-retries-to-0 workaround would do.\n>\n> The \"-r retries\" option is present in Perforce 2012.2 Command Reference,\n> but absent from Perforce 2012.1 Command Reference.\n\nThat looks like a good fix, thanks.\n\nI feel sad for anyone still using 2012.1.\n\nLuke\n\n\n>\n> Signed-off-by: Igor Kushnir <igorkuo@gmail.com>\n> ---\n>  Documentation/git-p4.txt | 1 +\n>  git-p4.py                | 4 +++-\n>  2 files changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\n> index bae862ddc..f4f1be5be 100644\n> --- a/Documentation/git-p4.txt\n> +++ b/Documentation/git-p4.txt\n> @@ -479,6 +479,7 @@ git-p4.client::\n>  git-p4.retries::\n>         Specifies the number of times to retry a p4 command (notably,\n>         'p4 sync') if the network times out. The default value is 3.\n> +       '-r 0' is not passed to p4 commands if this option is set to 0.\n>\n>  Clone and sync variables\n>  ~~~~~~~~~~~~~~~~~~~~~~~~\n> diff --git a/git-p4.py b/git-p4.py\n> index 22e3f57e7..e5a9e1cce 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -83,7 +83,9 @@ def p4_build_cmd(cmd):\n>      if retries is None:\n>          # Perform 3 retries by default\n>          retries = 3\n> -    real_cmd += [\"-r\", str(retries)]\n> +    if retries != 0:\n> +        # Provide a way to not pass this option by setting git-p4.retries to 0\n> +        real_cmd += [\"-r\", str(retries)]\n>\n>      if isinstance(cmd,basestring):\n>          real_cmd = ' '.join(real_cmd) + ' ' + cmd\n> --\n> 2.11.0\n>\n"},{"id":"308486","messageId":"6AA88917-5F94-498C-BB7A-60192127C36C@gmail.com","threadId":"44788","inReplyTo":"20161229090533.4717-1-igorkuo@gmail.com","subject":"Re: [PATCH] git-p4: do not pass '-r 0' to p4 commands","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-12-29T10:02:12Z","receivedAt":"2016-12-29T10:02:17Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 29 Dec 2016, at 10:05, Igor Kushnir <igorkuo@gmail.com> wrote:\n> \n> git-p4 crashes when used with a very old p4 client version\n> that does not support the '-r <number>' option in its commands.\n> \n> Allow making git-p4 work with old p4 clients by setting git-p4.retries to 0.\n> \n> Alternatively git-p4.retries could be made opt-in.\n> But since only very old, barely maintained p4 versions don't support\n> the '-r' option, the setting-retries-to-0 workaround would do.\n> \n> The \"-r retries\" option is present in Perforce 2012.2 Command Reference,\n> but absent from Perforce 2012.1 Command Reference.\n\nThanks for this workaround!\n\n\n> Signed-off-by: Igor Kushnir <igorkuo@gmail.com>\n> ---\n> Documentation/git-p4.txt | 1 +\n> git-p4.py                | 4 +++-\n> 2 files changed, 4 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/git-p4.txt b/Documentation/git-p4.txt\n> index bae862ddc..f4f1be5be 100644\n> --- a/Documentation/git-p4.txt\n> +++ b/Documentation/git-p4.txt\n> @@ -479,6 +479,7 @@ git-p4.client::\n> git-p4.retries::\n> \tSpecifies the number of times to retry a p4 command (notably,\n> \t'p4 sync') if the network times out. The default value is 3.\n> +\t'-r 0' is not passed to p4 commands if this option is set to 0.\n\nAt this point in the docs we have never talked about the \"-r\" flag and I\nwould argue it is an \"implementation detail\". Therefore, I would prefer\nsomething like: \"Set the value to 0 if want to disable retries or if \nyour p4 version does not support retries (pre 2012.2).\"\n\n\n> \n> Clone and sync variables\n> ~~~~~~~~~~~~~~~~~~~~~~~~\n> diff --git a/git-p4.py b/git-p4.py\n> index 22e3f57e7..e5a9e1cce 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -83,7 +83,9 @@ def p4_build_cmd(cmd):\n>     if retries is None:\n>         # Perform 3 retries by default\n>         retries = 3\n> -    real_cmd += [\"-r\", str(retries)]\n> +    if retries != 0:\n\nHow about \"retries > 0\"?\n\n\n> +        # Provide a way to not pass this option by setting git-p4.retries to 0\n> +        real_cmd += [\"-r\", str(retries)]\n> \n>     if isinstance(cmd,basestring):\n>         real_cmd = ' '.join(real_cmd) + ' ' + cmd\n> -- \n> 2.11.0\n> \n\n"}]}