Edgewall Software
Modify

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#11328 closed defect (fixed)

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

Reported by: samwwwblack@… Owned by: samwwwblack@…
Priority: normal Milestone: 1.0.2
Component: plugin/git Version: 1.0.1
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Fix wrongly omitting empty commits of Git repository in log view and broken revision graph.

API Changes:
Internal Changes:

Description

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

Attachments (3)

trac-git-error_graphs.png (47.1 KB ) - added by samwwwblack@… 10 years ago.
Trac version log incorrect branches graph
trac-git-fixed_graphs.png (39.2 KB ) - added by samwwwblack@… 10 years ago.
Trac version log fixed branches graph
trac-revlist-empty_path.patch (1.4 KB ) - added by samwwwblack@… 10 years ago.
PyGit.py patch

Download all attachments as: .zip

Change History (8)

by samwwwblack@…, 10 years ago

Attachment: trac-git-error_graphs.png added

Trac version log incorrect branches graph

by samwwwblack@…, 10 years ago

Attachment: trac-git-fixed_graphs.png added

Trac version log fixed branches graph

by samwwwblack@…, 10 years ago

PyGit.py patch

comment:1 by Jun Omae, 10 years ago

Milestone: 1.0.2
Owner: set to Jun Omae
Status: newassigned

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

comment:2 by Jun Omae, 10 years ago

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

comment:3 by samwwwblack@…, 10 years ago

Thanks, looks much cleaner than my patch.

comment:4 by Jun Omae, 10 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

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

comment:5 by Jun Omae, 10 years ago

Owner: changed from Jun Omae to samwwwblack@…

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain samwwwblack@….
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from samwwwblack@… to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.