{"thread":{"id":"57111","subject":"[PATCH v2 0/2] git-p4: improve formatting of numeric values","startedAt":"2021-12-17T20:39:21Z","lastAt":"2021-12-20T20:02:56Z","messageCount":7,"participants":["Joel Holdsworth","Joachim Kuebart","Luke Diamand","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"444387","messageId":"20211217203856.2339161-1-jholdsworth@nvidia.com","threadId":"57111","inReplyTo":null,"subject":"[PATCH v2 0/2] git-p4: improve formatting of numeric values","fromName":"Joel Holdsworth","fromEmail":"jholdsworth@nvidia.com","sentAt":"2021-12-17T20:38:54Z","receivedAt":"2021-12-17T20:39:21Z","isPatch":true,"sender":{"key":"jholdsworth@nvidia.com","avatar":"https://avatars.githubusercontent.com/u/1449493?v=4"},"body":"This patch-set contains patches designed to improvement the way in which\nthe git-p4 script reports files sizes and percentage values in an effort\nto improve the readability of the logged messages.\n\nJoel Holdsworth (2):\n git-p4: print size values in appropriate units\n git-p4: show progress as an integer\n\n git-p4.py | 25 +++++++++++++++++++------\n 1 file changed, 19 insertions(+), 6 deletions(-)\n\n-- \n2.34.1\n\n"},{"id":"444388","messageId":"20211217203856.2339161-2-jholdsworth@nvidia.com","threadId":"57111","inReplyTo":"20211217203856.2339161-1-jholdsworth@nvidia.com","subject":"[PATCH v2 1/2] git-p4: print size values in appropriate units","fromName":"Joel Holdsworth","fromEmail":"jholdsworth@nvidia.com","sentAt":"2021-12-17T20:38:55Z","receivedAt":"2021-12-17T20:39:22Z","isPatch":true,"sender":{"key":"jholdsworth@nvidia.com","avatar":"https://avatars.githubusercontent.com/u/1449493?v=4"},"body":"The git-p4 script reports file sizes in various log messages.\nPreviously, in each case the script would print them as the number of\nbytes divided by 1048576 i.e. the size in mebibytes, rounded down to an\ninteger. This resulted in small files being described as having a size\nof \"0 MB\".\n\nThis patch replaces the existing behaviour with a new helper function:\nformat_size_human_readable, which takes a number of bytes (or any other\nquantity), and computes the appropriate prefix to use: none, Ki, Mi, Gi,\nTi, Pi, Ei, Zi, Yi.\n\nFor example, a size of 123456 will now be printed as \"120.6 KiB\" greatly\nimproving the readability of the log output.\n\nLarge valued prefixes such as pebi, exbi, zebi and yobi are included for\ncompleteness, though they not expected to appear in any real-world\nPerforce repository!\n\nSigned-off-by: Joel Holdsworth \n---\nThis second version of the patch makes the following changes:\n\n * Corrects an issue with the messages being suffixed with an spurious\n \"MB\" text.\n * Makes the units of format_size_human_readable hard-coded to bytes.\n\n git-p4.py | 22 +++++++++++++++++-----\n 1 file changed, 17 insertions(+), 5 deletions(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 2b4500226a..4d8a249b85 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -56,6 +56,16 @@\n \n p4_access_checked = False\n \n+def format_size_human_readable(num):\n+ \"\"\" Returns a number of units (typically bytes) formatted as a human-readable\n+ string.\n+ \"\"\"\n+ for unit in [\"\", \"Ki\", \"Mi\", \"Gi\", \"Ti\", \"Pi\", \"Ei\", \"Zi\"]:\n+ if abs(num) < 1024.0:\n+ return \"{:3.1f} {}B\".format(num, unit)\n+ num /= 1024.0\n+ return \"{:.1f} Yi{}B\".format(num)\n+\n def p4_build_cmd(cmd):\n \"\"\"Build a suitable p4 command line.\n \n@@ -2966,7 +2976,8 @@ def streamOneP4File(self, file, contents):\n size = int(self.stream_file['fileSize'])\n else:\n size = 0 # deleted files don't get a fileSize apparently\n- sys.stdout.write('\\r%s --> %s (%i MB)\\n' % (file_path, relPath, size/1024/1024))\n+ sys.stdout.write('\\r%s --> %s (%s)\\n' % (\n+ file_path, relPath, format_size_human_readable(size)))\n sys.stdout.flush()\n \n (type_base, type_mods) = split_p4_type(file[\"type\"])\n@@ -3064,9 +3075,8 @@ def streamP4FilesCb(self, marshalled):\n if not err and 'fileSize' in self.stream_file:\n required_bytes = int((4 * int(self.stream_file[\"fileSize\"])) - calcDiskFree())\n if required_bytes > 0:\n- err = 'Not enough space left on %s! Free at least %i MB.' % (\n- os.getcwd(), required_bytes/1024/1024\n- )\n+ err = 'Not enough space left on %s! Free at least %s.' % (\n+ os.getcwd(), format_size_human_readable(required_bytes))\n \n if err:\n f = None\n@@ -3110,7 +3120,9 @@ def streamP4FilesCb(self, marshalled):\n size = int(self.stream_file[\"fileSize\"])\n if size > 0:\n progress = 100*self.stream_file['streamContentSize']/size\n- sys.stdout.write('\\r%s %d%% (%i MB)' % (self.stream_file['depotFile'], progress, int(size/1024/1024)))\n+ sys.stdout.write('\\r%s %d%% (%s)' % (\n+ self.stream_file['depotFile'], progress,\n+ format_size_human_readable(required_bytes)))\n sys.stdout.flush()\n \n self.stream_have_file_info = True\n-- \n2.34.1\n\n"},{"id":"444389","messageId":"20211217203856.2339161-3-jholdsworth@nvidia.com","threadId":"57111","inReplyTo":"20211217203856.2339161-1-jholdsworth@nvidia.com","subject":"[PATCH v2 2/2] git-p4: show progress as an integer","fromName":"Joel Holdsworth","fromEmail":"jholdsworth@nvidia.com","sentAt":"2021-12-17T20:38:56Z","receivedAt":"2021-12-17T20:39:24Z","isPatch":true,"sender":{"key":"jholdsworth@nvidia.com","avatar":"https://avatars.githubusercontent.com/u/1449493?v=4"},"body":"When importing files from Perforce, git-p4 periodically logs the\nprogress of file transfers as a percentage. However, the value is\nprinted as a float with an excessive number of decimal places.\n\nFor example a typical update might contain the following message:\n\nImporting revision 12345 (26.199617677553135%)\n\nThis patch simply rounds the value down to the nearest integer\npercentage value, greatly improving readability.\n\nSigned-off-by: Joel Holdsworth \n---\n git-p4.py | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/git-p4.py b/git-p4.py\nindex 4d8a249b85..3c621a6efd 100755\n--- a/git-p4.py\n+++ b/git-p4.py\n@@ -3635,7 +3635,8 @@ def importChanges(self, changes, origin_revision=0):\n self.updateOptionDict(description)\n \n if not self.silent:\n- sys.stdout.write(\"\\rImporting revision %s (%s%%)\" % (change, cnt * 100 / len(changes)))\n+ sys.stdout.write(\"\\rImporting revision %s (%d%%)\" % (\n+ change, (cnt * 100) // len(changes)))\n sys.stdout.flush()\n cnt = cnt + 1\n \n-- \n2.34.1\n\n"},{"id":"444416","messageId":"39F3A9A9-AE61-4C2E-AA66-6B072634D91E@gmail.com","threadId":"57111","inReplyTo":"20211217203856.2339161-2-jholdsworth@nvidia.com","subject":"Re: [PATCH v2 1/2] git-p4: print size values in appropriate units","fromName":"Joachim Kuebart","fromEmail":"joachim.kuebart@gmail.com","sentAt":"2021-12-18T07:44:53Z","receivedAt":"2021-12-18T07:45:00Z","isPatch":true,"sender":{"key":"joachim.kuebart@gmail.com","avatar":"https://avatars.githubusercontent.com/u/24879979?v=4"},"body":"\n\n> On 17 Dec 2021, at 21:38, Joel Holdsworth wrote:\n> \n> git-p4.py | 22 +++++++++++++++++-----\n> 1 file changed, 17 insertions(+), 5 deletions(-)\n> \n> diff --git a/git-p4.py b/git-p4.py\n> index 2b4500226a..4d8a249b85 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -56,6 +56,16 @@\n> \n> p4_access_checked = False\n> \n> +def format_size_human_readable(num):\n> + \"\"\" Returns a number of units (typically bytes) formatted as a human-readable\n> + string.\n> + \"\"\"\n> + for unit in [\"\", \"Ki\", \"Mi\", \"Gi\", \"Ti\", \"Pi\", \"Ei\", \"Zi\"]:\n> + if abs(num) < 1024.0:\n> + return \"{:3.1f} {}B\".format(num, unit)\n> + num /= 1024.0\n> + return \"{:.1f} Yi{}B\".format(num)\n\nThis now has an extra pair of braces.\n\nCheers,\nJoachim\n\n\n"},{"id":"444421","messageId":"CAE5ih7_knaVsxYBkWrJO4+jURM02E98iL9juroahW0k=ZF7Oiw@mail.gmail.com","threadId":"57111","inReplyTo":"39F3A9A9-AE61-4C2E-AA66-6B072634D91E@gmail.com","subject":"Re: [PATCH v2 1/2] git-p4: print size values in appropriate units","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2021-12-18T10:09:34Z","receivedAt":"2021-12-18T10:09:48Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On Sat, 18 Dec 2021 at 07:44, Joachim Kuebart wrote:\n>\n>\n>\n> > On 17 Dec 2021, at 21:38, Joel Holdsworth wrote:\n> >\n> > git-p4.py | 22 +++++++++++++++++-----\n> > 1 file changed, 17 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/git-p4.py b/git-p4.py\n> > index 2b4500226a..4d8a249b85 100755\n> > --- a/git-p4.py\n> > +++ b/git-p4.py\n> > @@ -56,6 +56,16 @@\n> >\n> > p4_access_checked = False\n> >\n> > +def format_size_human_readable(num):\n> > + \"\"\" Returns a number of units (typically bytes) formatted as a human-readable\n> > + string.\n> > + \"\"\"\n> > + for unit in [\"\", \"Ki\", \"Mi\", \"Gi\", \"Ti\", \"Pi\", \"Ei\", \"Zi\"]:\n> > + if abs(num) < 1024.0:\n> > + return \"{:3.1f} {}B\".format(num, unit)\n> > + num /= 1024.0\n> > + return \"{:.1f} Yi{}B\".format(num)\n>\n> This now has an extra pair of braces.\n>\n> Cheers,\n> Joachim\n\nIt also seems to add some slightly spurious floating point precision.\nFor example, 1<<20 comes out as \"1.0 MiB\" and 50 bytes comes out as\n\"50.0 B\".\n\nThere's actually an older python2->python3 conversion bug going on here.\n\nThe code uses:\n\n Size / 1024 / 1024\n\nWith python2 that came out as an integer, which for bytes is what you want.\nFor python3 it comes out as a float - it should have been converted to\nSize // 1024 // 1024.\n\nI guess Joel was trying to preserve that original bug, but really we\nshould just fix it!\n\nOverall I think this would definitely be an improvement, just needs to\ndrop the spurious precision (and fix the return for values beyond ZiB\nas noted by Joachim).\n\nLuke\n"},{"id":"444422","messageId":"CAE5ih7_uq2xos46tH_dR+gietA_5bhr3JzYNJOLmrjhuijy1Ng@mail.gmail.com","threadId":"57111","inReplyTo":"20211217203856.2339161-3-jholdsworth@nvidia.com","subject":"Re: [PATCH v2 2/2] git-p4: show progress as an integer","fromName":"Luke Diamand","fromEmail":"luke@diamand.org","sentAt":"2021-12-18T10:12:11Z","receivedAt":"2021-12-18T10:12:25Z","isPatch":true,"sender":{"key":"luke@diamand.org","avatar":"https://avatars.githubusercontent.com/u/5330967?v=4"},"body":"On Fri, 17 Dec 2021 at 20:39, Joel Holdsworth wrote:\n>\n> When importing files from Perforce, git-p4 periodically logs the\n> progress of file transfers as a percentage. However, the value is\n> printed as a float with an excessive number of decimal places.\n>\n> For example a typical update might contain the following message:\n>\n> Importing revision 12345 (26.199617677553135%)\n>\n> This patch simply rounds the value down to the nearest integer\n> percentage value, greatly improving readability.\n>\n> Signed-off-by: Joel Holdsworth \n> ---\n> git-p4.py | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-p4.py b/git-p4.py\n> index 4d8a249b85..3c621a6efd 100755\n> --- a/git-p4.py\n> +++ b/git-p4.py\n> @@ -3635,7 +3635,8 @@ def importChanges(self, changes, origin_revision=0):\n> self.updateOptionDict(description)\n>\n> if not self.silent:\n> - sys.stdout.write(\"\\rImporting revision %s (%s%%)\" % (change, cnt * 100 / len(changes)))\n> + sys.stdout.write(\"\\rImporting revision %s (%d%%)\" % (\n> + change, (cnt * 100) // len(changes)))\n> sys.stdout.flush()\n> cnt = cnt + 1\n\nThanks for fixing this - this is one of those small python2->python3\nbugs that's still left over.\n\nLooks good to me, ack.\n\nLuke\n"},{"id":"444533","messageId":"xmqqzgovujs2.fsf@gitster.g","threadId":"57111","inReplyTo":"CAE5ih7_uq2xos46tH_dR+gietA_5bhr3JzYNJOLmrjhuijy1Ng@mail.gmail.com","subject":"Re: [PATCH v2 2/2] git-p4: show progress as an integer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-20T20:02:37Z","receivedAt":"2021-12-20T20:02:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Luke Diamand writes:\n\n>> diff --git a/git-p4.py b/git-p4.py\n>> index 4d8a249b85..3c621a6efd 100755\n>> --- a/git-p4.py\n>> +++ b/git-p4.py\n>> @@ -3635,7 +3635,8 @@ def importChanges(self, changes, origin_revision=0):\n>> self.updateOptionDict(description)\n>>\n>> if not self.silent:\n>> - sys.stdout.write(\"\\rImporting revision %s (%s%%)\" % (change, cnt * 100 / len(changes)))\n>> + sys.stdout.write(\"\\rImporting revision %s (%d%%)\" % (\n>> + change, (cnt * 100) // len(changes)))\n>> sys.stdout.flush()\n>> cnt = cnt + 1\n>\n> Thanks for fixing this - this is one of those small python2->python3\n> bugs that's still left over.\n>\n> Looks good to me, ack.\n\nThanks. I'll wait for an updated [1/2] but this is an independently\ngood change that does not depend on it, so I'll take [2/2] alone for\nnow.\n"}]}