git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Thandesha VK <thanvk@gmail.com>
To: "Mazo, Andrey" <amazo@checkvideo.com>
Cc: Luke Diamand <luke@diamand.org>,
	George Vanburgh <gvanburgh@bloomberg.net>,
	"larsxschneider@gmail.com" <larsxschneider@gmail.com>,
	"miguel.torroja@gmail.com" <miguel.torroja@gmail.com>,
	Git Users <git@vger.kernel.org>
Subject: Re: [PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize'
Date: Tue, 17 Apr 2018 14:24:13 -0700	[thread overview]
Message-ID: <CAJJpmi8hb8iUDaNgrqxSTQAexgUcLQmiyBLx8MsHCa9BN9j43A@mail.gmail.com> (raw)
In-Reply-To: <BYAPR08MB3845FEE1844EF613C739CB66DAB70@BYAPR08MB3845.namprd08.prod.outlook.com>

My fix is for the case where p4 -G sizes not returning the key and
value for fileSize. This can happen in some cases. Only option at that
point of time is to warn the user about the problematic file and keep
moving (or should we abort??)

Thanks
Thandesha

On Tue, Apr 17, 2018 at 2:18 PM, Mazo, Andrey <amazo@checkvideo.com> wrote:
> Luke,
>
> Thank you for reviewing and acking my patch!
> By the way, did you see Thandesha's proposed patch [1] to print a warning in case of the missing "fileSize" attribute?
> Should we go that route instead?
> Or should we try harder to get the size by running `p4 -G sizes`?
>
> [1] https://public-inbox.org/git/CAJJpmi-pLb4Qcka5aLKXA8B1VOZFFF+OAQ0fgUq9YviobRpYGg@mail.gmail.com/t/#m6053d2031020e08edd24ada6c9eb49721ebc4e27
>
> Thank you,
> Andrey
>
> From: Luke Diamand <luke@diamand.org>
>> On Tue, 17 Apr 2018, 09:22 Andrey Mazo, <amazo@checkvideo.com> wrote:
>>>  Perforce server 2007.2 (and maybe others) doesn't return "fileSize"
>>> attribute in its reply to `p4 -G print` command.
>>> This causes the following traceback when running `git p4 sync --verbose`:
>>> """
>>>     Traceback (most recent call last):
>>>       File "/usr/libexec/git-core/git-p4", line 3839, in <module>
>>>         main()
>>>       File "/usr/libexec/git-core/git-p4", line 3833, in main
>>>         if not cmd.run(args):
>>>       File "/usr/libexec/git-core/git-p4", line 3567, in run
>>>         self.importChanges(changes)
>>>       File "/usr/libexec/git-core/git-p4", line 3233, in importChanges
>>>         self.commit(description, filesForCommit, branch, parent)
>>>       File "/usr/libexec/git-core/git-p4", line 2855, in commit
>>>         self.streamP4Files(files)
>>>       File "/usr/libexec/git-core/git-p4", line 2747, in streamP4Files
>>>         cb=streamP4FilesCbSelf)
>>>       File "/usr/libexec/git-core/git-p4", line 552, in p4CmdList
>>>         cb(entry)
>>>       File "/usr/libexec/git-core/git-p4", line 2741, in streamP4FilesCbSelf
>>>         self.streamP4FilesCb(entry)
>>>       File "/usr/libexec/git-core/git-p4", line 2689, in streamP4FilesCb
>>>         self.streamOneP4File(self.stream_file, self.stream_contents)
>>>       File "/usr/libexec/git-core/git-p4", line 2566, in streamOneP4File
>>>         size = int(self.stream_file['fileSize'])
>>>     KeyError: 'fileSize'
>>> """
>>>
>>> Fix this by omitting the file size information from the verbose print out.
>>> Also, don't use "self.stream_file" directly,
>>> but rather use passed in "file" argument.
>>> (which point to the same "self.stream_file" for all existing callers)
>>>
>>> Signed-off-by: Andrey Mazo <amazo@checkvideo.com>
>>> ---
>>>  git -p4.py | 8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/git-p4.py b/git-p4.py
>>> index 7bb9cadc6..6f05f915a 100755
>>> --- a/git-p4.py
>>> +++ b/git-p4.py
>>> @@ -2566,8 +2566,12 @@ class P4Sync(Command, P4UserMap):
>>>          relPath = self.stripRepoPath(file['depotFile'], self.branchPrefixes)
>>>          relPath = self.encodeWithUTF8(relPath)
>>>          if verbose:
>>> -            size = int(self.stream_file['fileSize'])
>>> -            sys.stdout.write('\r%s --> %s (%i MB)\n' % (file['depotFile'], relPath, size/1024/1024))
>>> +            size = file.get('fileSize', None)
>>> +            if size is None:
>>> +                sizeStr = ''
>>> +            else:
>>> +                sizeStr = ' (%i MB)' % (int(size)/1024/1024)
>>> +            sys.stdout.write('\r%s --> %s%s\n' % (file['depotFile'], relPath, sizeStr))
>>>              sys.stdout.flush()
>>>
>>>          (type_base, type_mods) = split_p4_type(file["type"])
>>> --
>>> 2.16.1
>>>
>> Thanks, that looks like a good fix to me.  Ack.



-- 
Thanks & Regards
Thandesha VK | Cellphone +1 (703) 459-5386

  reply	other threads:[~2018-04-17 21:24 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-16 23:35 [BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key Thandesha VK
2018-04-17 16:00 ` Mazo, Andrey
2018-04-17 16:22 ` Andrey Mazo
2018-04-17 16:22   ` [PATCH 1/1] git-p4: fix `sync --verbose` traceback due to 'fileSize' Andrey Mazo
     [not found]     ` <CAE5ih7-iQsBxM3Gn4B1Q9WZ2A0=eTHn9nt3a0LVURppOCQsAWA@mail.gmail.com>
2018-04-17 21:18       ` Mazo, Andrey
2018-04-17 21:24         ` Thandesha VK [this message]
2018-04-17 21:38           ` Mazo, Andrey
2018-04-17 22:29             ` Thandesha VK
2018-04-17 17:21   ` [BUG] git p4 clone fails when p4 sizes does not return 'fileSize' key Thandesha VK
2018-04-17 17:33     ` Mazo, Andrey
2018-04-17 18:01       ` Thandesha VK
2018-04-17 18:47         ` Mazo, Andrey
2018-04-17 19:12           ` Thandesha VK
2018-04-18 11:08             ` Luke Diamand
2018-04-18 14:59               ` Thandesha VK

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJJpmi8hb8iUDaNgrqxSTQAexgUcLQmiyBLx8MsHCa9BN9j43A@mail.gmail.com \
    --to=thanvk@gmail.com \
    --cc=amazo@checkvideo.com \
    --cc=git@vger.kernel.org \
    --cc=gvanburgh@bloomberg.net \
    --cc=larsxschneider@gmail.com \
    --cc=luke@diamand.org \
    --cc=miguel.torroja@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).