Automatic clone of glip during drush pm-enable

The patch looks sound, and the approach can even be backported to 6.x, to remove the hard dependency on Libraries.

There are a few minor coding style issues though, plus some random comments:

+++ b/git_deploy.drush.inc
@@ -0,0 +1,69 @@
+      return dt("Downloads the Glip library from https://github.com/halstead/glip/. Places it in the libraries directory. Skips download if library already present. This all happens automatically if you enable git_deploy using drush.");

Should be using single quotes. There's nothing that requires the double quotes, and you're using single quotes elsewhere in the patch...

+++ b/git_deploy.drush.inc
@@ -0,0 +1,69 @@
+ * A command callback.

This could probably be more descriptive.

+++ b/git_deploy.drush.inc
@@ -0,0 +1,69 @@
+function drush_git_deploy_download() {

I'm pretty sure we want git_deploy's function to start with git_deploy_ (or _git_deploy_). Using drush_git_deploy_FOO might cause issue with a possible future hook_git_deploy_FOO()

+++ b/git_deploy.drush.inc
@@ -0,0 +1,69 @@
+      $path .= '/'. drupal_get_path('module', 'git_deploy') . '/glip';

Space between '/' and .. :)

+++ b/git_deploy.drush.inc
@@ -0,0 +1,69 @@
+  elseif (drush_shell_cd_and_exec(dirname($path), 'git clone git://github.com/halstead/glip.git && cd glip && git checkout 1.0')) {

Perhaps cloning via the HTTPS might be safer choice? I know my current network doesn't allow traffic on the Git (or CVS or Svn or ...) port, but accepts HTTP[S] happily. It's the same repos, but might be accessible to more people. :)

+++ b/git_deploy.module
@@ -39,8 +39,8 @@ function git_deploy_system_info_alter(&$info, $file, $type = NULL) {
+  // Skip this item if it has no .git directory or no Glip library installed.
+  if (!file_exists($git_dir) || !_git_deploy_include_glip()) {

Hm. I have a feeling this really belongs in another issue. I like it, but how is it required for (or at least related to) the feature request at hand?

Powered by Dreditor.

+++ b/git_deploy.drush.inc
@@ -0,0 +1,69 @@
+    $path = drush_get_context('DRUSH_DRUPAL_ROOT');
+    if (module_exists('libraries')) {
+      $path .= libraries_get_path('glip') . '/glip';
+    }

This section was leaving out a '/' separator so I added it and I also create the libraries directory if it doesn't already exist.

I've committed it at http://drupalcode.org/project/git_deploy.git/commit/af5ac3d please update here if I've missed anything.

Thanks a lot. This is so convenient.

Powered by Dreditor.

If one is using git-submodule to track the additions of various contrib modules, themes, ... and external libraries, this could cause some trouble in cloning Glip "outside" git-submodule.

Git will see the newly made directory and recognise it as an untracked file (see http://pastie.org/1661462), however, you can't tell git-submodule to add it, unless you also give it the repository URL - and you might as well simply have started off by git-submodule add'ing it.

I'm not really sure of what to do though. Should we check for .gitmodules, and use git-submodule add instead of git-clone if it exists? Should it be a Drush option? A combination? Something else?

I've no idea what the prevailing way of Git-managing sites is, but automatically fetching Glip would cause some annoyance in my workflow.

We now need to get this functionality into 6.x as well.

Any value to putting glip directly into drush somewhere, then selectively including it? Like, maybe an spl_autoload_register()'d function for getting glip in, then (potentially - I haven't looked at the recent changes to the include mechanism) refactoring git_deploy so it gracefully handles that case?

It looks to me like this clones into sites/all/modules/git_deploy/glib, whereas the instructions say to put it into sites/all/libraries?

With it in sites/all/modules/glip, you can't use submodules correctly because you have a repo inside a child repo.

I like to have glip in sites/all/libraries and use the libraries module so that other code can take advantage of glip as well. If you have the libraries module installed then glip will go into sites/all/libraries and the problem will be avoided.

Would the --skip option mentioned in #7 fix the problem when the libraries module isn't present?

Not sure whether this is related or should be a new issue: When I ran drush pm-enable git_deploy it barfed:

Executing: git clone https://github.com/halstead/glip.git && cd glip && git checkout 1.0
error: SSL certificate problem, verify that the CA cert is OK. Details:
error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed while accessing https://github.com/halstead/glip.git/info/refs
fatal: HTTP request failed

I had to change "http://" to "git://" in git_deploy.drush.inc.

Patch file attached as "git_deploy.diff".

If this is not the appropriate thread, then please refer and or let me know and I will create new issue.

I don't think this will ever be backported to 6.x. I'm closing this issue.