Any update on plans to port Version Control API GIT backend to 6.X?

Comments

boombatower’s picture

Status: Active » Postponed

Waiting on versioncontrol port to 6.x.

marvil07’s picture

Status: Postponed » Needs work
Issue tags: +Drupal 6 porting
StatusFileSize
new16.83 KB

ported started :D

This patch only let the module be on 6.x, all api changes are pending

boombatower’s picture

Version: » 5.x-1.2

Patch contains a/ b/ in front of all file indexes. I can bypass that, but then it still doesn't apply.

Please re-roll.

boombatower’s picture

Title: Port to 6.x? » Port to 6.x
Version: 5.x-1.2 »

Branched project for 6.x development.

marvil07’s picture

Version: 5.x-1.2 »
StatusFileSize
new16.83 KB

:o

weird error patching info file(I suppose the problem is automatic changes with CVS :p), now it applies naturally

marvil07’s picture

patch -p1

boombatower’s picture

Not trying to be anal, but I get rej:

@@ -605,7 +603,7 @@
           $revision = $action['git_specific']['revision'];
         }
 
-        $item_revision_id = db_next_id('{versioncontrol_git_item_revisions}_item_revision_id');
+        $item_revision_id = db_last_insert_id('{versioncontrol_git_item_revisions}_item_revision_id');
         db_query(
           "INSERT INTO {versioncontrol_git_item_revisions}
            (item_revision_id, vc_op_id, type, path,
-- 
marvil07’s picture

I do not understand, two possibilities, about using diffent versions:

* of versioncontrol_git

my patch is against DRUPAL-5--1-2 tag

* of patch?

2.5.9

did I am miss anything?

boombatower’s picture

Well, it would be recommended to roll against HEAD as that is for 6.x dev, but DRUPAL-5 and DRUPAL-5--1-2 are all the same...I tried all three with same result.

The second patch posted is virtually identical to the first other than a hash change, not sure if you meant to upload a different file?

marvil07’s picture

I see now branches HEAD = DRUPAL-5, so, I really do not know what is the problem applying the patch.

Here is the diff between patches:

1c1
< From 55bf806ab6a12456d479c3fc6cf7597fa7bcee70 Mon Sep 17 00:00:00 2001
---
> From ba8f8aa9b1ac822a2abadf6808baeadd174b4bb2 Mon Sep 17 00:00:00 2001
40c40
< index be5c5e7..2eaabdd 100644
---
> index be5c5e7..5544fc0 100644
47a48
> +core=6.x
50d50
< +core=6.x

Aparently the patch problem was because where is the core=6.x line inserted, that was the reason of my re-role and the olny real change.

I tried download cvs DRUPAL-5 and HEAD, latest drupal 5 tarball, latest drupal 6 dev, but no problems.. maybe I could commit directly if you want (note that I have plans to contribute here :D with soc or not, I really like git, so maybe is the right time)

boombatower’s picture

After quick discussion in IRC I agree that since I have no time for this now and marvil07 is interest I have given him CVS rights to develop 6.x branch.

Look forward to seeing results.

marvil07’s picture

Assigned: Unassigned » marvil07

ok, initial commit on cvs, looking forward to make the 2nd part on this week, but probably there were more than one commit :D

jpetso’s picture

Great! Some comments on the patch:

  • db_last_insert_id() is not the same as db_next_id() - the latter retrieves a value for the next insert, whereas the former retrieves the value for the insert that happened just before. As you haven't done an insert query at that point yet, the db_last_insert_id() function is going to hand you an incorrect id. The proper way would be to use drupal_write_record() without id property (and with an empty $update array, so that it runs in insert mode) - that function automatically takes care of handling db_last_insert_id(), and fills the passed array or object with the actual new id.
  • The 'access callback' property for the menu entry can be left out if the callback is 'user_access', as that is the default anyways.
  • The "global $user;" line can be removed from hook_menu() too.

Apart from that, I can only refer to the FakeVCS, CVS and SVN modules - if you need any examples, much of the required code exists in an approximately similar way in those other backends. Thanks for starting the port, and good luck!

jpetso’s picture

As for the database stuff, a lot of stuff has changed there. Version Control API's versioncontrol.install tries to document the columns quite thoroughly, you might want to have a look at that one. Also, due to the changes from 5.x-1.x to 5.x-2.x you'll be able to drop the {versioncontrol_git_commit_branches}, {versioncontrol_git_item_revisions}, {versioncontrol_git_item_tags}, {versioncontrol_git_tag_operations} and {versioncontrol_git_item_source_revisions} tables - all of those are now centralized in Version Control API itself. Adapting to those changes will be the bulk of the upgrading work. (Given the low number of users of this backend, a data-preserving upgrade path probably won't be worth the considerable effort. Imho.)

Also, having the password stored in the {versioncontrol_git_accounts} table doesn't make sense in hindsight (see this issue for an explanation), and is not in active use as far as I remember, so you can pretty much delete that table without any side effects.

jpetso’s picture

Oh, and one more thing: I finally got around to putting some documentation into the handbook, you might want to have a look at that.

boombatower’s picture

@jpetso: I decided to give marvil07 commit access, since I don't have time to work on this.

jpetso’s picture

@boombatower: yup, I saw that, and I'm also subscribed to the commit feed so I notice when something happens there :)

marvil07’s picture

start working on this again!

thanks for pointing me to the right direction jpetso

liveoutloud2day’s picture

I'd love to help - I added the missing functions(from 5.1.x to the git 6.x-dev), but that just makes it more like the old version (so that's wrong). jpetson, can you give an example (or where to find an example) of how to make a commit log when you have dropped the {versioncontrol_git_commit_branches}, {versioncontrol_git_item_revisions}, {versioncontrol_git_item_tags}, {versioncontrol_git_tag_operations} and {versioncontrol_git_item_source_revisions} tables? I searched the versioncontrol_fakevcs and there is very little mention of the commit log. What function would be used? marvil07 - any updates I can work from? Thanks!

marvil07’s picture

Assigned: marvil07 » Unassigned

Hi liveoutloud2day,

You're welcome to contribute :-D

I've just commited two little changes(192778 and 192782),

Please make a patch against DRUPAL-6--1 branch.

Maybe you have read it, but there's relative new documentation of the API at its handbook

CorniI’s picture

subscribe, also interested in helping here!

marvil07’s picture

After some talking on irc, CorniI and me decided to make a gitorious clone to work(all history is there). I'll sync CVS time to time :D

@liveoutloud2day, please join us, just make an account there and mail me or post here saying your nick there.

Can you feel git love ;-D?

jpetso’s picture

Ok, let's have some more notes:

  • versioncontrol_git_get_label_id() shouldn't be required - instead, you can create a label array (e.g. array('name' => 'master', 'type' => VERSIONCONTROL_OPERATION_BRANCH)) and pass it to versioncontrol_ensure_branch(), then the return value label array will contain the 'label_id' element.
  • Don't waste time on porting versioncontrol_git_commit() and versioncontrol_git_get_commit_actions() - those functions will be removed anyways after the $commit/$commit_actions arrays have been converted to $operation/$operation_items format. Converting the log parser to generate arrays in that format is probably the best thing you can do in order to get noticable results (= commits that show up in the commit log) in the fastest possible way. Some code in versioncontrol_git_commit() and versioncontrol_git_get_commit_actions() might be useful for that, but the bulk of it is already in Version Control API itself.
  • Functions you can delete right away - because they're now moved to / handled by the API module, and the current functions in the Git backend don't include valuable code - would be _versioncontrol_git_get_item_revision_id(), versioncontrol_git_tag_operation(), versioncontrol_git_get_source_items(), versioncontrol_git_get_commit_actions() and versioncontrol_git_get_item_revision(), versioncontrol_git_get_parent_item(), versioncontrol_git_get_commit_branches(), versioncontrol_git_get_directory_item(), and versioncontrol_git_get_commit_statistics(). I checked those, and you won't need any part of them later. (I also think they're not called from any functions that you didn't uncomment, so the backend should run just the same after the functions have been deleted.)

Anyways, dudes, you rock. go go go!

CorniI’s picture

I pushed an initial commit version to the repo!
It's still very buggy though :P
It's using atm 3 loops to rewrite the parser output, in a further patch i'll modify the parser to ouput better suited arrays.
What do I have to provide for the versioncontrol_insert_operation() $operation_items[]['source_items']-array? atm it's left empty, but i'd like to know what it's for.

About deleting the funcs: i think they could stay atm, in case there's any reference needed, that's why i just commented them out instead of a direct delete.
@others: if you don't want to interfere with my code, someone should implement tags and maybe have a look at my branches, esp. a creation date (and an author) would be *very* nice, has anyone ideas how to get these data?

edit: i was wrong about the parser beeing incomplete for adding/deleting file, it is fine, i just overlooked it :D
edit2: pushed my changes till now, just read the commit message

jpetso’s picture

@Cornil: Like I said above - I specifically checked the functions listed above, and you won't need them as reference later. They're now just bloat. Note that I didn't list all functions that will be removed in the end, because some others actually might be handy as a reference.

jpetso’s picture

Regarding $operation_items format, there is (as always) comprehensive API documentation in the versioncontrol_insert_operation(). Which points to versioncontrol_get_operation_items() for the 'source_items' entries in order to avoid duplication, but essentially the apidox should explain the format better than if I try to recap it here in the comments.

CorniI’s picture

Assigned: Unassigned » CorniI

imho, the port to D6 is pretty complete and working. Code is supposed to be comitted to cvs.d.o by marvil07 soon, else you can find it here:
http://gitorious.org/projects/versioncontrol_git
there's also a wiki page with a roadmap and a bit text of what i've done till now. If boombatower/jpetsp agrees that the port itself is done, please close the issue, we'll then open further issues for the roadmap points :)

jpetso’s picture

Status: Needs work » Fixed

The code looks good, I believe the backend is there from a porting point of view. Some issues might certainly be left to fix, but if it works then I'd say let's mark the porting issue as fixed. Thanks for everyone's input (and especially CorniI's major contributions, without which the backend would still be far from working), I'm looking forward to more polish and a significant number of users.

Once you publish betas or release candidates (your call), please take care to make upgrade paths sufficiently work. For users updating from the 5.x version, note that there is no upgrade path - you have to remove all Git repositories and uninstall the Git backend in your 5.x version, and then set up all repositories again in the 6.x version.

Status: Fixed » Closed (fixed)
Issue tags: -Drupal 6 porting

Automatically closed -- issue fixed for 2 weeks with no activity.