Deleting tag via git does not remove the label from {versioncontrol_labels}

Indeed, we saw this during initial verification testing on d.o during the downtime window, but I never managed to create an issue for it. Not sure if the bug is here or in vcapi or vcapi_git, but we'll see. I'll try to find it in the DB tables...

The record of this tag is still in {versioncontrol_labels}:

mysql> SELECT vcl.name, vcl.type FROM versioncontrol_labels vcl INNER JOIN versioncontrol_project_projects vcpp ON vcl.repo_id = vcpp.repo_id WHERE vcpp.nid = 1072922;
+---------------+------+
| name          | type |
+---------------+------+
| 6.x-1.0-beta  |    3 | 
| 6.x-1.0-beta1 |    3 | 
| 6.x-1.x       |    2 | 
| master        |    2 | 
+---------------+------+
4 rows in set (0.00 sec)

Not sure if that makes the bug in VCAPI or in VC Git. Thoughts?

According to mikey_p and sdboyer in IRC, apparently the bug is inside versioncontrol_git_repo_parsing_worker_callback() from versioncontrol_git/versioncontrol_git_repo_manager/versioncontrol_git_repo_manager.module where it's calling $repository->fetchLogs();...

[7:00pm] sdboyer: well the big issue is that we just trigger the full-on, stupid parser logic whenever a push comes in
[7:01pm] dww: ok
[7:01pm] sdboyer: and that logic has no conception of a targeted, known change that's coming in. itjust tries to do a full-on sync
[7:01pm] sdboyer: deletions get complicated in that case
[7:01pm] sdboyer: however
[7:02pm] sdboyer: we do HAVE the data available. so figuring out that we should delete a branch or tag is trivial
[7:02pm] sdboyer: the instructions to do so are just like right there
[7:02pm] sdboyer: and now we get into some of teh code that really needs reworking...
[7:02pm] sdboyer: but
[7:03pm] sdboyer: the callback that gets fired by the worker to respond to an incoming parse job is:
[7:03pm] sdboyer: versioncontrol_git_repo_parsing_worker_callback()
[7:05pm] sdboyer: basically, in the short term we could just add targeted stuff to that callback which checks if there are any ref deletions - and if so, load them and delete them
[7:05pm] dww: sdboyer: so, fetchLogs() writes to the DB and re-populates all the operations and labels or something?
[7:06pm] sdboyer: dww: yeah. think of it as a sync operation - see what all we have in the db, compare to what's in the repo, and update the db to mirror the repo
[7:06pm] dww: right
[7:06pm] sdboyer: the "problem" with deletes is just that the logic was already  horribly inefficient
[7:06pm] sdboyer: and we basically just punted

There's a better system on the way care of halstead, but it's not ready yet. Not sure if it's worth the effort to try to fix the existing code or just put the energy into the new parsing. I lean towards the new.

Same with:
- amazon_store (project #424354) after removing the tag 6.x-1.0-beta4
- amazon (project #3219) after removing the tag 6.x-1.0-rc4

In the contrary, amazon project's empty branch 6.x-2.x disappeared immediately after being removed in git.

This has been actually an old decision:

  // Deleting tags is *not* supported. Read the manual if you want to know why...
  // Check for new tags.
  $tags_new = array_diff_key($tags_in_repo, $tags_in_db_by_name);
  if (!empty($tags_new)) {
    _versioncontrol_git_log_process_tags($repository, $tags_new);
  }

But we can change that code ;-) if we change our approach.

IIRC the point behind this is: if someone have pushed a tag, then anyone can download the repo with that tag, and deleting it and then push it again will result in a problem in the new pull of the person that already have that tag.

Changing status, but please reopen if we want to change the approach.

Ok, I have just confirmed with Sam that hook scripts are taking care of preventing delete a tag associated with a release node, so in fact we can implement the logic for deleting tags :-)

the only case that stays in a non-so-fine state is what I mentioned on #5, but people using git directly should know what to do ;-) so I will provide a patch here.

Ready for review ;-)

Committed after a quick chat with sdboyer.

Naturally I am not really sure when this will hit production on d.o, since I am not really aware about how that will work :-p

Yay, thanks for the fix! Very happy to see this resolved, as it was going to lead to confusion and badness for folks trying to make releases from things that vcapi knew about but Git didn't. ;)

In terms of deployment, here's how it basically works:

A) Code/test/review/commit/push the fix.

B) Tag the issue "needs drupal.org deployment"

C) If it's urgent, find someone with jenkins and bzr access to push changes live (me, sdboyer, drumm, DamZ, etc) and ask us to do it right away. Otherwise, we periodically go through this tag when we're ready to do a deployment and see what all can be merged in and deployed. Frankly, it's better to do smaller deployments that don't change too many things at once so that in case something goes wrong, it's easier to revert and to know what the problem is. So, we try to do this fairly regularly.

