Edgewall Software
Modify

Opened 7 years ago

Closed 6 years ago

Last modified 4 years ago

#10594 closed enhancement (fixed)

Move Git plugin to Trac

Reported by: Peter Suter Owned by: Peter Suter
Priority: normal Milestone: 1.0
Component: plugin/git Version:
Severity: normal Keywords: git
Cc: hvr@…, peter@…
Release Notes:

Trac now features built-in support for the ​Git version control system

API Changes:

Description

The Git plugin should be included with Trac and maintained here on trac.edgewall.org.

This means:

References

Mailing list discussion, th:GitPlugin, GitHub

Attachments (0)

Change History (17)

comment:1 Changed 7 years ago by hvr@…

Cc: hvr@… added

comment:2 in reply to:  description Changed 7 years ago by Peter Suter

Component: generalplugin/git

Replying to psuter:

Commit snapshot to SVN

Committed in [10998] and replaced usage of (uncommitted) future27.py in [10999].

Adjust the license file headers

Committed in [11001].

Review and cleanup according to TracDev/CodingStyle

First adjustments committed in [11002].

Create TracGit page

Done, plus GitPlugin redirection page.

Create a ticket component plugin/git

Done.

Review existing tickets and copy relevant ones

Done: #10607, #10606, #10605, #10604, #10603, #10602, #10601, #10600, #10599, #10598, #10597 (Of course if you feel there should be other tickets, please create them.)

Last edited 7 years ago by Peter Suter (previous) (diff)

comment:3 Changed 7 years ago by Peter Suter

In [11003] I convert the PyGIT.py __main__ tests in unit tests and added them to the test suite. (Most of it is more of a performance / memory usage test though. I commented these out for now.)

comment:4 in reply to:  description Changed 7 years ago by Peter Suter

Any suggestions how to best approach this?

I initially assumed we should follow the Mercurial plugin (#10225 / hooks.py). But Mercurial seems to have the advantage of a Python based hook system that can cleanly access configuration settings, while the Git hooks have to be modified to be configured and wrapped in a platform specific shell script. This makes it sound more like the SVN scripts in contrib (trac-svn-hook / trac-svn-post-commit-hook.cmd).

I'm also not really sure which of the attached hook variant(s) would be appropriate to commit here, if any.

comment:5 Changed 7 years ago by Christian Boos

Given that Git on Windows is run either via msysGit or via Cygwin, we only need something like the trac-svn-hook script. Such a script is also the key component if we want to support some kind of "pushlog" (#9205).

comment:6 Changed 7 years ago by peter@…

Cc: peter@… added

I'm taking a stab at rewriting the plugin to use libgit2 and pygit2. I am incredibly happy with the work that hvr and all others have put into the plugin (thank you so much!) but I am also fed up with the rotten performance and broken subprocess management. Let's see how it goes!

comment:7 Changed 7 years ago by peter@…

Meh. pygit2 doesn't wrap git_reference_foreach() yet, which enumerates refs. Down the rabbit hole we go.

comment:8 Changed 7 years ago by peter@…

Not so fast. git_reference_listall() is available. I'll stop spamming now.

comment:9 in reply to:  6 Changed 7 years ago by anonymous

Replying to peter@…:

I'm taking a stab at rewriting the plugin to use libgit2 and pygit2.

good plan; can this be implemented as switchable "git backend", i.e. in such a way, that a dependency on libgit2/pygit2 is optional?

comment:10 Changed 7 years ago by Christian Boos

Before doing such a drastic change of the implementation, could we first discuss where are the problems with the current approach and the performance problems? I don't think using the git executables is necessarily a suboptimal approach, maybe those problems can be fixed. In particular, I fear that we could trade those subprocess issues for memory leaks or even threading issues…

TracDev/Performance/Git (or TracDev/GitBackend, to follow-up on the comment from anonymous above…)

comment:11 in reply to:  10 ; Changed 7 years ago by peter@…

Replying to cboos:

could we first discuss where are the problems with the current approach and the performance problems?

No. I've been trying to do exactly that on #trac, for years!, with zero response. Sorry, too late for that.

I don't think using the git executables is necessarily a suboptimal approach,

Think again. :)

maybe those problems can be fixed.

I disagree violently. git is an application made for humans to interact with git repositories, and sometimes it fails even at that. git is an infinitely worse interface for programs wanting interact with git repositories. And I'm not even starting to talk about the cost of fork and exec now.

In particular, I fear that we could trade those subprocess issues for memory leaks or even threading issues…

libgit2 is thread safe and was written specifically for the purpose of accessing git repositories from programs. It's the only proper way for Trac to do it. Of course libgit2/pygit2 hasn't been around for very many years, so I am absolutely not blaming anyone for not having used it sooner - I just think that the time is now.

Have you run Trac with git repositories much? I have, for a few years now, and I can tell you that it is not a happy story.

IMO further work on PyGIT is mostly waste of time and will likely just be micro-optimizations.

One could compare the situation to httpd running perl for CGI scripts, and perl running some twenty other executables per request, in true unix fashion, to what is perhaps a more modern and certainly more efficient approach of a long-running FastCGI handler.

TracDev/Performance/Git (or TracDev/GitBackend, to follow-up on the comment from anonymous above…)

I don't expect that whatever I come up with will replace the existing plugin overnight (although that would be awesome! :) and I think it's a fine idea to have libgit2/pygit2 somehow be an option, but I will first try to make it work at all, and then we can look at how it should be made pretty.

comment:12 in reply to:  11 Changed 7 years ago by Christian Boos

Replying to peter@…:

Replying to cboos:

could we first discuss where are the problems with the current approach and the performance problems?

No. I've been trying to do exactly that on #trac, for years!, with zero response. Sorry, too late for that.

Well, I'm not sure you'll find many Trac developers on #trac this days. I haven't been there for years either, so that doesn't count. Next time, try the trac-dev MailingList ;-)

I don't think using the git executables is necessarily a suboptimal approach,

Think again. :)

