Install Drupal modules/themes/profiles to the correct location #65
Conversation
| /** | ||
| * Exhaustive depth-first search to try and locate the Drupal root directory. | ||
| */ | ||
| protected function locateDrupalRoot($path = '') |
There was a problem hiding this comment.
I haven't done drupal development in quite some time; so i'm not familiar with current development and composer practices with it, but i would almost expect drupal to be in a child directory just as often as a parent directory. is that case covered with this PR?
There was a problem hiding this comment.
No, it doesn't scan child directories, nor am I sure if that should be a considered use case. If you're doing Drupal stuff like that, then you have symlinks set up to handle such a thing. Most cases though, it would be a parent directory.
Worth considering further down the line though! What are your thoughts?
There was a problem hiding this comment.
I know drupal has changed significantly since i've worked with it... I was accustomed to working with it fairly similar to how i interact with wordpress. Now that it has a symfony stack it's significantly different, with wordpress it needs to be within the webroot and my standard implementation has the root composer.json above the wordpress directory. However, in that case it may be getting into handling webroot packages which i believe is outside of the desired scope of this project. I created a separate installer for that use case, and i have PR #52 to take care of the ability to manipulate where the project lives in the root composer.json. Maybe implementation of child directory installations would not be necessary with that PR being merged in.
There was a problem hiding this comment.
Very nice PR, @slbmeh. I'll be sure to read through it closer soon. Your work on getting Wordpress to be composer.json-compliant is great, and definitely something that I see the DrupalInstaller benefiting from.
This PR mimics the effect that the Drupal command line utility has, and wouldn't require the extra notes in most composer.json files. I understand having a composer.json in a child path and not within the root is somewhat backwards from a best practice Composer project, but that is unfortunately how Drupal is designed. There are plans to change that within the next couple years though, however, for Drupal 9. We could adapt this solution quite easily to match accordingly by then.
|
Just quickly scanning this looks good. Thanks @RobLoach! I just really need to get #43 implemented or at the very least something basic that actually installs a package. Otherwise I'd be maintaining all this more complex path determining logic blindly. So sorry to do this to you again. Hopefully I can move on that issue soon. Thanks for understanding! |
|
I get detecting whether something should be installed in /modules or /sites/all/modules, but I'm not sure I understand the references to detecting the drupal root, what is the purpose of that? Thanks! |
|
Hey guys, There is a possibility that this patch will be merged? |
|
This PR has merge conflicts. |
|
Thanks for the ping, @ofry. Going to close this one as it's old and has conflicts. Feel free to re-open if desired. |
This makes it so that the Drupal installer detects where Drupal is installed, and then will install it to
sites/all/modulesormodulesdepending on what Drupal version they're running.Fixes #44 and #42 .
Ripped most of the code from DrupalInstaller, with some touch up and some new additions from the latest Drush.