GitPlugin: PyGIT.py Storage history/last_change functions omit revisions if an empty path is specified

[samwwwblack@…]

Our internal company trac+Git instance was incorrectly displaying the branches for a project, where some commits were missing from the browser and revisionlog, but putting in the changeset manually showed the correct information (including children and parent commits). (See trac-git-error_graphs.png)

We found that tracopt.PyGIT.Storage.history() wasn't returning a full set of commits for some revisions, eg

>>> from trac.env import Environment
>>> env = Environment('trac')
>>> repos = env.get_repository()
>>> d = repos.get_node('', '345cff8f7a0c878ee46d834704ac75539dbcebd2')
>>> repos.git.history(d.rev, '')[:5]
['345cff8f7a0c878ee46d834704ac75539dbcebd2', 
 '7a2dd63e541da728a2b4680afce78a5057c8ad1f', 
 'e01068548e1034bceb0f20cdc4e77f7da411bb4e', 
 'f0439ff0e4a2557826796b35a64355306394c58b', 
 '6afff969d8c8baf9eed5582082819e76f6b932ab']
>>> repos.close()

Setting the path to '/' also returned this, as Node instantiation strips '/'.

Running the generated git command in Popen yielded the same results;

>>> git_cmd
['git', '--git-dir=/srv/git/project.git', 'rev-list', '--max-count=-1', '345cff8f7a0c878ee46d834704ac75539dbcebd2', '--', '']
>>> p = Popen(git_cmd, stdout=PIPE, stderr=PIPE, close_fds=True)
>>> stdout, stderr = p.communicate()
>>> stdout.split()[:5]
['345cff8f7a0c878ee46d834704ac75539dbcebd2',
 '7a2dd63e541da728a2b4680afce78a5057c8ad1f', 
 'e01068548e1034bceb0f20cdc4e77f7da411bb4e', 
 'f0439ff0e4a2557826796b35a64355306394c58b', 
 '6afff969d8c8baf9eed5582082819e76f6b932ab']

But running this command in a shell returned the correct commits;

$ git --git-dir=/srv/git/project.git rev-list --max-count=-1 345cff8f7a0c878ee46d834704ac75539dbcebd2 -- | head -5
345cff8f7a0c878ee46d834704ac75539dbcebd2
545a6b4444025a7a05ba50fc5a1b145f1a2353f4
7a2dd63e541da728a2b4680afce78a5057c8ad1f
e01068548e1034bceb0f20cdc4e77f7da411bb4e
f0439ff0e4a2557826796b35a64355306394c58b

Note 545a6b44… is now included.

git rev-list --stdin states;

In addition to the <commit> listed on the command line, 
read them from the standard input. 

If a -- separator is seen, stop reading commits and 
start reading paths to limit the result.

I'm assuming that because the -- is included without a following path, git assumes to look for some STDIN which never arrives; testing this

>>> git_cmd2
['git', '--git-dir=/srv/git/project.git', 'rev-list', '--max-count=-1', '345cff8f7a0c878ee46d834704ac75539dbcebd2']
>>> p = Popen(git_cmd2, stdout=PIPE, stderr=PIPE, close_fds=True)
>>> stdout, stderr = p.communicate()
>>> stdout.split()[:5]
['345cff8f7a0c878ee46d834704ac75539dbcebd2', 
 '545a6b4444025a7a05ba50fc5a1b145f1a2353f4', 
 '7a2dd63e541da728a2b4680afce78a5057c8ad1f', 
 'e01068548e1034bceb0f20cdc4e77f7da411bb4e', 
 'f0439ff0e4a2557826796b35a64355306394c58b']
>>>

545a6b44… is correctly included.

The attached diff edits PyGIT.py history/last_change to check for a path argument, and if none/empty to strip off the -- from the git command.

This seems to return the right commits;