Well, I had the impression that the git internals were written in a fire&forget spirit, not taking so much care about being conservative with memory usage and being used in long running programs. But that may have changed or I could be wrong…

IMO further work on PyGIT is mostly waste of time and will likely just be micro-optimizations.

TracDev/Performance/Git (or TracDev/GitBackend, to follow-up on the comment from anonymous above…)

I don't expect that whatever I come up with will replace the existing plugin overnight (although that would be awesome! :) and I think it's a fine idea to have libgit2/pygit2 somehow be an option, but I will first try to make it work at all, and then we can look at how it should be made pretty.

Sure, that's a good approach, so that we have something to compare with.

But I don't think listing precisely the performance problems and the current bottlenecks would be a waste of time. Probably some of the bottlenecks are not depending on the particular way chosen to access the repository (exe or lib).

comment:13 Changed 6 years ago by Remy Blank

Owner: set to Peter Suter

comment:14 in reply to:  5 ; Changed 6 years ago by Peter Suter

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

Replying to cboos:

we only need something like the trac-svn-hook script.

This is the only thing that's arguably missing here. But I don't really know enough about shell scripting, Git hooks and the Git data model to do this.

So I'm simply closing this ticket without that (non-essential?) script. If someone wants to contribute a script in the next few days please reopen. (After the 1.0 release a new ticket should be created.)

comment:15 in reply to:  14 Changed 6 years ago by Peter Suter

Replying to psuter:

So I'm simply closing this ticket without that (non-essential?) script. If someone wants to contribute a script in the next few days please reopen. (After the 1.0 release a new ticket should be created.)

I opened #10730 to keep track of this separately.

comment:16 Changed 6 years ago by Christian Boos

I had a few clean-ups on the topic of TracDev/CodingStyle, plus a small feature:

  • don't show git-author/git-committer fields if they're redundant with Author and Timestamp (i.e. if they're all showing the same information)
  • refactor the config options to follow the usual style, plus some minor clarifications

See repos:cboos.git:single-git-author.

comment:17 in reply to:  16 Changed 6 years ago by Christian Boos

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Peter Suter.
The resolution will be deleted.
to The owner will be changed from Peter Suter 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.