Redmine

Next time use checkbox to attach the file, dude! ;)

Damn! I've missed that.

BTW, this is just an adaptation of previously posted patch:
http://rubyforge.org/tracker/index.php?func=detail&aid=15389&group_id=1850&atid=7162

If you remove the changes to environment.rb, the patch
applies cleanly to r1162. However I get this exception when I try to view a repository:

Processing RepositoriesController#show (for my.ip.address at 2008-02-18 14:56:11) [GET]
  Session ID: d138520c27cae12c47bd395d851de3db
  Parameters: {"action"=>"show", "id"=>"project", "controller"=>"repositories"}
Rendering template within layouts/base
Rendering repositories/show

ActionView::TemplateError (can't convert Range into Integer) on line #16 of repositories/_revisions.rhtml:
13: <% line_num = 1 %>
14: <% revisions.each do |changeset| %>
15: <tr class="changeset <%= cycle 'odd', 'even' %>">
16: <td class="id"><%= link_to changeset.revision[0..5], :action => 'revision', :id => project, :rev => changeset.revision %></td>
17: <td class="checkbox"><%= radio_button_tag('rev', changeset.revision, (line_num==1), :id => "cb-#{line_num}", :onclick => "$('cbto-#{line_num+1}').checked=true;") if show_diff && (line_num < revisions.size) %></td>
18: <td class="checkbox"><%= radio_button_tag('rev_to', changeset.revision, (line_num==2), :id => "cbto-#{line_num}", :onclick => "if ($('cb-#{line_num}').checked==true) {$('cb-#{line_num-1}').checked=true;}") if show_diff && (line_num > 1) %></td>
19: <td class="committed_on"><%= format_time(changeset.committed_on) %></td>

    app/views/repositories/_revisions.rhtml:16:in `[]'
    app/views/repositories/_revisions.rhtml:16:in `_run_erb_47app47views47repositories47_revisions46rhtml'
    app/views/repositories/_revisions.rhtml:14:in `each'
    app/views/repositories/_revisions.rhtml:14:in `_run_erb_47app47views47repositories47_revisions46rhtml'
    /usr/lib64/ruby/gems/1.8/gems/actionpack-2.0.2/lib/action_view/helpers/capture_helper.rb:142:in `call'
    /usr/lib64/ruby/gems/1.8/gems/actionpack-2.0.2/lib/action_view/helpers/capture_helper.rb:142:in `capture_erb_with_buffer'
    /usr/lib64/ruby/gems/1.8/gems/actionpack-2.0.2/lib/action_view/helpers/capture_helper.rb:44:in `capture'
    /usr/lib64/ruby/gems/1.8/gems/actionpack-2.0.2/lib/action_view/helpers/form_tag_helper.rb:417:in `form_tag_in_block'
    /usr/lib64/ruby/gems/1.8/gems/actionpack-2.0.2/lib/action_view/helpers/form_tag_helper.rb:39:in `form_tag'

When it processed the repository it seems to have created the entries
required for the "Activity" view. All of the "View Differences" links are
broken though, giving the error mesage
Entry and/or revision doesn't exist in the repository.

OK, I didn't notice the database migration script in the patch. After running the migration it works a lot better, although the "View Differences" links on individual files still give the same error. "View Differences" for a complete changeset works OK though.

Is the patch to app/controllers/sys_controller.rb correct?

+  def users
+       Users.fund :all
+  end

Shouldn't that be User.find :all ?

Also, two files contain x.split('<').first, namely app/views/repositories/_revisions.rhtml (<%=h changeset.committer.split('<').first %>) and app/views/repositories/_dir_list_content.rhtml (<%=h(entry.lastrev.author.split('<').first) if entry.lastrev %>).

I'm pretty new to Git but looking at the "," key on the keyboard it seems like that code should be splitting on "," rather than "<". Unless Git really does separate names with <?

Since this patch is to support Git, and since Git is distributed, is anyone actually developing this patch in Git itself? ;)

I did a git svn clone of trunk today and applied the patch, making the User.fund fix. The patch applied cleanly to today's trunk. I'll try to keep it synced with trunk, but no guarantees. Find it here:

git://thewordnerd.info/redmind.git

Need to update my email address on my server's git configs, so the first two commits are a bit broken in that regard, but it should be fine from now on. Has anyone else made this conversion? I think it would be more ideal to maintain these changes in git rather than to continue applying new revisions of this patch. To that end, I'll also try to integrate any patches sent my way into this branch, though I'm a bit crunched for time, so any work you can do ensuring these patches work against trunk would be great.

git://thewordnerd.info/redmine.git

great idea. It will also help me to learn more about git. :)

The fact that it seems to run OK with Users.fund makes me wonder whether that method is required at all.

Do you have any insight into the separator chars?

separator char: I think it's because the username will be "Username <user@example.com>", so splitting on "<" and selecting the first chunk will get you the username without the email address.

@nolan: I have a github account but no invitation. Do you want I create there a copy of your git?

Ooo, good idea. I don't see the advantage of using Github right now because my own hosting lets others clone the repository, but <a href="http://www.gitorious.org">Gitorious</a> offers simple project-hosting and repository cloning for public development and is currently open to everyone.

I've just created <a href="http://www.gitorious.org/projects/redmine-git">Redmine-git</a> and am pushing my repository plus some further patches I was sent this morning. I don't really have time to quality-control or test patches, but I'm happy to poke it from time to time and try to keep it updated with trunk, and I'd be happy to make others committers to the mainline repository, so if you think you'd like to hack on this but don't want to wait for me to merge stuff, or if you'd like to be proactive about merging from others, let me know. I'm just facilitating, don't have the time for another project. :)

Wait! Wait!

What you're talking about is more a fork than just assuring correct integration of git-adapter. There's a huge risk that, if patches not related to git is accepted in that repository, regression will be introduced in other redmine features and people will come crying here, creating a real mess that can be very prejudicial to redmine! Please, accept only patches about the git-adapter for redmine, and precise it in your Gitorious repository! T_T

Uh, what? boggles No offense intended, but please look at the commit logs on the project, which is specifically named redmine-git, before just assuming that I'm forking Redmine. :) John sent me four new patches for the Git integration, and I applied them this morning after glancing over them and ensuring that they didn't appear to do anything malicious or braindead, and specifically made sure they didn't add new features or do anything non git-related. I have zero intention or even time to fork Redmine, I just think that developing Git support in Git itself is preferable to what I did, which was to search all new issues and find the latest available patch. I'm interested in seeing this integrated, I just don't have the time to be responsible for it, so instead I'm doing everything I can to make just these changes available for anyone else to develop or integrate.

Sorry for over-reacting, yesterday wasn't really what I could call a good day :)

The worst is that I saw that the only commits was git-support related. So, hoping this repository will help the git-adapter a way into the redmine trunk :)

FWIW, I have found the git repo now lives at:

git://gitorious.org/redmine-git/mainline.git

Thanks to whomever is maintaining that, and here's my vote to get it integrated into redmine!

Here's a bit of elaboration on this, so hopefully newcomers won't get confused.

This patch, or more appropriately now, series of patches is not by me. I merely take them, glance over them to ensure that they are focused on git support, don't do anything obviously bad then merge them. As of now, git support has three separate contributors, and at least one person has emailed me privately to let me know that he is following the branch and is using it.

Here's the problem. I recently merged with trunk, and there's a new migration sharing the same version number as the one necessary for git. Either those of us using the git fork are going to have to play migration shuffle, bumping the 89 migration up to 90 and breaking upgradability for the rest of you, or we'll have to perform some nice tricks to keep using this support.

I've been emailed privately and asked why this patch isn't being committed. I'm not sure, either. It doesn't seem out of line with existing features, and at least two people are using it in production. Do we need to file an issue linking to this patch? Should I keep attaching updated diffs as someone new submits a patch and I apply it on this branch? Does it need tests? It would be great to have tangible feedback so that the folks from whom patches are trickling in might redirect their efforts in whatever direction is necessary to get this on the mainline and off of a separate fork.

I'm attaching another diff for folks not wanting to mess with downloading from git. This should apply cleanly to trunk as of this morning. sans the double 089 migration which I'm not sure how to resolve. Bumping the git migration to 90 is the obvious resolution, though it would break the deployments currently existing on this fork, which is why I think a fast response would make sense here.

Thanks.

Unfortunately, it looks like a fast response is not forthcoming. I suggest bumping the git migration to 90.

Here's a new git.diff. It makes two changes since the last git.patch that was posted:

1. Removes the changes to config/environment.rb that were apparently accidentally included

2. Renamed 089_make_revisions_string.rb to 090_make_revisions_string.rb since there is another migration at 089

Forgot the attachment.

I guess I should also add:

latest diff applies atop svn r1197

Here is the explanation for why this isn't getting integrated:

http://rubyforge.org/forum/forum.php?thread_id=23059&forum_id=7504

I'm attaching a first go at a much-revised patch.

This one uses the numeric revision identifier as suggested in the link above. It also adds test suites, which the unit tests pass. It also does not interfere with other VCS systems. It has a new parser for the changeset output, which results in the correct number of resulting changesets.

However, there is a design flaw here.

The numeric identifier means nothing to Git. Yet a ton of functions in git_adapter.rb are designed to take a numeric identifier. The database does not appear to be accessible at this level, or at least I don't know how, so it is not possible to use any of these functions that take a numeric identifier, because we have to convert it to a scmid before passing it to Git.

Incidentally, the Darcs backend has the same problem. Lots of things (annotate, cat, etc.) are broken because of this.

Ideas on that one?

OK, it seems sometimes it takes an scmid, and I've found a way to transform a numeric ID into it.

More tests are passing. A few kinks to work out and debug code to remove, but here goes.

Attaching ./test/fixtures/repositories/git_repository.tar.gz separately since diffs can't handle binary data

Here's my best shot at Git support for now.

It passes all its unit tests, and all but one of the other tests. This is better than we can say for Mercurial.

I've played with it a lot in the interface and it seems solid. I will be testing performance against the Linux kernel tree in the future.

Please let me know what you think.

I've created a branch to work on git support: /branches/work/git

I've integrated your patch and finally decided to change the revision field to string. It required some little work to make the other SCM work but it's the right solution.
I've fixed the tests for the other SCM (except Darcs for which there's no tests). I've also added "annotate" support for git.

Let me know if you want a commit access to the repository.

Wonderful!

What do you believe remains to be done before this can be integrated into trunk?

I will proceed to test this as well.

I also have a couple of minor patches here to remove unused variables, and to improve performance of git. I haven't tested this against your branch yet, but on mine it made it fast enough to work with the Linux kernel repository (20,000 commits) in a tolerable amount of time after the initial import.

I'm content to continue working on this on my own git tree, or to push my changes into the branch on svn, whatever you feel is easiest for you. If I do push to svn, I would want to know whether you would prefer to just hack on that branch as-is, or if I should periodically merge trunk into it as well.

The attached patch is meant to be applied on the git branch. It:

• updates the git branch to stay in sync with trunk
• Makes the comments added to an issue use commit:"scmid" instead of rREV in situations where REV is non-numeric and will fail to parse
• Incorporates the changes in above 259-optimize.diff
• Other minor tweaks

If you would like these broken down into smaller discrete diffs, just let me know, I'm happy to generate it that way.

All local changes now checked in to branches/work/git as of r1212

FWIW, the SVN work/git branch has a migrate at 90, and now trunk also does.

I haven't modified the SVN work/git branch to rename its commits.

However, I do maintain, in git, a branch that is consistently updated to apply atop HEAD, and in this branch I have renamed the Git DB migrates to 91 and 92.

For those interested, URL will always fetch a diff against trunk for Git support:

http://git.complete.org/branches/redmine-integration/?a=commitdiff_plain;h=fb-bug-259-git;hp=upstream

Or, people running Git can follow the information at RedmineGitTracking and use the branch fb-bug-259-git or master.

Git branch was merged into trunk in r1236.
Migrations were re-numbered as 91 & 92 to match the migration sequence in trunk.
I close this ticket since Git support is now part of trunk :-)