sites/all support for modules and themes

I think this is 4.8 material. It's something we can commit as soon HEAD opens up.

Thank you for your great help. Just this line of code takes me two whole days to search. Thank you very much. You do a great help for me.

Crell, I have installed this and as a test installed ecommerce into the all directory. So great, and something I will use, except I need to patch 4.7 to do this first. :-)

OK then, I'd better test the themes directory and then I'll be confident that everything is ok. Doing this this morning.

In the meantime, I came across this book page about multisites http://drupal.org/node/43816, that refers to this similar "all" subject http://drupal.org/node/5942. Because the ticket is from a fairly long time ago, I wouldn't have thought twice, except for the fact that it's referenced in the handbook.

I have tested this patch (#3) using two domains with the same root. I installed an applied modules and themes in "all", "default" and "www.example.com" directories and the themes and modules are behaving normally.

This is a great benefit for a multisite installation - avoids maintaining duplicate non-core modules in each websites sites/ directory. Even better it is only 1 line of code. Thanks Crell.

Marking RTBC.

INSTALL.txt would need to be updated too.

Crell's patch had some changes to other files and I am ignoring those.

The new text reads well, I just made the paragraph (section 4) a little less pithy, because the example that follows is a good explanation in itself.

New patch attached.

So, the current applicable patches are at #3 and #14

-1 from me. I just don't see this as very useful. You can accomplish the same thing with symlinks in the sites/ directory, or by creating a modules/site-specific/ parent directory and sticking everything in there. I'm also not much of a fan of the new documentation, which makes it seem /required/ to use these new directories over the existing locations. Resetting back to "needs review" (regardless of my comment, a new patch was attached that needs review, not committing).

I have to disagree. Setting up symlinks should NOT be necessary for this kind of setup. On moderate-sized installations with a number of multisite installations, dropping key modules like Views, Devel, etc into the /all directory is self-explanitory IMO. That said, I agree that the documentation needs updating to clarify that the new /all directory is not required for use.

+1 for the concept, but as Morbus said the code needs review, not RTBC.

Here is a new INSTALL.txt patch which makes the instructions a little clearer (eg. recommended not required).

(Also, I acknowledge my earlier error of marking RTBC on a new patch.)

@Crell, ouch! :-)

Well, no pain no gain. He is a combined patch (system.module, install.txt) on head.

Three minor suggests on the text:

1. Delete the word "also" from "You can also download..."

2. Delete the sentence that begins "One way to install code." It no longer serves any purpose.

3. Use the full URL for the handbook, i.e. http://drupal.org/handbook, instead of saying "consult the Drupal handbook at drupal.org". This is just a courtesy to the reader, especially if they're using a text reader that automatically creates the links.

I'm also not fond of the sub.example.com example, and would rather see site1.example.com. It may be a subdomain in the DNS sense, but it's more likely to be a peer site from the Webmater's viewpoint. However, this usage appears elsewhere in the file, and I hesitate to suggest changing it here and not elsewhere.

Gary

> Sheesh, we're spending more time on documentation than on the code itself. I don't know if that's good or bad. :-)

It's good, if you want your patch committed. ;)

Shouldn't $searchdir[] = 'sites/all'; be wrapped in an if (file_exists()) check like $config/$directory?

If sites/all directory will be added to CVS, as I hope it will, I assume we won't need the file_exists check.

Hm. A multisite install is an optional thing though... the vast majority of sites out there will have their contrib modules stored in folders in the normal "modules" directory. You can argue about whether or not that's a smart thing to do, but the upgrade burden would be tremendous if people were *forced* to move their stuff under sites/all (ever had to move more than a dozen folders/files over a slow FTP link? Now imagine 500 or more as in a typical site with several contrib modules and themes. :P For some people this will be the only option).

This seems like a good patch for power users, but I don't think it should be required.. unless I'm missing something? And since it will not be required, it should be wrapped in an if (file_exist()) call. Even if it's included in the tarball by default, people might not upload it since it's empty, etc. Seems like just a basic sanity check thing to me.

No, people can still put modules anywhere. Although we have structured the new wording to encourage people to put non-core modules under sites/ to get them into good habits.

On a side note. Personal opinion. There are many times when I think this extra section, targetted at the average admin, is not in keeping with the rest of the INSTALL.txt file, which *I think* written above the head of the newbie. But to echo Crell, is this to much worry for a patch that won't break anything - I almost vote to leave out the documentation patch altogether, to look at this issue separately.

Hmm. Although I agree about "people might not upload it if it's empty". I hadn't really thought about that...

CVS can't manage empty directories.

I've been using the sites/*/*/* stuff for a while and often find it annoying that it is that many levels deep. I can't think of a good solution, but things are somewhat burried away in sites/*/*/*.

Dries: the only solution I can really think of is a dramatic reworking of the Drupal directory structure. Perhaps something that puts index.php, update.php, and cron.php at the main level, each site dirctory at the main level, and a /core directory at the main level containing the stuff that's currently loose in the drupal directory.

That's another can of snakes, though, as the dev mailing list can demonstrate... This patch, on its own, is definitely a boon. It means I have to make *fewer* visits to those buried subdirectories for updates and upgrades.

Eaton: agreed. It's another issue, and probably shouldn't affect this patch. Just wanted to point it out, in case it would have implications. :-)

i love the sites/all functionality, and the docs improvement. symlink solution imposes an unneeded burden on the admin. i also agree that we should ship with sites/all directory and even in that directory we should put subdirs for themes/ and modules/ and a README.txt in each. CVS needs a file in each directory so we have to use this README technique. How about:

"Place your custom directories in this directory. You may also place custom versions of core modules here."

At the sites/all, perhaps say: Place your custom modsules and themes into the subdirectories here. You might also make periodic backups of this directory in order to backup the custom code for your sites.

+1 @moshe - empty modules & themes directories, yes.

This patch contains a sites/README.txt which /encourages/ the newbie explorer to look deeper. This is followed up with 2 more README's in
- sites/all/modules/
- sites/all/themes/

OK, I know that scope creep is not the best way to get a patch applied (sorry Crell!). Hmm. But to me, it all falls into the same bucket, making sites/ a welcoming place.

oops, use this one instead

No complaints here.

Actually, number #25 seems to be intact. AFAICT, all issues and objections are resolved. (There are a few ideas for further doc improvements, but let's defer them.)

So, with the uncertainty of a drupal newbie, RTBC patch at #25.

Committed to CVS HEAD, minus the README.txt. We can ponder about those some more.

Here's an updated patch for 4.7, in case anyone needs it. Original still applies to 4.7 fine, with minor offsets.