Git / drupal.org integration

+++ commands/pm/package_handler/git_drupalorg.inc
@@ -0,0 +1,93 @@
+  if (drush_shell_exec("cd " . escapeshellarg($project['base_project_path']) . " ; git rev-parse --git-dir")) {

these sorts of lines have been troublesome in windows. @greg - see any errors here? also see shell_exec lines below

whats the advantage of this versus a more generic git integration?

@Damien: Suppose the developer does create a clone of the upstream repository; what then? You have hardcoded git.drupal.org in your patch:

$repository = 'git://git.drupal.org/contributions-stable/' . $project_name . '.git';

Shouldn't there be some $option that contains the base repository URL? I would recommend renaming this module to just git.inc and make it work with other git repositories, defaulting to git.drupal.org -- unless git has some feature that says "my version of "git://git.drupal.org/contributions-stable/somemodule.git" is at "...", in which case this could use a little documentation. (And I'd still rename the file to git.inc).

Regarding the handling of paths, in our experience, both escapeshellarg and escapeshellcommand are destructive to paths when used on Windows.

@Moshe: The cvs code looks like this:

  if (!drush_shell_exec('cd ' . $checkout_path . ' ; cvs ' . implode(' ', $cvsparts))) {

It seems that perhaps this should be wrapped in \', just like in #477720: Commands do not escape spaces in directory names.

I think that the best way to handle paths is like this:

if (drush_shell_exec("cd \'" . str_replace("'","'\''",$project['base_project_path']) . "\' ; git rev-parse --git-dir")) {

At least, that's what I did in #766080: Windows support for drush: escaping the path to drush in backend invoke and elsewhere, but that issue has not yet been reviewed and committed. We don't have any Windows drush users active in the issue queue...

Oooh, very nice, and very useful! I'm just gonna sidestep the escaping discussion...

Per my bellyaching in #782684-4: Establish and codify standard set of git-for-drupal workflows (meta issue), though, this ought to support more than just submodules. I'd prefer to see the use of submodules as an optional parameter (at least, when getting a new repo), as there are equally legitimate use cases for either workflow.

I'll circle back round on this patch in a few days to add that parameter - just wanted to get my 2c in on it now, since I just saw the issue.

OK, patch attached that addresses the issues I raised. You can now pick between using submodules or not, defaulting to not. Using submodules requires the whole Drupal instance to be versioncontrolled with git so it can act as the superproject, whereas the non-submodule approach works regardless of the outer vcs being used.

It might be nice if someone added a note to the submodule approach that reminds users that they'll have staged, uncommitted changes after having added the submodule that they ought to add. Or possibly add some more logic that commits it automatically. But the basics are there, now.

Thanks Sam. I'd like some feedback on this and also escaping changes per Greg's earlier note. Lets assume his solution is correct as we have not heard otherwise.

Subscribe

Subscribing...

EDIT: For some unknown reason, while subscribing to this issue the Component got changed. Formerly it was set to 'Code', but I can't see it as an option in the dropdown. Please, someone with better knowledge change it to something more accurate.

So, is this still valid given all the progress on git.drupal.org? Anything else we should add before commit?

@MyXelf - the component changed because one of the project maintainers changed the component list (removed Code and added some others) in the project settings, and with the way project works, issues using old components only get updated when somebody actually responds. And it just happened to be changed to the right one. So, not your fault, don't worry about it.

@moshe - I've attached another patch which updates uses the URIs for the new repository layout, and so is at least fixed from that perspective. Beyond that, it's identical to what I did in #6.

This looks promising.

This seems to work fine. I'm not used to seeing (no branch). Is that OK? See below which I ran on my new download of votingapi project. Other projects end up in same (no branch) state.

~/htd/d6/sites/all/modules/votingapi$ gb
* (no branch)
  6.x-2.x

I guess this git msg is part of the story

Note: checking out '6.x-1.0'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

git checkout -b new_branch_name

Yes, it happens because the checkout refers to a tag (i.e. to a specific commit), not a branch itself.

I committed this.

Regarding Greg's comments in #4 ...

1. Seems like noone really wants to take on the paths issue described at Hopefully we will get a windows maintainer one day.
2. As for renaming this to git.inc, I am unsure. This is a package handler and thus it is fair for it to only work with one source. Our other package handler, cvs.inc, also only works with drupal.org. We do need a git.inc as a version control handler but thats a somewhat different issue.

Patch to exploit drush_set_error()'s return value is still welcome?

If so, here it is:

Did not test, but #17 definitely more correct.

committed

Some modules dl work fine, and some others end with an error :

remote: Counting objects: 297, done.
remote: Compressing objects: 100% (110/110), done.
remote: Total 297 (delta 187), reused 297 (delta 187)
Receiving objects: 100% (297/297), 149.35 KiB | 84 KiB/s, done.
Resolving deltas: 100% (187/187), done.
Unable to retrieve cck from git.drupal.org.
An error occurred at function : drush_pm_download

(seen on both a d6 and a d7 install, e.g on cck-6.x-2.x-dev, or field_group-7.x-1.x-dev)

Looks like it was a transient infra problem. This works for me:

drush dl cck -v -y --package-handler=git_drupalorg

drush dl cck -v -y --package-handler=git_drupalorg : works

but any attempt with dev releases fail :

drush dl cck-6.x-2.x-dev -v -y --package-handler=git_drupalorg
drush dl views-6.x-2.x-dev -v -y --package-handler=git_drupalorg
drush dl admin_menu-6.x-3.x-dev -v -y --package-handler=git_drupalorg

quite possibly an infra problem with git.drupal.org, dunno

OK, It does look like drush code is at fault here. Here is what drush does:

git clone 'git://git.drupal.org/project/cck.git'             [notice]
'/tmp/drush_tmp_1291471226/cck' && cd '/tmp/drush_tmp_1291471226/cck'
&& git checkout 'DRUPAL-6--2'

The failure is in the final line. We are trying to checkout a branch that does not exist AFAICT. I can't verify that since http://git.drupalcode.org/project/cck.git won't load.

Just committed a fix. Would be good if sdboyer or other could verify. We needed to 'git checkout 6.x-2.x' instead. My fix is at http://drupalcode.org/viewvc/drupal/contributions/modules/drush/commands...

Works fine. Thanks Moshe !

re: #24 - yep, that looks like it should take care of it.

There's something important that needs to be understood about how git.drupalcode.org works - every project is rebuilt every two hours, on the half hour. The migration process blows away the repo, rebuilds it, then puts it back in place. So, during the rebuild process, there's no data there - which explains why clones occasionally fail. For most projects, this window is really small - maybe 15 seconds. For core, it's more like 7-10 minutes, and for big projects like cck, maybe 2 minutes. So the bigger the project, the more likelihood of getting caught in that window. I'd like to eliminate that window, of course, but with the current architecture of the migration scripts, it's not really feasible. That could change soon, but don't hold your breath, as it'd only change if we get to it in the course of other changes that are on the critical path.

This might have been what happened in #20, though the fact that yched got that initial output means that it would've had to disappear mid-clone.

@sdboyer : thanks for the explanation - the errors I had really seem to be related to the drush command trying to checkout the wrong branch at the end of the cloning - only and systematically happened whith -dev branches, while everything was ok for release tags.

Just curious : will that 'window' effect persist after we definitely switch CVS off, or is only a transition thing ?

@sdboyer: Is there any way drush could proactively test to see if the project it is trying to access is "busy"? Maybe a REST service that returned the name of the project being rebuilt?

It would be nifty if the project rebuild process would take no action unless the code actually changed.

I'm pretty sure the rebuilding is only happenning between now and mid Feb when we are running CVS and git concurrently. So, no need to handle the interim in a cleaner way, IMO.

@yched - the window occurs because we're migrating the entire CVS history to git every two hours. Once the migration's done...well, no migration, no window :)

@greg.1.anderson - Nope, not possible to set up such a service. Not in any kind of reasonable timeframe, anyway. #901562: Rebuild git projects more frequently if there are commits would be a simpler way to solve this problem, but the current git team members don't have the bandwidth to work on it right now. We're always open to having more people help, of course, but that particular problem requires a lot of infra access to solve, so it's not exactly low-hanging fruit. Not impossible, just not the easiest thing.

All of these are reasons, though, why I have always publicly stressed that while git.drupalcode.org is a nice and handy tool, it is not something to be dependent on in a production environment. We are trying to get it more stable (and it is possible that it would reach a point where I'd call it production-ready PRIOR to the full launch), but I can't promise that.

I agree with moshe; if this is just an interim CVS -> git migration thing, then there's no point spending the time to fix it.

Fully agreed, thanks for the details Sam.

FYI, checkout of HEAD branch with git_drupalorg is broken until #994244: Rename the master branch to a proper version branch is resolved

I've made some changes that don't alter functionality.
* Use of drush_shell_cd_and_exec().
* Dropped several calls to escapeshellargs() --it is already done by _drush_shell_exec().
* better support for --simulate and a non-destructive check for project's realpath().
* also some docstrings.
* I've also remove strtok($project['name'], ' ') .... project names do not contain spaces :/

http://drupal.org/cvs?commit=466400

FYI: in #999480: Git Submodules and Drush temp directories we've deeply reworked the code around --gitsubmodule