Anyway, I'm tagging for now. I'll push this out to d.o soon, I'm just in the middle of reviewing issues and seeing if there's anything else I need to look into first.

Thanks again!
-Derek

p.s. In cases where the tag is already gone from Git but still lingering in VCAPI, the maintainer just has to push *something* to the upstream repo to re-trigger a full parse which should clear out the dead tags, right?

Thanks for the deployment process explanation :-)

p.s. In cases where the tag is already gone from Git but still lingering in VCAPI, the maintainer just has to push *something* to the upstream repo to re-trigger a full parse which should clear out the dead tags, right?

Exactly.

Hi
This also prevents creating a new release when an another tag has incorrect case.
See http://drupal.org/project/user_files
I created tag 6.x-1.0-BETA5 and should have created 6.x-1.0-beta5, which I created soon after - however after deleting the uppercase tag, I still couldn't create a release for the lowercase version.

Pasting IRC chat as per request from dww
10:22:04) larowlan: morning - anyone about that can help with creating a beta release on a project
(10:22:17) larowlan: i've followed the instructions on the module maintainers guide tab of my project
(10:22:27) larowlan: and I can see the tag in the repo browser
(10:22:36) larowlan: but the add release page isn't recognising it
(10:24:11) rocketeerbkw: larowlan: what's your project?
(10:27:55) larowlan: sorry, user_files
(10:28:04) larowlan: http://drupal.org/project/user_files
(10:31:40) dww: larowlan: http://drupal.org/node/add/project-release/1002336
(10:32:53) dww: we'd like to add a listing of tags/branches for your project that aren't valid for releases, ideally with suggestions for what's not valid about that, but that hasn't happened yet...
(10:33:40) larowlan: hi dww, sorry I don't see what I've missed - http://drupalcode.org/project/user_files.git - I've got a BETA5 (old CVS format) and a beta5
(10:35:15) ***dww looks at the DB for a sec...
(10:35:19) rocketeerbkw: tag looks correct to me according to http://drupal.org/node/1015226
(10:36:00) dww: larowlan: congratulations, you've found a bug in the versioncontrol API. ;)
(10:36:15) dww: rather, stumbled into one. ;)
(10:36:28) dww: vcapi thinks this label is called "| 107944 | 17082 | 6.x-1.0-BETA5 | 3 | | "
(10:36:36) larowlan: dww: no worries, not a case issue with mysql is it
(10:36:39) dww: it doesn't seem to have noticed that you renamed it and changed the case to be valid.
(10:37:02) dww: drupalorg_versioncontrol is correctly refusing to let you create a release node like that.
(10:37:17) dww: hence the big warning page at node/add/project-release.
(10:37:30) dww: but yeah, Git itself has the right tag...
(10:37:35) larowlan: I deleted the old one using git tag -d 6.x-1.0-BETA5 and then git push origin :refs/tags/6.x-1.0-BETA5
(10:37:36) dww: and vcapi just doesn't match yet.
(10:37:48) dww: Oh, well, right.
(10:37:57) dww: there's also a bug (already fixed) that prevents VCAPI from deleting
(10:38:00) dww: tags
(10:38:06) dww: which we haven't deployed yet.
(10:38:13) dww: so, probably it's really just the same bug.
(10:38:16) dww: and when you added the new tag...
(10:38:36) dww: mysql case badness prevented vcapi from seeing a new tag.

(10:36:28) dww: vcapi thinks this label is called "| 107944 | 17082 | 6.x-1.0-BETA5 | 3 | | "

Not sure if there is a problem, I do not have access to d.o database, but I cloned the repository mentioned in local and parsed it and end up with this labels in DB:

6.x-1.0-beta5
6.x-1.0-beta4
6.x-1.0-beta1
6.x-1.0-beta2
6.x-1.0-beta3
master

i've just encountered this issue myself. sandbox account.
http://drupalcode.org/sandbox/SKAUGHT/1072620.git/shortlog
change time: Tue, 1 Mar 2011 06:05:42 +0000 (01:05 -0500)

the '7.x-0.76' tag is the error (of course, my fault in it's self...) should've been 6.x-0.76
i had checked, the tag was removed until i pushed everything else up.
now, when i re-enter delete command, my local still says 'tag not found' and then try to push tags, says 'everything up-to-date'

i then pulled project down. re-entered delete command. response was 'Deleted tag '7.x-0.76' (was 761a588)'
then pushed back up, 'Everything up-to-date' yet tag still remains.

This is now live. If you could visit http://drupal.org/node/add/project-release/3281 you'd see that the 6.x-1.0-alpha5 tag which used to be there and which I removed from Git weeks ago is now gone as an option. Yay!

Thanks,
-Derek