I just used this for the first time and noticed that it creates the module in the modules/contib directory. I know that everyone's directory structure could be different, but I would expect the last place for a custom module to be created is in the place specifically designated for contributed modules.

It should really make the modules in either /modules or in /modules/custom

CommentFileSizeAuthor
#9 module_builder.custom-vs-contrib.-966930.patch1.01 KBaaronott
#8 966930.module_builder.custom-vs-contrib.patch914 bytesjoachim

Comments

joachim’s picture

joachim commented
Status: Active » Postponed (maintainer needs more info)

It puts it in /sites/all/modules, unless you tell it otherwise. What command are you using?

frob’s picture

frob
Primary language English
commented

drush mb module_name node_info menu --write

I just double checked it, yea that is what happens to me.
I am using drush 3.3

joachim’s picture

joachim commented

Sorry, I can't reproduce this with Drush 3.x.

There might be some Drush special-casing where if the /contrib folder exists it goes for that -- try it when that doesn't exist.

frob’s picture

frob
Primary language English
commented

I just tested it and that seems to be the case.

If there is a contrib directory then I puts it in there, if not then it just puts it in the module directory.

I would put this under category "Annoying" if it existed.

joachim’s picture

joachim commented
Title: drush writes modules to modules/contrib » drush writes modules to modules/contrib when it exists
Status: Postponed (maintainer needs more info) » Active

Ok so what's causing it is this:

On the one hand, Drush wants to be helpful, so 'drush dl' downloads modules to modules/contrib if that exists, and /modules if not. That's fair enough.

Then module builder tries to figure out where to write a new module. To do this, I've just hijacked 'drush dl' to give me a location for the modules folder. So the problem is here.... we shouldn't rely as much on dl's internal stuff.

frob’s picture

frob
Primary language English
commented

how about just adding a validation. If those are the only two options that drush dl gives us then we could just validate what dl gives us.
put an if check after the path is retrieved.

if (path returned by dl) ends with "/contrib" {
remove "/contrib"
}

I don't have time to write a patch for this but just turn that into real code and that should work then, right?

joachim’s picture

joachim commented

Hmm, you're right, hacking it out is probably the way to go, as this is quite deep in drush pm:

function pm_dl_destination_lookup($type, $drupal_root, $sitepath, $create = FALSE) {
  switch ($type) {
    case 'module':
      // Prefer sites/all/modules/contrib if it exists.
      $destination = $sitepath . 'modules/';
      $contrib = $destination . 'contrib/';
      if (is_dir($contrib)) {
        $destination = $contrib;
      }
      break;
joachim’s picture

joachim commented
Status: Active » Needs review
StatusFileSize
new 966930.module_builder.custom-vs-contrib.patch914 bytes

Here's a patch I don't have time to commit right now... feel free to give it a try.

aaronott’s picture

aaronott commented
StatusFileSize
new module_builder.custom-vs-contrib.-966930.patch1.01 KB

Yep that works for me, but only if custom exists as well. What if it checks to see if custom exists, if not go ahead and add to contrib or I suppose you could add to modules instead.

quick patch (I only added a bit of checking to your patch)

joachim’s picture

joachim commented
Status: Needs review » Patch (to be ported)

Thanks for the patch!

Fixed on 7.x.

Issue #966930 by aaronott, joachim: Fixed module being written to 'contrib' subfolder when it exists.

I should probably cherry-pick this one to 6...

joachim’s picture

joachim commented
Status: Patch (to be ported) » Fixed

Pushed to 6 too.

Status: Fixed » Closed (fixed)

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

donquixote’s picture

donquixote commented
Title: drush writes modules to modules/contrib when it exists » Filesystem location of modules created with "drush mb": contrib vs custom.
Status: Closed (fixed) » Needs work

It should rather put those modules into sites/all/modules/custom, if that exists, and sites/all/modules, otherwise.
Besides, it would be nice to have an option to create in the current location.

joachim’s picture

joachim commented
Status: Needs work » Fixed

> It should rather put those modules into sites/all/modules/custom, if that exists, and sites/all/modules, otherwise.

That's what it does.

> Besides, it would be nice to have an option to create in the current location.

See the --parent option.

donquixote’s picture

donquixote commented

Not for me :(
They currently all go in sites/all/modules, even though sites/all/modules/custom does exist.
Besides, mb does not tell where it puts a module. I have to look myself.

Could this be caused by my drush settings?
(don't know where to find those, atm..)

joachim’s picture

joachim commented

Works for me.
Check you have the most recent mb.
What's your version of drush?

Status: Fixed » Closed (fixed)

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