{"thread":{"id":"48306","subject":"[BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key","startedAt":"2018-04-16T23:36:25Z","lastAt":"2018-04-18T14:59:59Z","messageCount":15,"participants":["Thandesha VK","Andrey Mazo","Mazo, Andrey","Luke Diamand"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"344849","messageId":"CAJJpmi-pLb4Qcka5aLKXA8B1VOZFFF+OAQ0fgUq9YviobRpYGg@mail.gmail.com","threadId":"48306","inReplyTo":null,"subject":"[BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key","fromName":"Thandesha VK","fromEmail":"thanvk@gmail.com","sentAt":"2018-04-16T23:35:39Z","receivedAt":"2018-04-16T23:36:25Z","isPatch":false,"sender":{"key":"thanvk@gmail.com","avatar":null},"body":"git p4 clone fails when p4 sizes does not return 'fileSize' key. There\nare few cases when p4 sizes returens 0 size and with marshaled output,\nit doesn’t return the fileSize attribute.\n\nHere is the demonstration and potential fix\n\n\n\n$ cd /tmp/git/\n\n\n\n$ git remote -v\n\norigin https://github.com/git/git.git (fetch)\n\norigin https://github.com/git/git.git (push)\n\n\n\n$ git branch -v\n\n* master fe0a9eaf3 Merge branch 'svn/authors-prog-2' of\ngit://bogomips.org/git-svn\n\n\n\nProblem:\n\n\n\n$ /tmp/git/git-p4.py clone //depot//@all . –verbose\n\n.\n\n.\n\n.\n\nTraceback (most recent call last):\n\n File \"/tmp/git/git-p4.py\", line 3840, in \n\n main()\n\n File \"/tmp/git/git-p4.py\", line 3834, in main\n\n if not cmd.run(args):\n\n File \"/tmp/git/git-p4.py\", line 3706, in run\n\n if not P4Sync.run(self, depotPaths):\n\n File \"/tmp/git/git-p4.py\", line 3568, in run\n\n self.importChanges(changes)\n\n File \"/tmp/git/git-p4.py\", line 3240, in importChanges\n\n self.initialParent)\n\n File \"/tmp/git/git-p4.py\", line 2858, in commit\n\n self.streamP4Files(files)\n\n File \"/tmp/git/git-p4.py\", line 2750, in streamP4Files\n\n cb=streamP4FilesCbSelf)\n\n File \"/tmp/git/git-p4.py\", line 552, in p4CmdList\n\n cb(entry)\n\n File \"/tmp/git/git-p4.py\", line 2744, in streamP4FilesCbSelf\n\n self.streamP4FilesCb(entry)\n\n File \"/tmp/git/git-p4.py\", line 2692, in streamP4FilesCb\n\n self.streamOneP4File(self.stream_file, self.stream_contents)\n\n File \"/tmp/git/git-p4.py\", line 2569, in streamOneP4File\n\n size = int(self.stream_file['fileSize'])\n\nKeyError: 'fileSize'\n\n\n\nSignature of the sizes output resulting in this problem:\n\n$ p4 -p sizes //foo.c\n\n//foo.c#5 bytes\n\n\n\n$ p4 -p -G sizes //foo.c\n\n{scodesstats depotFiles4//fooc.c50\n\n\n\nSignature for a file without problem:\n\n\n\n$ p4 -p sizes //bar.c\n\n//bar.c#5 1105 bytes\n\n\n\n$ p4 -p -G sizes //bar.c\n\n{scodesstats depotFiles;//bar.csrevs5fileSizes11050\n\n\n\nPatch:\n\n$ git diff\n\ndiff --git a/git-p4.py b/git-p4.py\n\nindex 7bb9cadc6..f908e805e 100755\n\n--- a/git-p4.py\n\n+++ b/git-p4.py\n\n@@ -2565,7 +2565,7 @@ class P4Sync(Command, P4UserMap):\n\n def streamOneP4File(self, file, contents):\n\n relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)\n\n relPath = self.encodeWithUTF8(relPath)\n\n- if verbose:\n\n+ if verbose and 'fileSize' in self.stream_file:\n\n size = int(self.stream_file['fileSize'])\n\n sys.stdout.write('\\r%s --> %s (%i MB)\\n' %\n(file['depotFile'], relPath, size/1024/1024))\n\n sys.stdout.flush()\n\n\n\nThanks & Regards\n\nThandesha\n"},{"id":"344871","messageId":"cover.1523981210.git.amazo@checkvideo.com","threadId":"48306","inReplyTo":"CAJJpmi-pLb4Qcka5aLKXA8B1VOZFFF+OAQ0fgUq9YviobRpYGg@mail.gmail.com","subject":"Re: [BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key","fromName":"Andrey Mazo","fromEmail":"amazo@checkvideo.com","sentAt":"2018-04-17T16:22:12Z","receivedAt":"2018-04-17T16:22:31Z","isPatch":false,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Huh, I actually have a slightly different fix for the same issue.\nIt doesn't suppress the corresponding verbose output completely, but just removes the size information from it.\n\nAlso, I'd mention that the workaround is trivial -- simply omit the \"--verbose\" option.\n\nAndrey Mazo (1):\n git-p4: fix `sync --verbose` traceback due to 'fileSize'\n\n git-p4.py | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\n\nbase-commit: 468165c1d8a442994a825f3684528361727cd8c0\n-- \n2.16.1\n\n"},{"id":"344872","messageId":"2e2b2add4e4fffa4228b8ab9f6cd47fa9bf25207.1523981210.git.amazo@checkvideo.com","threadId":"48306","inReplyTo":"cover.1523981210.git.amazo@checkvideo.com","subject":"[PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'","fromName":"Andrey Mazo","fromEmail":"amazo@checkvideo.com","sentAt":"2018-04-17T16:22:13Z","receivedAt":"2018-04-17T16:23:08Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Perforce server 2007.2 (and maybe others) doesn't return \"fileSize\"\nattribute in its reply to `p4 -G print` command.\nThis causes the following traceback when running `git p4 sync --verbose`:\n\"\"\"\n Traceback (most recent call last):\n File \"/usr/libexec/git-core/git-p4\", line 3839, in \n\tmain()\n File \"/usr/libexec/git-core/git-p4\", line 3833, in main\n\tif not cmd.run(args):\n File \"/usr/libexec/git-core/git-p4\", line 3567, in run\n\tself.importChanges(changes)\n File \"/usr/libexec/git-core/git-p4\", line 3233, in importChanges\n\tself.commit(description, filesForCommit, branch, parent)\n File \"/usr/libexec/git-core/git-p4\", line 2855, in commit\n\tself.streamP4Files(files)\n File \"/usr/libexec/git-core/git-p4\", line 2747, in streamP4Files\n\tcb=streamP4FilesCbSelf)\n File \"/usr/libexec/git-core/git-p4\", line 552, in p4CmdList\n\tcb(entry)\n File \"/usr/libexec/git-core/git-p4\", line 2741, in streamP4FilesCbSelf\n\tself.streamP4FilesCb(entry)\n File \"/usr/libexec/git-core/git-p4\", line 2689, in streamP4FilesCb\n\tself.streamOneP4File(self.stream_file, self.stream_contents)\n File \"/usr/libexec/git-core/git-p4\", line 2566, in streamOneP4File\n\tsize = int(self.stream_file['fileSize'])\n KeyError: 'fileSize'\n\"\"\"\n\nFix this by omitting the file size information from the verbose print out.\nAlso, don't use \"self.stream_file\" directly,\nbut rather use passed in \"file\" argument.\n(which point to the same \"self.stream_file\" for all existing callers)\n\nSigned-off-by: Andrey Mazo \n---\n git-p4.py | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 7bb9cadc6..6f05f915a 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2566,8 +2566,12 @@ class P4Sync(Command, P4UserMap):\n relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)\n relPath = self.encodeWithUTF8(relPath)\n if verbose:\n- size = int(self.stream_file['fileSize'])\n- sys.stdout.write('\\r%s --> %s (%i MB)\\n' % (file['depotFile'], relPath, size/1024/1024))\n+ size = file.get('fileSize', None)\n+ if size is None:\n+ sizeStr = ''\n+ else:\n+ sizeStr = ' (%i MB)' % (int(size)/1024/1024)\n+ sys.stdout.write('\\r%s --> %s%s\\n' % (file['depotFile'], relPath, sizeStr))\n sys.stdout.flush()\n \n (type_base, type_mods) = split_p4_type(file[\"type\"])\n-- \n2.16.1\n\n"},{"id":"344893","messageId":"BYAPR08MB3845A1B2FF344CCA8D14A2F7DAB70@BYAPR08MB3845.namprd08.prod.outlook.com","threadId":"48306","inReplyTo":"CAJJpmi-pLb4Qcka5aLKXA8B1VOZFFF+OAQ0fgUq9YviobRpYGg@mail.gmail.com","subject":"Re: [BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2018-04-17T16:00:12Z","receivedAt":"2018-04-17T17:13:04Z","isPatch":false,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Huh, I actually have a slightly different fix for the same issue.\nIt doesn't suppress the corresponding verbose output completely, but just removes the size information from it.\nI'll (try to) post it as a reply to this email.\n\nAlso, I'd mention that the workaround is trivial -- simply omit the \"--verbose\" option.\n\nThank you,\nAndrey"},{"id":"344894","messageId":"CAJJpmi9OQicqEonVwWMo+yimU5MBdJ9gwzbtY1GXSMB+E69AGA@mail.gmail.com","threadId":"48306","inReplyTo":"cover.1523981210.git.amazo@checkvideo.com","subject":"Re: [BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key","fromName":"Thandesha VK","fromEmail":"thanvk@gmail.com","sentAt":"2018-04-17T17:21:02Z","receivedAt":"2018-04-17T17:22:02Z","isPatch":false,"sender":{"key":"thanvk@gmail.com","avatar":null},"body":"*I* think keeping the filesize info is better with --verbose option as\nthat gives some clue about the file we are working on. What do you\nthink?\nScript has similar checks of key existence at other places where it is\nlooking for fileSize.\n\nOn Tue, Apr 17, 2018 at 9:22 AM, Andrey Mazo wrote:\n> Huh, I actually have a slightly different fix for the same issue.\n> It doesn't suppress the corresponding verbose output completely, but just removes the size information from it.\n>\n> Also, I'd mention that the workaround is trivial -- simply omit the \"--verbose\" option.\n>\n> Andrey Mazo (1):\n> git-p4: fix `sync --verbose` traceback due to 'fileSize'\n>\n> git-p4.py | 8 ++++++--\n> 1 file changed, 6 insertions(+), 2 deletions(-)\n>\n>\n> base-commit: 468165c1d8a442994a825f3684528361727cd8c0\n> --\n> 2.16.1\n>\n\n\n\n-- \nThanks & Regards\nThandesha VK | Cellphone +1 (703) 459-5386\n"},{"id":"344897","messageId":"BYAPR08MB384591845049E50D98A42303DAB70@BYAPR08MB3845.namprd08.prod.outlook.com","threadId":"48306","inReplyTo":"CAJJpmi9OQicqEonVwWMo+yimU5MBdJ9gwzbtY1GXSMB+E69AGA@mail.gmail.com","subject":"Re: [BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2018-04-17T17:33:26Z","receivedAt":"2018-04-17T17:33:36Z","isPatch":false,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Sure, I totally agree.\nSorry, I just wasn't clear enough in my previous email.\nI meant that your patch suppresses \"%s --> %s (%i MB)\" line in case \"fileSize\" is not available,\nwhile my patch suppresses just \"(%i MB)\" portion if the \"fileSize\" is not known.\nIn other words,\n * if \"fileSize\" is known:\n ** both yours and mine patches don't change existing behavior;\n * if \"fileSize\" is not known:\n ** your patch makes streamOneP4File() not print anything;\n ** my patch makes streamOneP4File() print \"%s --> %s\".\n\nHope, I'm clearer this time.\n \nThank you,\nAndrey\n\nFrom: Thandesha VK \n> *I* think keeping the filesize info is better with --verbose option as\n> that gives some clue about the file we are working on. What do you\n> think?\n> Script has similar checks of key existence at other places where it is\n> looking for fileSize.\n> \n> On Tue, Apr 17, 2018 at 9:22 AM, Andrey Mazo wrote:\n>> Huh, I actually have a slightly different fix for the same issue.\n>> It doesn't suppress the corresponding verbose output completely, but just removes the size information from it.\n>>\n>> Also, I'd mention that the workaround is trivial -- simply omit the \"--verbose\" option.\n>>\n>> Andrey Mazo (1):\n>>   git-p4: fix `sync --verbose` traceback due to 'fileSize'\n>>\n>>  git-p4.py | 8 ++++++--\n>>  1 file changed, 6 insertions(+), 2 deletions(-)\n>>\n>>\n>> base-commit: 468165c1d8a442994a825f3684528361727cd8c0\n>> --\n>> 2.16.1\n>>\n> \n> -- \n> Thanks & Regards\n> Thandesha VK | Cellphone +1 (703) 459-5386"},{"id":"344901","messageId":"CAJJpmi_Qk-Q3ndiOFiYy5fGsKsJ0mF=nKbSDkdY-NE0DRkZTEg@mail.gmail.com","threadId":"48306","inReplyTo":"BYAPR08MB384591845049E50D98A42303DAB70@BYAPR08MB3845.namprd08.prod.outlook.com","subject":"Re: [BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key","fromName":"Thandesha VK","fromEmail":"thanvk@gmail.com","sentAt":"2018-04-17T18:01:23Z","receivedAt":"2018-04-17T18:02:08Z","isPatch":false,"sender":{"key":"thanvk@gmail.com","avatar":null},"body":"Sounds good. How about an enhanced version of fix from both of us.\nThis will let us know that something is not right with the file but\nwill not bark\n\n$ git diff\ndiff --git a/git-p4.py b/git-p4.py\nindex 7bb9cadc6..df901976f 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -2566,7 +2566,12 @@ class P4Sync(Command, P4UserMap):\n relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)\n relPath = self.encodeWithUTF8(relPath)\n if verbose:\n- size = int(self.stream_file['fileSize'])\n+ if 'fileSize' not in self.stream_file:\n+ print \"WARN: File size from perforce unknown. Please\nverify by p4 sizes %s\" %(file['depotFile'])\n+ size = \"-1\"\n+ else:\n+ size = self.stream_file['fileSize']\n+ size = int(size)\n sys.stdout.write('\\r%s --> %s (%i MB)\\n' %\n(file['depotFile'], relPath, size/1024/1024))\n sys.stdout.flush()\n\n\nOn Tue, Apr 17, 2018 at 10:33 AM, Mazo, Andrey wrote:\n> Sure, I totally agree.\n> Sorry, I just wasn't clear enough in my previous email.\n> I meant that your patch suppresses \"%s --> %s (%i MB)\" line in case \"fileSize\" is not available,\n> while my patch suppresses just \"(%i MB)\" portion if the \"fileSize\" is not known.\n> In other words,\n> * if \"fileSize\" is known:\n> ** both yours and mine patches don't change existing behavior;\n> * if \"fileSize\" is not known:\n> ** your patch makes streamOneP4File() not print anything;\n> ** my patch makes streamOneP4File() print \"%s --> %s\".\n>\n> Hope, I'm clearer this time.\n>\n> Thank you,\n> Andrey\n>\n> From: Thandesha VK \n>> *I* think keeping the filesize info is better with --verbose option as\n>> that gives some clue about the file we are working on. What do you\n>> think?\n>> Script has similar checks of key existence at other places where it is\n>> looking for fileSize.\n>>\n>> On Tue, Apr 17, 2018 at 9:22 AM, Andrey Mazo wrote:\n>>> Huh, I actually have a slightly different fix for the same issue.\n>>> It doesn't suppress the corresponding verbose output completely, but just removes the size information from it.\n>>>\n>>> Also, I'd mention that the workaround is trivial -- simply omit the \"--verbose\" option.\n>>>\n>>> Andrey Mazo (1):\n>>> git-p4: fix `sync --verbose` traceback due to 'fileSize'\n>>>\n>>> git-p4.py | 8 ++++++--\n>>> 1 file changed, 6 insertions(+), 2 deletions(-)\n>>>\n>>>\n>>> base-commit: 468165c1d8a442994a825f3684528361727cd8c0\n>>> --\n>>> 2.16.1\n>>>\n>>\n>> --\n>> Thanks & Regards\n>> Thandesha VK | Cellphone +1 (703) 459-5386\n\n\n\n-- \nThanks & Regards\nThandesha VK | Cellphone +1 (703) 459-5386\n"},{"id":"344922","messageId":"BYAPR08MB3845A66624486DD1D534050EDAB70@BYAPR08MB3845.namprd08.prod.outlook.com","threadId":"48306","inReplyTo":"CAJJpmi_Qk-Q3ndiOFiYy5fGsKsJ0mF=nKbSDkdY-NE0DRkZTEg@mail.gmail.com","subject":"Re: [BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2018-04-17T18:47:45Z","receivedAt":"2018-04-17T18:47:52Z","isPatch":false,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Does a missing \"fileSize\" actually mean that there is something wrong with the file?\nBecause, for me, `p4 -G print` doesn't print \"fileSize\" for _any_ file.\n(which I attribute to our rather ancient (2007.2) Perforce server)\nI'm not an expert in Perforce, so don't know for sure.\n\nHowever, `p4 -G sizes` works fine even with our p4 server.\nShould we then go one step further and use `p4 -G sizes` to obtain the \"fileSize\" when it's not returned by `p4 -G print`?\nOr is it an overkill for a simple verbose print out?\n\nAlso, please, find one comment inline below.\n\nThank you,\nAndrey\n\nFrom: Thandesha VK \n> Sounds good. How about an enhanced version of fix from both of us.\n> This will let us know that something is not right with the file but\n> will not bark\n> \n> $ git diff\n> diff --git a/git-p4.py b/git-p4.py\n> index 7bb9cadc6..df901976f 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -2566,7 +2566,12 @@ class P4Sync(Command, P4UserMap):\n>          relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)\n>          relPath = self.encodeWithUTF8(relPath)\n>          if verbose:\n> -            size = int(self.stream_file['fileSize'])\n> +            if 'fileSize' not in self.stream_file:\n> +               print \"WARN: File size from perforce unknown. Please verify by p4 sizes %s\" %(file['depotFile'])\nFor whatever reason, the code below uses sys.stdout.write() instead of print().\nShould it be used here for consistency as well?\n\n> +               size = \"-1\"\n> +            else:\n> +               size = self.stream_file['fileSize']\n> +            size = int(size)\n>              sys.stdout.write('\\r%s --> %s (%i MB)\\n' %\n> (file['depotFile'], relPath, size/1024/1024))\n>              sys.stdout.flush()\n> \n> \n> On Tue, Apr 17, 2018 at 10:33 AM, Mazo, Andrey wrote:\n>> Sure, I totally agree.\n>> Sorry, I just wasn't clear enough in my previous email.\n>> I meant that your patch suppresses \"%s --> %s (%i MB)\" line in case \"fileSize\" is not available,\n>> while my patch suppresses just \"(%i MB)\" portion if the \"fileSize\" is not known.\n>> In other words,\n>>  * if \"fileSize\" is known:\n>>  ** both yours and mine patches don't change existing behavior;\n>>  * if \"fileSize\" is not known:\n>>  ** your patch makes streamOneP4File() not print anything;\n>>  ** my patch makes streamOneP4File() print \"%s --> %s\".\n>>\n>> Hope, I'm clearer this time.\n>>\n>> Thank you,\n>> Andrey\n>>\n>> From: Thandesha VK \n>>> *I* think keeping the filesize info is better with --verbose option as\n>>> that gives some clue about the file we are working on. What do you\n>>> think?\n>>> Script has similar checks of key existence at other places where it is\n>>> looking for fileSize.\n>>>\n>>> On Tue, Apr 17, 2018 at 9:22 AM, Andrey Mazo wrote:\n>>>> Huh, I actually have a slightly different fix for the same issue.\n>>>> It doesn't suppress the corresponding verbose output completely, but just removes the size information from it.\n>>>>\n>>>> Also, I'd mention that the workaround is trivial -- simply omit the \"--verbose\" option.\n>>>>\n>>>> Andrey Mazo (1):\n>>>>   git-p4: fix `sync --verbose` traceback due to 'fileSize'\n>>>>\n>>>>  git-p4.py | 8 ++++++--\n>>>>  1 file changed, 6 insertions(+), 2 deletions(-)\n>>>>\n>>>>\n>>>> base-commit: 468165c1d8a442994a825f3684528361727cd8c0\n>>>> --\n>>>> 2.16.1\n>>>>\n>>>\n>>> --\n>>> Thanks & Regards\n>>> Thandesha VK | Cellphone +1 (703) 459-5386\n>\n>\n>\n> -- \n> Thanks & Regards\n> Thandesha VK | Cellphone +1 (703) 459-5386"},{"id":"344926","messageId":"CAJJpmi-Uq8vfSOYaH+quMA87=P=66+JMGGvuLByAKQUy-fpDxg@mail.gmail.com","threadId":"48306","inReplyTo":"BYAPR08MB3845A66624486DD1D534050EDAB70@BYAPR08MB3845.namprd08.prod.outlook.com","subject":"Re: [BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key","fromName":"Thandesha VK","fromEmail":"thanvk@gmail.com","sentAt":"2018-04-17T19:12:45Z","receivedAt":"2018-04-17T19:13:31Z","isPatch":false,"sender":{"key":"thanvk@gmail.com","avatar":null},"body":"I have few cases where even p4 -G sizes (or p4 sizes) is not returning\nthe size value even with latest version of p4 (17.2). In that case, we\nhave to regenerate the digest for file save it - It mean something is\nwrong with the file in perforce.\nRegarding, sys.stdout.write v/s print, I see script using both of them\nwithout a common pattern. I can change it to whatever is more\nappropriate.\n\nOn Tue, Apr 17, 2018 at 11:47 AM, Mazo, Andrey wrote:\n> Does a missing \"fileSize\" actually mean that there is something wrong with the file?\n> Because, for me, `p4 -G print` doesn't print \"fileSize\" for _any_ file.\n> (which I attribute to our rather ancient (2007.2) Perforce server)\n> I'm not an expert in Perforce, so don't know for sure.\n>\n> However, `p4 -G sizes` works fine even with our p4 server.\n> Should we then go one step further and use `p4 -G sizes` to obtain the \"fileSize\" when it's not returned by `p4 -G print`?\n> Or is it an overkill for a simple verbose print out?\n>\n> Also, please, find one comment inline below.\n>\n> Thank you,\n> Andrey\n>\n> From: Thandesha VK \n>> Sounds good. How about an enhanced version of fix from both of us.\n>> This will let us know that something is not right with the file but\n>> will not bark\n>>\n>> $ git diff\n>> diff --git a/git-p4.py b/git-p4.py\n>> index 7bb9cadc6..df901976f 100755\n>> --- a/git-p4.py\n>> +++ b/git-p4.py\n>> @@ -2566,7 +2566,12 @@ class P4Sync(Command, P4UserMap):\n>> relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)\n>> relPath = self.encodeWithUTF8(relPath)\n>> if verbose:\n>> - size = int(self.stream_file['fileSize'])\n>> + if 'fileSize' not in self.stream_file:\n>> + print \"WARN: File size from perforce unknown. Please verify by p4 sizes %s\" %(file['depotFile'])\n> For whatever reason, the code below uses sys.stdout.write() instead of print().\n> Should it be used here for consistency as well?\n>\n>> + size = \"-1\"\n>> + else:\n>> + size = self.stream_file['fileSize']\n>> + size = int(size)\n>> sys.stdout.write('\\r%s --> %s (%i MB)\\n' %\n>> (file['depotFile'], relPath, size/1024/1024))\n>> sys.stdout.flush()\n>>\n>>\n>> On Tue, Apr 17, 2018 at 10:33 AM, Mazo, Andrey wrote:\n>>> Sure, I totally agree.\n>>> Sorry, I just wasn't clear enough in my previous email.\n>>> I meant that your patch suppresses \"%s --> %s (%i MB)\" line in case \"fileSize\" is not available,\n>>> while my patch suppresses just \"(%i MB)\" portion if the \"fileSize\" is not known.\n>>> In other words,\n>>> * if \"fileSize\" is known:\n>>> ** both yours and mine patches don't change existing behavior;\n>>> * if \"fileSize\" is not known:\n>>> ** your patch makes streamOneP4File() not print anything;\n>>> ** my patch makes streamOneP4File() print \"%s --> %s\".\n>>>\n>>> Hope, I'm clearer this time.\n>>>\n>>> Thank you,\n>>> Andrey\n>>>\n>>> From: Thandesha VK \n>>>> *I* think keeping the filesize info is better with --verbose option as\n>>>> that gives some clue about the file we are working on. What do you\n>>>> think?\n>>>> Script has similar checks of key existence at other places where it is\n>>>> looking for fileSize.\n>>>>\n>>>> On Tue, Apr 17, 2018 at 9:22 AM, Andrey Mazo wrote:\n>>>>> Huh, I actually have a slightly different fix for the same issue.\n>>>>> It doesn't suppress the corresponding verbose output completely, but just removes the size information from it.\n>>>>>\n>>>>> Also, I'd mention that the workaround is trivial -- simply omit the \"--verbose\" option.\n>>>>>\n>>>>> Andrey Mazo (1):\n>>>>> git-p4: fix `sync --verbose` traceback due to 'fileSize'\n>>>>>\n>>>>> git-p4.py | 8 ++++++--\n>>>>> 1 file changed, 6 insertions(+), 2 deletions(-)\n>>>>>\n>>>>>\n>>>>> base-commit: 468165c1d8a442994a825f3684528361727cd8c0\n>>>>> --\n>>>>> 2.16.1\n>>>>>\n>>>>\n>>>> --\n>>>> Thanks & Regards\n>>>> Thandesha VK | Cellphone +1 (703) 459-5386\n>>\n>>\n>>\n>> --\n>> Thanks & Regards\n>> Thandesha VK | Cellphone +1 (703) 459-5386\n\n\n\n-- \nThanks & Regards\nThandesha VK | Cellphone +1 (703) 459-5386\n"},{"id":"344937","messageId":"BYAPR08MB3845FEE1844EF613C739CB66DAB70@BYAPR08MB3845.namprd08.prod.outlook.com","threadId":"48306","inReplyTo":"CAE5ih7-iQsBxM3Gn4B1Q9WZ2A0=eTHn9nt3a0LVURppOCQsAWA@mail.gmail.com","subject":"Re: [PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2018-04-17T21:18:39Z","receivedAt":"2018-04-17T21:18:50Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Luke,\n\nThank you for reviewing and acking my patch!\nBy the way, did you see Thandesha's proposed patch [1] to print a warning in case of the missing \"fileSize\" attribute?\nShould we go that route instead?\nOr should we try harder to get the size by running `p4 -G sizes`?\n\n[1] https://public-inbox.org/git/CAJJpmi-pLb4Qcka5aLKXA8B1VOZFFF+OAQ0fgUq9YviobRpYGg@mail.gmail.com/t/#m6053d2031020e08edd24ada6c9eb49721ebc4e27\n\nThank you,\nAndrey\n\nFrom: Luke Diamand \n> On Tue, 17 Apr 2018, 09:22 Andrey Mazo, wrote:\n>> Perforce server 2007.2 (and maybe others) doesn't return \"fileSize\"\n>> attribute in its reply to `p4 -G print` command.\n>> This causes the following traceback when running `git p4 sync --verbose`:\n>> \"\"\"\n>>     Traceback (most recent call last):\n>>       File \"/usr/libexec/git-core/git-p4\", line 3839, in \n>>         main()\n>>       File \"/usr/libexec/git-core/git-p4\", line 3833, in main\n>>         if not cmd.run(args):\n>>       File \"/usr/libexec/git-core/git-p4\", line 3567, in run\n>>         self.importChanges(changes)\n>>       File \"/usr/libexec/git-core/git-p4\", line 3233, in importChanges\n>>         self.commit(description, filesForCommit, branch, parent)\n>>       File \"/usr/libexec/git-core/git-p4\", line 2855, in commit\n>>         self.streamP4Files(files)\n>>       File \"/usr/libexec/git-core/git-p4\", line 2747, in streamP4Files\n>>         cb=streamP4FilesCbSelf)\n>>       File \"/usr/libexec/git-core/git-p4\", line 552, in p4CmdList\n>>         cb(entry)\n>>       File \"/usr/libexec/git-core/git-p4\", line 2741, in streamP4FilesCbSelf\n>>         self.streamP4FilesCb(entry)\n>>       File \"/usr/libexec/git-core/git-p4\", line 2689, in streamP4FilesCb\n>>         self.streamOneP4File(self.stream_file, self.stream_contents)\n>>       File \"/usr/libexec/git-core/git-p4\", line 2566, in streamOneP4File\n>>         size = int(self.stream_file['fileSize'])\n>>     KeyError: 'fileSize'\n>> \"\"\"\n>> \n>> Fix this by omitting the file size information from the verbose print out.\n>> Also, don't use \"self.stream_file\" directly,\n>> but rather use passed in \"file\" argument.\n>> (which point to the same \"self.stream_file\" for all existing callers)\n>> \n>> Signed-off-by: Andrey Mazo \n>> ---\n>>  git -p4.py | 8 ++++++--\n>>  1 file changed, 6 insertions(+), 2 deletions(-)\n>> \n>> diff --git a/git-p4.py b/git-p4.py\n>> index 7bb9cadc6..6f05f915a 100755\n>> --- a/git-p4.py\n>> +++ b/git-p4.py\n>> @@ -2566,8 +2566,12 @@ class P4Sync(Command, P4UserMap):\n>>          relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)\n>>          relPath = self.encodeWithUTF8(relPath)\n>>          if verbose:\n>> -            size = int(self.stream_file['fileSize'])\n>> -            sys.stdout.write('\\r%s --> %s (%i MB)\\n' % (file['depotFile'], relPath, size/1024/1024))\n>> +            size = file.get('fileSize', None)\n>> +            if size is None:\n>> +                sizeStr = ''\n>> +            else:\n>> +                sizeStr = ' (%i MB)' % (int(size)/1024/1024)\n>> +            sys.stdout.write('\\r%s --> %s%s\\n' % (file['depotFile'], relPath, sizeStr))\n>>              sys.stdout.flush()\n>> \n>>          (type_base, type_mods) = split_p4_type(file[\"type\"])\n>> -- \n>> 2.16.1\n>>\n> Thanks, that looks like a good fix to me.  Ack."},{"id":"344938","messageId":"CAJJpmi8hb8iUDaNgrqxSTQAexgUcLQmiyBLx8MsHCa9BN9j43A@mail.gmail.com","threadId":"48306","inReplyTo":"BYAPR08MB3845FEE1844EF613C739CB66DAB70@BYAPR08MB3845.namprd08.prod.outlook.com","subject":"Re: [PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'","fromName":"Thandesha VK","fromEmail":"thanvk@gmail.com","sentAt":"2018-04-17T21:24:13Z","receivedAt":"2018-04-17T21:24:59Z","isPatch":true,"sender":{"key":"thanvk@gmail.com","avatar":null},"body":"My fix is for the case where p4 -G sizes not returning the key and\nvalue for fileSize. This can happen in some cases. Only option at that\npoint of time is to warn the user about the problematic file and keep\nmoving (or should we abort??)\n\nThanks\nThandesha\n\nOn Tue, Apr 17, 2018 at 2:18 PM, Mazo, Andrey wrote:\n> Luke,\n>\n> Thank you for reviewing and acking my patch!\n> By the way, did you see Thandesha's proposed patch [1] to print a warning in case of the missing \"fileSize\" attribute?\n> Should we go that route instead?\n> Or should we try harder to get the size by running `p4 -G sizes`?\n>\n> [1] https://public-inbox.org/git/CAJJpmi-pLb4Qcka5aLKXA8B1VOZFFF+OAQ0fgUq9YviobRpYGg@mail.gmail.com/t/#m6053d2031020e08edd24ada6c9eb49721ebc4e27\n>\n> Thank you,\n> Andrey\n>\n> From: Luke Diamand \n>> On Tue, 17 Apr 2018, 09:22 Andrey Mazo, wrote:\n>>> Perforce server 2007.2 (and maybe others) doesn't return \"fileSize\"\n>>> attribute in its reply to `p4 -G print` command.\n>>> This causes the following traceback when running `git p4 sync --verbose`:\n>>> \"\"\"\n>>> Traceback (most recent call last):\n>>> File \"/usr/libexec/git-core/git-p4\", line 3839, in \n>>> main()\n>>> File \"/usr/libexec/git-core/git-p4\", line 3833, in main\n>>> if not cmd.run(args):\n>>> File \"/usr/libexec/git-core/git-p4\", line 3567, in run\n>>> self.importChanges(changes)\n>>> File \"/usr/libexec/git-core/git-p4\", line 3233, in importChanges\n>>> self.commit(description, filesForCommit, branch, parent)\n>>> File \"/usr/libexec/git-core/git-p4\", line 2855, in commit\n>>> self.streamP4Files(files)\n>>> File \"/usr/libexec/git-core/git-p4\", line 2747, in streamP4Files\n>>> cb=streamP4FilesCbSelf)\n>>> File \"/usr/libexec/git-core/git-p4\", line 552, in p4CmdList\n>>> cb(entry)\n>>> File \"/usr/libexec/git-core/git-p4\", line 2741, in streamP4FilesCbSelf\n>>> self.streamP4FilesCb(entry)\n>>> File \"/usr/libexec/git-core/git-p4\", line 2689, in streamP4FilesCb\n>>> self.streamOneP4File(self.stream_file, self.stream_contents)\n>>> File \"/usr/libexec/git-core/git-p4\", line 2566, in streamOneP4File\n>>> size = int(self.stream_file['fileSize'])\n>>> KeyError: 'fileSize'\n>>> \"\"\"\n>>>\n>>> Fix this by omitting the file size information from the verbose print out.\n>>> Also, don't use \"self.stream_file\" directly,\n>>> but rather use passed in \"file\" argument.\n>>> (which point to the same \"self.stream_file\" for all existing callers)\n>>>\n>>> Signed-off-by: Andrey Mazo \n>>> ---\n>>> git -p4.py | 8 ++++++--\n>>> 1 file changed, 6 insertions(+), 2 deletions(-)\n>>>\n>>> diff --git a/git-p4.py b/git-p4.py\n>>> index 7bb9cadc6..6f05f915a 100755\n>>> --- a/git-p4.py\n>>> +++ b/git-p4.py\n>>> @@ -2566,8 +2566,12 @@ class P4Sync(Command, P4UserMap):\n>>> relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)\n>>> relPath = self.encodeWithUTF8(relPath)\n>>> if verbose:\n>>> - size = int(self.stream_file['fileSize'])\n>>> - sys.stdout.write('\\r%s --> %s (%i MB)\\n' % (file['depotFile'], relPath, size/1024/1024))\n>>> + size = file.get('fileSize', None)\n>>> + if size is None:\n>>> + sizeStr = ''\n>>> + else:\n>>> + sizeStr = ' (%i MB)' % (int(size)/1024/1024)\n>>> + sys.stdout.write('\\r%s --> %s%s\\n' % (file['depotFile'], relPath, sizeStr))\n>>> sys.stdout.flush()\n>>>\n>>> (type_base, type_mods) = split_p4_type(file[\"type\"])\n>>> --\n>>> 2.16.1\n>>>\n>> Thanks, that looks like a good fix to me. Ack.\n\n\n\n-- \nThanks & Regards\nThandesha VK | Cellphone +1 (703) 459-5386\n"},{"id":"344947","messageId":"BYAPR08MB3845AD0BC8F24814E0B9936EDAB70@BYAPR08MB3845.namprd08.prod.outlook.com","threadId":"48306","inReplyTo":"CAJJpmi8hb8iUDaNgrqxSTQAexgUcLQmiyBLx8MsHCa9BN9j43A@mail.gmail.com","subject":"Re: [PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'","fromName":"Mazo, Andrey","fromEmail":"amazo@checkvideo.com","sentAt":"2018-04-17T21:38:44Z","receivedAt":"2018-04-17T21:38:54Z","isPatch":true,"sender":{"key":"amazo@checkvideo.com","avatar":null},"body":"Thandesha,\n\nIf I read your patch correctly, it adds a warning in case `p4 -G print` doesn't return \"fileSize\" (not `p4 sizes`).\nI don't see `p4 sizes` being used by git-p4 at all.\nAs I said earlier, for our ancient Perforce server, `p4 -G print` _never_ returns \"fileSize\".\nSo, it's definitely not a reason to abort.\n\nThank you,\nAndrey\n\nFrom: Thandesha VK \n> My fix is for the case where p4 -G sizes not returning the key and\n> value for fileSize. This can happen in some cases. Only option at that\n> point of time is to warn the user about the problematic file and keep\n> moving (or should we abort??)\n> \n> Thanks\n> Thandesha\n> \n> On Tue, Apr 17, 2018 at 2:18 PM, Mazo, Andrey wrote:\n>> Luke,\n>>\n>> Thank you for reviewing and acking my patch!\n>> By the way, did you see Thandesha's proposed patch [1] to print a warning in case of the missing \"fileSize\" attribute?\n>> Should we go that route instead?\n>> Or should we try harder to get the size by running `p4 -G sizes`?\n>>\n>> [1] https://public-inbox.org/git/CAJJpmi-pLb4Qcka5aLKXA8B1VOZFFF+OAQ0fgUq9YviobRpYGg@mail.gmail.com/t/#m6053d2031020e08edd24ada6c9eb49721ebc4e27\n>>\n>> Thank you,\n>> Andrey\n>>\n>> From: Luke Diamand \n>>> On Tue, 17 Apr 2018, 09:22 Andrey Mazo, wrote:\n>>>>  Perforce server 2007.2 (and maybe others) doesn't return \"fileSize\"\n>>>> attribute in its reply to `p4 -G print` command.\n>>>> This causes the following traceback when running `git p4 sync --verbose`:\n>>>> \"\"\"\n>>>>     Traceback (most recent call last):\n>>>>       File \"/usr/libexec/git-core/git-p4\", line 3839, in \n>>>>         main()\n>>>>       File \"/usr/libexec/git-core/git-p4\", line 3833, in main\n>>>>         if not cmd.run(args):\n>>>>       File \"/usr/libexec/git-core/git-p4\", line 3567, in run\n>>>>         self.importChanges(changes)\n>>>>       File \"/usr/libexec/git-core/git-p4\", line 3233, in importChanges\n>>>>         self.commit(description, filesForCommit, branch, parent)\n>>>>       File \"/usr/libexec/git-core/git-p4\", line 2855, in commit\n>>>>         self.streamP4Files(files)\n>>>>       File \"/usr/libexec/git-core/git-p4\", line 2747, in streamP4Files\n>>>>         cb=streamP4FilesCbSelf)\n>>>>       File \"/usr/libexec/git-core/git-p4\", line 552, in p4CmdList\n>>>>         cb(entry)\n>>>>       File \"/usr/libexec/git-core/git-p4\", line 2741, in streamP4FilesCbSelf\n>>>>         self.streamP4FilesCb(entry)\n>>>>       File \"/usr/libexec/git-core/git-p4\", line 2689, in streamP4FilesCb\n>>>>         self.streamOneP4File(self.stream_file, self.stream_contents)\n>>>>       File \"/usr/libexec/git-core/git-p4\", line 2566, in streamOneP4File\n>>>>         size = int(self.stream_file['fileSize'])\n>>>>     KeyError: 'fileSize'\n>>>> \"\"\"\n>>>>\n>>>> Fix this by omitting the file size information from the verbose print out.\n>>>> Also, don't use \"self.stream_file\" directly,\n>>>> but rather use passed in \"file\" argument.\n>>>> (which point to the same \"self.stream_file\" for all existing callers)\n>>>>\n>>>> Signed-off-by: Andrey Mazo \n>>>> ---\n>>>>  git -p4.py | 8 ++++++--\n>>>>  1 file changed, 6 insertions(+), 2 deletions(-)\n>>>>\n>>>> diff --git a/git-p4.py b/git-p4.py\n>>>> index 7bb9cadc6..6f05f915a 100755\n>>>> --- a/git-p4.py\n>>>> +++ b/git-p4.py\n>>>> @@ -2566,8 +2566,12 @@ class P4Sync(Command, P4UserMap):\n>>>>          relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)\n>>>>          relPath = self.encodeWithUTF8(relPath)\n>>>>          if verbose:\n>>>> -            size = int(self.stream_file['fileSize'])\n>>>> -            sys.stdout.write('\\r%s --> %s (%i MB)\\n' % (file['depotFile'], relPath, size/1024/1024))\n>>>> +            size = file.get('fileSize', None)\n>>>> +            if size is None:\n>>>> +                sizeStr = ''\n>>>> +            else:\n>>>> +                sizeStr = ' (%i MB)' % (int(size)/1024/1024)\n>>>> +            sys.stdout.write('\\r%s --> %s%s\\n' % (file['depotFile'], relPath, sizeStr))\n>>>>              sys.stdout.flush()\n>>>>\n>>>>          (type_base, type_mods) = split_p4_type(file[\"type\"])\n>>>> --\n>>>> 2.16.1\n>>>>\n>>> Thanks, that looks like a good fix to me.  Ack.\n>\n> -- \n> Thanks & Regards\n> Thandesha VK | Cellphone +1 (703) 459-5386\n"},{"id":"344950","messageId":"CAJJpmi9Dpnyont6nBCq6pkZvhKsVW-k6OPRrgzwsx+SpLYxU1A@mail.gmail.com","threadId":"48306","inReplyTo":"BYAPR08MB3845AD0BC8F24814E0B9936EDAB70@BYAPR08MB3845.namprd08.prod.outlook.com","subject":"Re: [PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'","fromName":"Thandesha VK","fromEmail":"thanvk@gmail.com","sentAt":"2018-04-17T22:29:10Z","receivedAt":"2018-04-17T22:29:57Z","isPatch":true,"sender":{"key":"thanvk@gmail.com","avatar":null},"body":"Ah. I didn't realize the script is not using p4 sizes to get the size.\nI assumed that it is using p4 sizes. Now I am looking at it using p4\n-G print.\nHowever, when the stack trace happened, I verified what is wrong and\nfound out that the fileSize key is not returned for \"p4 -G sizes\"\ncommand.\n\n\nSo what I am saying is that we cannot use p4 sizes in this case to\nsolve the problem as even p4 sizes will fail with the same fileSize\nnot found stack trace.\nWe can continue as this is not as Critical. However it is a good idea\nto let user know that they need to take action to correct this file at\nthe perforce side.\n\nSo irrespective of we are using p4 print or p4 sizes, the fileSize is\nnot returned in some cases and warning or aborting is something we\nneed to do.\nJust ignoring may not be a good choice.\n\nOn Tue, Apr 17, 2018 at 2:38 PM, Mazo, Andrey wrote:\n> Thandesha,\n>\n> If I read your patch correctly, it adds a warning in case `p4 -G print` doesn't return \"fileSize\" (not `p4 sizes`).\n> I don't see `p4 sizes` being used by git-p4 at all.\n> As I said earlier, for our ancient Perforce server, `p4 -G print` _never_ returns \"fileSize\".\n> So, it's definitely not a reason to abort.\n>\n> Thank you,\n> Andrey\n>\n> From: Thandesha VK \n>> My fix is for the case where p4 -G sizes not returning the key and\n>> value for fileSize. This can happen in some cases. Only option at that\n>> point of time is to warn the user about the problematic file and keep\n>> moving (or should we abort??)\n>>\n>> Thanks\n>> Thandesha\n>>\n>> On Tue, Apr 17, 2018 at 2:18 PM, Mazo, Andrey wrote:\n>>> Luke,\n>>>\n>>> Thank you for reviewing and acking my patch!\n>>> By the way, did you see Thandesha's proposed patch [1] to print a warning in case of the missing \"fileSize\" attribute?\n>>> Should we go that route instead?\n>>> Or should we try harder to get the size by running `p4 -G sizes`?\n>>>\n>>> [1] https://public-inbox.org/git/CAJJpmi-pLb4Qcka5aLKXA8B1VOZFFF+OAQ0fgUq9YviobRpYGg@mail.gmail.com/t/#m6053d2031020e08edd24ada6c9eb49721ebc4e27\n>>>\n>>> Thank you,\n>>> Andrey\n>>>\n>>> From: Luke Diamand \n>>>> On Tue, 17 Apr 2018, 09:22 Andrey Mazo, wrote:\n>>>>> Perforce server 2007.2 (and maybe others) doesn't return \"fileSize\"\n>>>>> attribute in its reply to `p4 -G print` command.\n>>>>> This causes the following traceback when running `git p4 sync --verbose`:\n>>>>> \"\"\"\n>>>>> Traceback (most recent call last):\n>>>>> File \"/usr/libexec/git-core/git-p4\", line 3839, in \n>>>>> main()\n>>>>> File \"/usr/libexec/git-core/git-p4\", line 3833, in main\n>>>>> if not cmd.run(args):\n>>>>> File \"/usr/libexec/git-core/git-p4\", line 3567, in run\n>>>>> self.importChanges(changes)\n>>>>> File \"/usr/libexec/git-core/git-p4\", line 3233, in importChanges\n>>>>> self.commit(description, filesForCommit, branch, parent)\n>>>>> File \"/usr/libexec/git-core/git-p4\", line 2855, in commit\n>>>>> self.streamP4Files(files)\n>>>>> File \"/usr/libexec/git-core/git-p4\", line 2747, in streamP4Files\n>>>>> cb=streamP4FilesCbSelf)\n>>>>> File \"/usr/libexec/git-core/git-p4\", line 552, in p4CmdList\n>>>>> cb(entry)\n>>>>> File \"/usr/libexec/git-core/git-p4\", line 2741, in streamP4FilesCbSelf\n>>>>> self.streamP4FilesCb(entry)\n>>>>> File \"/usr/libexec/git-core/git-p4\", line 2689, in streamP4FilesCb\n>>>>> self.streamOneP4File(self.stream_file, self.stream_contents)\n>>>>> File \"/usr/libexec/git-core/git-p4\", line 2566, in streamOneP4File\n>>>>> size = int(self.stream_file['fileSize'])\n>>>>> KeyError: 'fileSize'\n>>>>> \"\"\"\n>>>>>\n>>>>> Fix this by omitting the file size information from the verbose print out.\n>>>>> Also, don't use \"self.stream_file\" directly,\n>>>>> but rather use passed in \"file\" argument.\n>>>>> (which point to the same \"self.stream_file\" for all existing callers)\n>>>>>\n>>>>> Signed-off-by: Andrey Mazo \n>>>>> ---\n>>>>> git -p4.py | 8 ++++++--\n>>>>> 1 file changed, 6 insertions(+), 2 deletions(-)\n>>>>>\n>>>>> diff --git a/git-p4.py b/git-p4.py\n>>>>> index 7bb9cadc6..6f05f915a 100755\n>>>>> --- a/git-p4.py\n>>>>> +++ b/git-p4.py\n>>>>> @@ -2566,8 +2566,12 @@ class P4Sync(Command, P4UserMap):\n>>>>> relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)\n>>>>> relPath = self.encodeWithUTF8(relPath)\n>>>>> if verbose:\n>>>>> - size = int(self.stream_file['fileSize'])\n>>>>> - sys.stdout.write('\\r%s --> %s (%i MB)\\n' % (file['depotFile'], relPath, size/1024/1024))\n>>>>> + size = file.get('fileSize', None)\n>>>>> + if size is None:\n>>>>> + sizeStr = ''\n>>>>> + else:\n>>>>> + sizeStr = ' (%i MB)' % (int(size)/1024/1024)\n>>>>> + sys.stdout.write('\\r%s --> %s%s\\n' % (file['depotFile'], relPath, sizeStr))\n>>>>> sys.stdout.flush()\n>>>>>\n>>>>> (type_base, type_mods) = split_p4_type(file[\"type\"])\n>>>>> --\n>>>>> 2.16.1\n>>>>>\n>>>> Thanks, that looks like a good fix to me. Ack.\n>>\n>> --\n>> Thanks & Regards\n>> Thandesha VK | Cellphone +1 (703) 459-5386\n\n\n\n-- \nThanks & Regards\nThandesha VK | Cellphone +1 (703) 459-5386\n"},{"id":"344976","messageId":"CAE5ih79Psc_dVOW54nszxFb+ma4k2bxnnUXJGiCx_qfSrwQitQ@mail.gmail.com","threadId":"48306","inReplyTo":"CAJJpmi-Uq8vfSOYaH+quMA87=P=66+JMGGvuLByAKQUy-fpDxg@mail.gmail.com","subject":"Re: [BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2018-04-18T11:08:19Z","receivedAt":"2018-04-18T11:08:25Z","isPatch":false,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On 17 April 2018 at 20:12, Thandesha VK wrote:\n> I have few cases where even p4 -G sizes (or p4 sizes) is not returning\n> the size value even with latest version of p4 (17.2). In that case, we\n> have to regenerate the digest for file save it - It mean something is\n> wrong with the file in perforce.\n\nJust to be clear - git-p4 does not use the p4 \"sizes\" command anywhere AFAIK.\n\nWe are just talking about the output from \"p4 print\" and the\n\"fileSize\" key, right?\n\nDoes that happen with the 17.2 version of p4?\n\n> Regarding, sys.stdout.write v/s print, I see script using both of them\n> without a common pattern. I can change it to whatever is more\n> appropriate.\n\nprint() probably makes more sense; can we try to use the function form\nso that we don't deliberately make the path to python3 harder (albeit\nin a very tiny way).\n\n>\n> On Tue, Apr 17, 2018 at 11:47 AM, Mazo, Andrey wrote:\n>> Does a missing \"fileSize\" actually mean that there is something wrong with the file?\n>> Because, for me, `p4 -G print` doesn't print \"fileSize\" for _any_ file.\n>> (which I attribute to our rather ancient (2007.2) Perforce server)\n>> I'm not an expert in Perforce, so don't know for sure.\n\nMy 2015 version of p4d reports a fileSize.\n\n>>\n>> However, `p4 -G sizes` works fine even with our p4 server.\n>> Should we then go one step further and use `p4 -G sizes` to obtain the \"fileSize\" when it's not returned by `p4 -G print`?\n>> Or is it an overkill for a simple verbose print out?\n\nIf your server isn't reporting \"fileSize\" then there are a few other\nplaces where I would expect git-p4 to also fail.\n\nIf we're going to support this very ancient version of p4d, then\ngracefully handling a missing fileSize will be useful.\n\n>>\n>> Also, please, find one comment inline below.\n>>\n>> Thank you,\n>> Andrey\n>>\n>> From: Thandesha VK \n>>> Sounds good. How about an enhanced version of fix from both of us.\n>>> This will let us know that something is not right with the file but\n>>> will not bark\n>>>\n>>> $ git diff\n>>> diff --git a/git-p4.py b/git-p4.py\n>>> index 7bb9cadc6..df901976f 100755\n>>> --- a/git-p4.py\n>>> +++ b/git-p4.py\n>>> @@ -2566,7 +2566,12 @@ class P4Sync(Command, P4UserMap):\n>>> relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)\n>>> relPath = self.encodeWithUTF8(relPath)\n>>> if verbose:\n>>> - size = int(self.stream_file['fileSize'])\n>>> + if 'fileSize' not in self.stream_file:\n>>> + print \"WARN: File size from perforce unknown. Please verify by p4 sizes %s\" %(file['depotFile'])\n>> For whatever reason, the code below uses sys.stdout.write() instead of print().\n>> Should it be used here for consistency as well?\n>>\n>>> + size = \"-1\"\n>>> + else:\n>>> + size = self.stream_file['fileSize']\n>>> + size = int(size)\n>>> sys.stdout.write('\\r%s --> %s (%i MB)\\n' %\n>>> (file['depotFile'], relPath, size/1024/1024))\n>>> sys.stdout.flush()\n>>>\n>>>\n>>> On Tue, Apr 17, 2018 at 10:33 AM, Mazo, Andrey wrote:\n>>>> Sure, I totally agree.\n>>>> Sorry, I just wasn't clear enough in my previous email.\n>>>> I meant that your patch suppresses \"%s --> %s (%i MB)\" line in case \"fileSize\" is not available,\n>>>> while my patch suppresses just \"(%i MB)\" portion if the \"fileSize\" is not known.\n>>>> In other words,\n>>>> * if \"fileSize\" is known:\n>>>> ** both yours and mine patches don't change existing behavior;\n>>>> * if \"fileSize\" is not known:\n>>>> ** your patch makes streamOneP4File() not print anything;\n>>>> ** my patch makes streamOneP4File() print \"%s --> %s\".\n>>>>\n>>>> Hope, I'm clearer this time.\n>>>>\n>>>> Thank you,\n>>>> Andrey\n>>>>\n>>>> From: Thandesha VK \n>>>>> *I* think keeping the filesize info is better with --verbose option as\n>>>>> that gives some clue about the file we are working on. What do you\n>>>>> think?\n>>>>> Script has similar checks of key existence at other places where it is\n>>>>> looking for fileSize.\n>>>>>\n>>>>> On Tue, Apr 17, 2018 at 9:22 AM, Andrey Mazo wrote:\n>>>>>> Huh, I actually have a slightly different fix for the same issue.\n>>>>>> It doesn't suppress the corresponding verbose output completely, but just removes the size information from it.\n>>>>>>\n>>>>>> Also, I'd mention that the workaround is trivial -- simply omit the \"--verbose\" option.\n>>>>>>\n>>>>>> Andrey Mazo (1):\n>>>>>> git-p4: fix `sync --verbose` traceback due to 'fileSize'\n>>>>>>\n>>>>>> git-p4.py | 8 ++++++--\n>>>>>> 1 file changed, 6 insertions(+), 2 deletions(-)\n>>>>>>\n\nThanks\nLuke\n"},{"id":"344988","messageId":"CAJJpmi8rxJ506HaxWFYmeJ8g2Z+Lvyuei0i6O1pv1fQ7_wC8Rg@mail.gmail.com","threadId":"48306","inReplyTo":"CAE5ih79Psc_dVOW54nszxFb+ma4k2bxnnUXJGiCx_qfSrwQitQ@mail.gmail.com","subject":"Re: [BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key","fromName":"Thandesha VK","fromEmail":"thanvk@gmail.com","sentAt":"2018-04-18T14:59:13Z","receivedAt":"2018-04-18T14:59:59Z","isPatch":false,"sender":{"key":"thanvk@gmail.com","avatar":null},"body":"Just to be clear - git-p4 does not use the p4 \"sizes\" command anywhere AFAIK.\n\nWe are just talking about the output from \"p4 print\" and the\n\"fileSize\" key, right?\n--> Correct.\n\nDoes that happen with the 17.2 version of p4?\n-->Correct.\n\nprint() probably makes more sense; can we try to use the function form\nso that we don't deliberately make the path to python3 harder (albeit\nin a very tiny way)\n-->Sure.\n\nIf your server isn't reporting \"fileSize\" then there are a few other\nplaces where I would expect git-p4 to also fail.\n-->Most of other places are already doing key check in the hash. Looks\nlike this line was missed out.\n\nOn Wed, Apr 18, 2018 at 4:08 AM, Luke Diamand wrote:\n> On 17 April 2018 at 20:12, Thandesha VK wrote:\n>> I have few cases where even p4 -G sizes (or p4 sizes) is not returning\n>> the size value even with latest version of p4 (17.2). In that case, we\n>> have to regenerate the digest for file save it - It mean something is\n>> wrong with the file in perforce.\n>\n> Just to be clear - git-p4 does not use the p4 \"sizes\" command anywhere AFAIK.\n>\n> We are just talking about the output from \"p4 print\" and the\n> \"fileSize\" key, right?\n>\n> Does that happen with the 17.2 version of p4?\n>\n>> Regarding, sys.stdout.write v/s print, I see script using both of them\n>> without a common pattern. I can change it to whatever is more\n>> appropriate.\n>\n> print() probably makes more sense; can we try to use the function form\n> so that we don't deliberately make the path to python3 harder (albeit\n> in a very tiny way).\n>\n>>\n>> On Tue, Apr 17, 2018 at 11:47 AM, Mazo, Andrey wrote:\n>>> Does a missing \"fileSize\" actually mean that there is something wrong with the file?\n>>> Because, for me, `p4 -G print` doesn't print \"fileSize\" for _any_ file.\n>>> (which I attribute to our rather ancient (2007.2) Perforce server)\n>>> I'm not an expert in Perforce, so don't know for sure.\n>\n> My 2015 version of p4d reports a fileSize.\n>\n>>>\n>>> However, `p4 -G sizes` works fine even with our p4 server.\n>>> Should we then go one step further and use `p4 -G sizes` to obtain the \"fileSize\" when it's not returned by `p4 -G print`?\n>>> Or is it an overkill for a simple verbose print out?\n>\n> If your server isn't reporting \"fileSize\" then there are a few other\n> places where I would expect git-p4 to also fail.\n>\n> If we're going to support this very ancient version of p4d, then\n> gracefully handling a missing fileSize will be useful.\n>\n>>>\n>>> Also, please, find one comment inline below.\n>>>\n>>> Thank you,\n>>> Andrey\n>>>\n>>> From: Thandesha VK \n>>>> Sounds good. How about an enhanced version of fix from both of us.\n>>>> This will let us know that something is not right with the file but\n>>>> will not bark\n>>>>\n>>>> $ git diff\n>>>> diff --git a/git-p4.py b/git-p4.py\n>>>> index 7bb9cadc6..df901976f 100755\n>>>> --- a/git-p4.py\n>>>> +++ b/git-p4.py\n>>>> @@ -2566,7 +2566,12 @@ class P4Sync(Command, P4UserMap):\n>>>> relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)\n>>>> relPath = self.encodeWithUTF8(relPath)\n>>>> if verbose:\n>>>> - size = int(self.stream_file['fileSize'])\n>>>> + if 'fileSize' not in self.stream_file:\n>>>> + print \"WARN: File size from perforce unknown. Please verify by p4 sizes %s\" %(file['depotFile'])\n>>> For whatever reason, the code below uses sys.stdout.write() instead of print().\n>>> Should it be used here for consistency as well?\n>>>\n>>>> + size = \"-1\"\n>>>> + else:\n>>>> + size = self.stream_file['fileSize']\n>>>> + size = int(size)\n>>>> sys.stdout.write('\\r%s --> %s (%i MB)\\n' %\n>>>> (file['depotFile'], relPath, size/1024/1024))\n>>>> sys.stdout.flush()\n>>>>\n>>>>\n>>>> On Tue, Apr 17, 2018 at 10:33 AM, Mazo, Andrey wrote:\n>>>>> Sure, I totally agree.\n>>>>> Sorry, I just wasn't clear enough in my previous email.\n>>>>> I meant that your patch suppresses \"%s --> %s (%i MB)\" line in case \"fileSize\" is not available,\n>>>>> while my patch suppresses just \"(%i MB)\" portion if the \"fileSize\" is not known.\n>>>>> In other words,\n>>>>> * if \"fileSize\" is known:\n>>>>> ** both yours and mine patches don't change existing behavior;\n>>>>> * if \"fileSize\" is not known:\n>>>>> ** your patch makes streamOneP4File() not print anything;\n>>>>> ** my patch makes streamOneP4File() print \"%s --> %s\".\n>>>>>\n>>>>> Hope, I'm clearer this time.\n>>>>>\n>>>>> Thank you,\n>>>>> Andrey\n>>>>>\n>>>>> From: Thandesha VK \n>>>>>> *I* think keeping the filesize info is better with --verbose option as\n>>>>>> that gives some clue about the file we are working on. What do you\n>>>>>> think?\n>>>>>> Script has similar checks of key existence at other places where it is\n>>>>>> looking for fileSize.\n>>>>>>\n>>>>>> On Tue, Apr 17, 2018 at 9:22 AM, Andrey Mazo wrote:\n>>>>>>> Huh, I actually have a slightly different fix for the same issue.\n>>>>>>> It doesn't suppress the corresponding verbose output completely, but just removes the size information from it.\n>>>>>>>\n>>>>>>> Also, I'd mention that the workaround is trivial -- simply omit the \"--verbose\" option.\n>>>>>>>\n>>>>>>> Andrey Mazo (1):\n>>>>>>> git-p4: fix `sync --verbose` traceback due to 'fileSize'\n>>>>>>>\n>>>>>>> git-p4.py | 8 ++++++--\n>>>>>>> 1 file changed, 6 insertions(+), 2 deletions(-)\n>>>>>>>\n>\n> Thanks\n> Luke\n\n\n\n-- \nThanks & Regards\nThandesha VK | Cellphone +1 (703) 459-5386\n"}]}