Replace the entire VersioncontrolOperation "subsystem" with VersioncontrolCommit

Marking as critical since this is going to be done before a critical.

Release nodes need to know about branches and tags.

sdboyer and I hashed this out in more detail in IRC. To summarize:

- VersioncontrolBranch and VersioncontrolTag don't currently know anything about datestamps when the branch/tag they represent were created.

- Having tags and branches interleaved in the commit logs would be a major UI win when viewing the VCS activity for a given project. In fact, cvs.module would be doing this already if there were more of me and/or I had more time. We already collect all the data we need (there's a timestamp field in our existing {cvs_tags} table that's accurately maintained), I just never had time to fix the commit log pages to include the tags at the right spots. :( #339054: Store datestamps for tags so that CVS commit listings can interleave tags, too. for the interested reader.

- Although VCAPI has historically suffered from over-abstraction and over-generalization, the idea that every tag/branch has a date when it was created is both universally true across all VCS systems and useful VCAPI-wide. So, punting this into the backends would be a bad idea.

- Sure, in Git every tag just points to a commit, and we already know the date of that commit, so it's duplication to store the datestamp again, but that's really a backend implementation detail that the core API shouldn't have to care about. Different VCSs have different low-level implementation details of how they're stored. However, conceptually, every tag always has (at least) 3 things: 1) a name, 2) a set of files and versions it's pointing to, 3) a time it was created. CVS has 1 stupid way of storing this (writing another line to every single file that the tag touches). Svn has a different stupid way of storing this. Git has a pretty smart way. Bzr has a different way which I don't as much of the details about, but I bet it's relatively close to Git. None of this matters to VCAPI itself. We just need to know the name and the date. I don't think VCAPI itself cares about all the revision info that the tag points to. That's for the VCS itself to care about. We're not trying to build our own repo browser here. But the name and date are useful and important.

- The real problem Sam is getting at with this issue is that currently, due to identity crisis (and perhaps poorly named variables/functions/classes) some of the VCAPI backends are using this whole "operations" thing to actually store the primary data. That's no good. I agree with Sam we want the primary data about commits in a commit-specific place, and branches and tags in their own place, too. The kinds of data we care about for these are different, and it's confusing to try to overly abstract them in this way.

- But, we still need to know when tags/branches were committed, so either we have to add datestamp info to the VersioncontrolBranch and VersioncontrolTag classes, or we need to keep something like the "operation" subsystem. If you're trying to build a view of per-project VCS activity, it's going to be a pain in the neck to have a single view that interleaves commits and branches/tags in any kind of performant way. To prevent DB server killing queries from extremely complicated views (or having two separate views that you try to merge in PHP), you're going to end up needing a denormalized table somewhere.

We all agree that commits live in VersioncontrolCommit, branches in VersioncontrolBranch and tags in VersioncontrolTag.

The question is:

A) Kill VersioncontrolOperation entirely, but ensure that the branch and tag classes/tables store datestamps so we can still do reasonable activity streams.

B) Keep a single table in the core API to record the activity stream, but ensure no one is storing primary data in there and the whole API makes it clear this is basically a denormalized place to just keep the history timestamps in an easy-to-access format.

Ultimately, the denormalized table we're going to want to actually build most of the activity stream views is also going to need a project nid key in it. So that should probably live in vc_project, not vcapi itself. That'd be an argument in favor of (A). Don't complicate the core API with a premature optimization that doesn't actually solve the end problem we're going to face...

Thanks for the great summary!

I just want to add a little comment that IIRC Sam and I are more less in consensus about this:

B.1) lazy loading data from external tables(activity/operation) on the tag class through some getters methods sounds reasonable to me

But, like mentioned, it would have problems with direct views integrations, I mean yeah, we would need to add a join to get the right timestamp from the external table, and I really do not know how much ould cost us in performance.

Tagging git phase 2 and git sprint 8 since it's cited as a pre-requisite for a vc_api release.

Taking this issue. Some additional notes from sdboyer on IRC:

one important thing that's not noted in the issue, and will make a little more sense when you look through more: we actually probably need to retain the VersioncontrolOperation class, and create a VersioncontrolCommit class that extends it, rather than simply replacing VersioncontrolOperation with VersioncontrolCommit... there are still cases where we need to record/represent operation data for branches/tags, and we'll have no way to do it in the entity system if we simply switch VersioncontrolCommit in for VersioncontrolOperation

This patch adds VersioncontrolCommit, VersioncontrolCommitControler, and VersioncontrolCommitUnitTestingTestCase classes. The VersioncontrolOperation* classes are still in, but where things looked relevant I switched them from operation-centric to commit-centric. VcOp's hooks are switched to more generic "operation" hooks (presently not documented in the api file, because I wasn't sure if they are really needed or not) and the "commit" hooks have been moved into the VcCommit class. The drush command is now commit-centric. The submodules haven't been touched.

Please let me know what additional work it needs!

Whoops I remembered to fakeadd the new files, but forgot to do the diff arg to include them in the patch. Below is the corrected patch, including your mod to the controller.

As for the grep of operation - For making my changes I ran `grep -RiI "VersioncontrolOp" /path/to/vcapi` to update the class references and `grep -RiI "[\$'>l]operation" /path/to/vcapi` to search for other operation related references (and generally scanned files for "operation" while editing. Reasons you're seeing operation in there:

• I haven't touched commitlog, commit_restriction, or versioncontrol_fakevcs code (I figured these could wait until the class change was approved).
• VcOp classes & respective tests were retained
• Backend adds 'commit' entity & controller but doesn't remove 'operation'
• Tables: versioncontrol_operation* were not renamed
• There is some batch processing code in there where the term "operation" is unrelated to vcapi

I have made a first pass to the last patch, and it looks fine. Thanks Jonathan!

I am going to do a second pass looking for more places where renaming/using commit class makes sense.

BTW We should not have to forget that we need another issue or at least another commit in git backend after this change got in.