Version Control API store operation labels, but we are only storing relations between branches and commits, instead of storing both branches and tags.

Comments

marvil07’s picture

Assigned: marvil07 » sdboyer
Status: Active » Needs review
StatusFileSize
new9.31 KB

@sdboyer: It would be great if you take a look at this patch before I commit it.

BTW the impact here is that now you can see the list of tags the commit is in on commitlog, that already shows all labels associations since #1043506: Use versioncontrol_handler_field_operation_labels views field handler to avoid operation duplicates

sdboyer’s picture

No, I don't think I agree. I understand that Git treats them this way, but I think I'd prefer to continue to treat them as atomic and individual, rather than containing the full history. Not least because of how ridiculous the storage costs would be to store this data.

Is there a particular use case that you feel is deficient that made you want to add this?

marvil07’s picture

Actually the main reason I end up writing this patch is because I think versioncontrol concept of operation label encourage the backend to make associations between labels(as we defined them on vcs api: branches and tags) and operations. I mean, my main motivation is consistency with the requirements of the api.

So, if we really do not want to encourage this maybe it's a good idea to change main api: versioncontrol_operation_labels then should become versioncontrol_operation_branches on vcs api(i.e. rename table and views field).

sdboyer’s picture

I'm not opposed to separating the table (back) into two separate tables. I never really liked the 'labels' abstraction in the first place - another term that is unique to vcapi and clutters the conceptual space. But I don't quite agree that "api consistency" demands that all labels be the same, either. We do already provide a mapping of the tag to an operation, right? It's just that it's an atomic mapping, since we inherently assume that all vcses we work with are atomic (and fake it with CVS), as opposed to a full historical mapping, as with branches.

marvil07’s picture

An old issue I did not find when I created this issue that is a subset of this: #1072224: Add tags to operation label relations

About separating in two table I did not really see the benefit.

We do already provide a mapping of the tag to an operation, right?

Not really, we only store the tag, but we never create a operation tag nor we store the relation on operation labels table.

I would definitely be in favour to store the relation on the operation labels table, it would be really cool to figure out through commitlog which commits where to which branches(i. e. figure out when a commit is released).

I would say to not store an operation tag, or maybe only do it when we have a real tag(annotated/signed).

marvil07’s picture

Title: Store tag/commit relations » Store tag/commit relations to be saved optionally
Assigned: sdboyer » Unassigned
Issue tags: -versioncontrol-6.x-2.0-release-blocker +versioncontrol_git-6.x-2.0-release-blocker

Let's do this optionally to avoid at least triple the rows on versioncontrol_operation_labels table.

marvil07’s picture

Status: Needs review » Needs work
marvil07’s picture

Assigned: Unassigned » marvil07
marvil07’s picture

Title: Store tag/commit relations to be saved optionally » Add an option for reposync plugins to store tag/commit relations optionally
Project: Version Control API -- Git backend » Version Control API
Version: 6.x-2.x-dev » 7.x-1.x-dev
Component: Git interaction » API module
Category: bug » feature
Issue tags: -versioncontrol_git-6.x-2.0-release-blocker +Needs backport to D6, +versioncontrol-6.x-2.0-release-blocker

Reflect current state.

Then, implement it at versioncontrol_git.

marvil07’s picture

Status: Needs work » Needs review
StatusFileSize
new9.5 KB

Let's see what bot thinks.

marvil07’s picture

Status: Needs review » Needs work

Still a little to do, patch later.

marvil07’s picture

Status: Needs work » Needs review
StatusFileSize
new16.02 KB
new10.02 KB
new793 bytes

These patches should be working correctly.

Tests confirm git log parsing is working correctly(VersioncontrolGitDataIntegrityTests and VersioncontrolGitEventDataIntegrityTests passed correctly).

@sdboyer: Pending on your review to add these patches upstream.
Update: Sorry, still pending to teach the option value to the git backend.

marvil07’s picture

Ok, now it's ready for review.

Here some hints about variables for versioncontrol_git git_default reposync plugin options:

// map branch relations: 0x01;
// map tags relations: 0x02;

// To replicate current behavior(only map branches) set the sync plugin options variable accordingly:
$conf['versioncontrol_repository_synchronizer_options_git_default'] = array(
  'operation_labels_mapping' => 0x01,
);

// To only map tags:
$conf['versioncontrol_repository_synchronizer_options_git_default'] = array(
  'operation_labels_mapping' => 0x02,
);

// To map both branches and tags:
$conf['versioncontrol_repository_synchronizer_options_git_default'] = array(
  'operation_labels_mapping' => 0x03,
);

Notice this behavior is not retroactive(will not remove already added branches/tags) to preserve data, but I'm open to suggestions.

marvil07’s picture

Status: Needs review » Needs work

After reviewing this again I added it to 7.x-1.x, the patch for versioncontrol_git still needs some work, so I opened a new issue for that: #1985426: Implement versioncontrol reposync plugin option to store tag/commit relations optionally.

Back to NW for D6 version.

marvil07’s picture

Version: 7.x-1.x-dev » 6.x-2.x-dev
marvil07’s picture

Version: 6.x-2.x-dev » 7.x-1.x-dev
Status: Needs work » Fixed

Since 6.x-2.x entered in maintenance mode, this will not make it there.

marvil07’s picture

Removing unnecessary tags.

Status: Fixed » Closed (fixed)

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

  • Commit abc0d52 on 7.x-1.x, drush-vc-sync-unlock by marvil07:
    Issue #1293956: Added an option for reposync plugins to store tag/commit...
  • Commit 20c146e on 7.x-1.x, drush-vc-sync-unlock by marvil07:
    Issue #1293956 follow-up: Fix repository getSynchronizerOptions method.
    

  • Commit abc0d52 on 7.x-1.x by marvil07:
    Issue #1293956: Added an option for reposync plugins to store tag/commit...
  • Commit 20c146e on 7.x-1.x by marvil07:
    Issue #1293956 follow-up: Fix repository getSynchronizerOptions method.