Issue 7004 - Actually return git's result

packager.py

Index: packager.py
diff --git a/packager.py b/packager.py
index ab9557148de8834951ea1ea93e83c55a0be18cf9..d4f7b8fd35848077864fed893314cbe91862eec8 100644
--- a/packager.py
+++ b/packager.py
@@ -72,8 +72,11 @@ def getBuildNum(baseDir):
             result = subprocess.check_output(['hg', 'id', '-R', baseDir, '-n'])
             return re.sub(r'\D', '', result)
         elif Git().istype(baseDir):
-            result = subprocess.check_output(['git', 'rev-list', 'HEAD'], cwd=baseDir)
-            return str(len(result.splitlines()))
+            result = subprocess.check_output(
+                ['git', 'rev-list', '--count', '--branches', '--tags'],
+                cwd=baseDir,
+            )
+            return result.strip()

[Sebastian Noack, 2018/10/08 14:01:04]

Perhaps we can unify the code paths here:

  if Mercurial().istype(baseDir):
      output = subprocess.check_output(['hg', 'id', '-R', baseDir, '-n'])
  else:
      output = subprocess.check_output(
          ['git', 'rev-list', '--count', '--branches', '--tags'],
          cwd=baseDir,
      )
  return re.sub(r'\D', '', output)

1. While strip() would be sufficient for Git it's not for Mercurial (as it might
add a plus character at the end). So if the code path is unified we have to go
back using the regexp for both cases.
2. Explicitly checking for Git is redundant since if it's neither a Mercurial
nor a Git repository CalledProcess Error is raised which is handled below.

[tlucas, 2018/10/08 14:19:42]

I see how this would improve the code, but since we already agreed on eventually
moving the buildtools to Node.js, i'd rather not introduce any cosmetic changes.

What do you think?

[Sebastian Noack, 2018/10/08 14:26:06]

I think the requested change is trivial, and makes this code easier to maintain
for the the time being (and who knows how long that will be in the end). If we'd
be talking about a larger refactoring, I'd agree of course.

But it's probably not worth to argue about it. LGTM, either with or without my
suggested change.

     except subprocess.CalledProcessError:
         pass