I'm working on the Git hooks (specifically the 'update' hook right now) and am trying to figure out what sorts of things we would want in a pre-commit (or pre-push, in the case of DVCSs) hook.
What I have now
For now, I have only been checking whether the user attempting to push has an account with the given repository. Combined with the "admin must approve accounts" setting on repositories, this seems good enough to be useful.
Do we want more?
I seem to recall there being restrictions on what branch and tag names were allowed in the CVS repository, since they were globally visible, but with one repository per project (for drupal.org), this would seem to be a non-issue.
Do we still want some sort of restriction on who can commit where, aside from membership with the repository? I can envision something like having certain users able to push to certain branches (this would apply more to DVCSs). This would allow for a style where there is a "master" branch, to which only the project admin has push access, but anyone can create other branches. That is what I'm planning on working on later (the item "Multiple project branches" on my project wiki), but is there anything else we would want?
Comments
Tough one
Yeah, that's a difficult issue, complicated by the fact that DVCS users commit code before the server has any chance to get to see them. So I'm not even sure if we really want to restrict commits, as that would potentially force the developer to rewrite commit history even if current HEAD is ok.
On the other hand, a couple of important checks are done as part of (commit/branch/tag) operation checks. Checking label (branch/tag) operations for correct label names is indispensable for drupal.org as they determine version numbers of project releases. I could imagine that a project's "official repository" on d.o (the maintainer one) allows additional branches like DRUPAL-6--1 but other people's repositories can only upload master. (In addition, we probably want the website to allow whether to have a single "official" repository with branches, or whether to have multiple "official" repositories for a single branch each. But that's mostly stuff for later, probably.)
The other thing that operation checks has to check is whether a person has commit access not only to a given repository but also to a directory inside that repository. Using Git for one project each would lower the need for such detailed access control, but again d.o makes an exception here: translators can commit freely into the translations/ (D6) and po/ (D5) directories of a project, without being listed in the maintainer list of a project.
So all in all, I think we won't get around checking commits like for CVS, and expect the admin to configure the checks in a sensible way so that rewriting history is not necessary except when something's really wrong (such as a translator committing into a module's main directory).
And yeah, I'm the first person who'll claim that d.o's commit restrictions are ridiculously strict and unprecedented especially in distributed version control, it's so paranoid it's a joke. But the masses of unattentive amateur committers on d.o make the enforcement of branch and tag names necessary as the CVS repository would otherwise turn out to be a big mess, and if we want to see Git on d.o we probably won't get around allowing a similar set of restriction capabilities. Just ask dww if you want to know what I mean.
Oh, and speaking of which... please cross-post this discussion in the "Issue tracking and software releases" group so that dww (and other project* people that might be missing from the Revision Control Management group) can chime in too.
Checking label (branch/tag)
Heck, I think I haven't thought enough outside the box here. The main reason why drupal.org CVS forbids branches and tags other than those for official releases (branches: HEAD and DRUPAL-{5,6,7}--$n, tags: DRUPAL-{5,6,7}--$n-[UNSTABLE,ALPHA,BETA,RC]$m) is that tags are global for the whole CVS repository, and especially the branches can't be deleted after they have been created. So the current label restrictions prevent the CVS repository from drowning in branch chaos.
With DVCS though, that should not be a problem at all. On the one hand, branches can be renamed and deleted anyways, and on the other hand it's all local to the respective repository. So I see little reason to deny people the creation of non-release branches. Maybe even in the projects' official repositories. So, that should be less of an issue than I originally thought.
So I'm not even sure if we
Well, to be completely thorough, we would probably need to check each commit to make sure that the committer didn't (for example) change a directory they weren't supposed to and then change it back before the end of their commit. This isn't such a huge deal, as the changes in the "restricted directories" would have to result in no overall change for the whole push (otherwise a change would show up at the end of the push and that would trigger the permission restrictions). Even if this were the case, a malicious (or inexperienced) user could still muddy the commit history for files for which they weren't supposed to have access.
This makes sense, and is fairly easy to do, as the
updatehook of Git runs once for each branch or tag being updated.I have comments about this, but they should really be discussed elsewhere, as they are about multiple branches/repositories rather than permissions for branch/tag names.
That would be done as an implementation of
hook_versioncontrol_write_access(), correct? In this case, it really wouldn't be up to the VCS hook, since it is merely responsible for gathering the data from the hook event and passing it to the VCS api, right?That means that the only access checking that the
updateorpre-commithook would need to do is a call toversioncontrol_has_write_access(), since all other access control will be done as implementations of thewrite_accesshook, right?Done.
Access checks
Right. In fact, the Commit Restrictions module that ships with Version Control API already implements such a check, but we would need to make sure to propagate those directory settings to cloned repositories and make sure that the appropriate people (translators) have access to those repositories too. Or something.
Right, that's the plan.