>>> from trac.env import Environment
>>> env = Environment('trac')
>>> repos = env.get_repository()
>>> d = repos.get_node('', '345cff8f7a0c878ee46d834704ac75539dbcebd2')
>>> repos.git.history(d.rev, '')[:5]
['345cff8f7a0c878ee46d834704ac75539dbcebd2', 
 '545a6b4444025a7a05ba50fc5a1b145f1a2353f4', 
 '7a2dd63e541da728a2b4680afce78a5057c8ad1f', 
 'e01068548e1034bceb0f20cdc4e77f7da411bb4e', 
 'f0439ff0e4a2557826796b35a64355306394c58b']
>>> repos.close()

and also fixes the graph (see trac-git-fixed_graphs.png)

System information

Trac	1.0.1
Docutils	0.8.1
Genshi	0.6 (with speedups)
GIT	1.7.9.5
Pygments	1.4
pysqlite	2.6.0
Python	2.7.3 (default, Apr 10 2013, 06:20:15) [GCC 4.6.3]
RPC	1.1.2-r12546
setuptools	0.6
SQLite	3.7.9
Subversion	1.6.17 (r1128011)
jQuery	1.7.2

[samwwwblack@…]

Trac version log incorrect branches graph

[samwwwblack@…]

Trac version log fixed branches graph

[samwwwblack@…]

PyGit.py patch

[Jun Omae]

Good catch! Thanks for your investigation and patch. The patch looks good. I'll push it with unit tests.

I consider that git rev-list -- '' skips the commit if a git commit has no paths.

$ git remote -v | grep mirror
mirror  http://svn.edgewall.org/git/trac/mirror (fetch)
mirror  http://svn.edgewall.org/git/trac/mirror (push)
$ diff -u <(git --git-dir=.git rev-list mirror/trunk) \
          <(git --git-dir=.git rev-list mirror/trunk -- '') | head -10
--- /dev/fd/63  2013-10-15 15:39:51.832787576 +0900
+++ /dev/fd/62  2013-10-15 15:39:51.833787592 +0900
@@ -20,9 +20,7 @@
 f0baa2936a0920f0f92884333770d66ed37fc8d7
 dc053295e3e3032cc833d71fe504b3c6606f811b
 11bd25487692d82d6c62f7b0cf075131091f2891
-f56e18e0506e23d3cd72b245eff2197f7f9cfba3
 e6826e31e818a2130668c5f0858c2da67e2a1cc2
-b8b912bcda3da0788692c8207c185e9a42c30f57
 e19c777535ec1cfe4fe270bbfee938fc109cdf32
$ git --git-dir=.git show --name-status 11bd25487692d82d6c62f7b0cf075131091f2891
commit 11bd25487692d82d6c62f7b0cf075131091f2891
Author: rjollos <rjollos@af82e41b-90c4-0310-8c96-b1721e28e2e2>
Date:   Thu Sep 26 16:20:56 2013 +0000

    1.1.2dev: Merged [12108] from 1.0-stable. Refs #11244.


    git-svn-id: http://trac.edgewall.org/intertrac/log:/trunk@12109 af82e41b-90c4-0310-8c96-b1721e28e2e2

M       trac/tests/functional/testenv.py
$ git --git-dir=.git show --name-status f56e18e0506e23d3cd72b245eff2197f7f9cfba3
commit f56e18e0506e23d3cd72b245eff2197f7f9cfba3
Author: rjollos <rjollos@af82e41b-90c4-0310-8c96-b1721e28e2e2>
Date:   Thu Sep 26 16:14:25 2013 +0000

    1.1.2dev: Record-only merge on [12106]. Refs #11244.


    git-svn-id: http://trac.edgewall.org/intertrac/log:/trunk@12107 af82e41b-90c4-0310-8c96-b1721e28e2e2

[Jun Omae]

The fix can be found in log:jomae.git:ticket11328. I'll push it later.

[samwwwblack@…]

Thanks, looks much cleaner than my patch.

[Jun Omae]

Pushed [12173] and merged into trunk in [12174]. Thanks!