Missing sites/all/modules folder is confusing

1000 times yes. This is something that is needed to stop some people from installing modules into the /modules folder.

Subscribing.

It's not only a usability issue for new users, it's also annoying do cd sites/all/modules [..swear..] mkdir sites/all/modules [..swear again..] cd sites/all/modules cvs checkout ......

+This directory should be used to place downloaded and custom themes
+which are common to all sites. This will allow you to more easily
+update Drupal core files.

How about:

Place downloaded and custom themes in this directory. This simplifies future updates and separates your custom files from Drupal core. In a multisite installation, themes in this directory are available to all sites.

(and again for modules)

And, this is a great idea.

"1000 times yes. This is something that is needed to stop some people from installing modules into the /modules folder."

It is natural behavior to spot /modules and put modules there because:

-It works
-Its in the root folder

Maybe what we really need to stop this kind of confusion is a /core/modules directory or something similar, we could even put /includes and /themes in there, maybe even rename the folder /core-modules and /core-themes to avoid confusion.

I've also wanted to put all core themes and modules in the profiles/default folder before. A /core folder sounds better though. Agreed it's a separate issue, I assume the main problem would be moving the files around in CVS.

sites/all/modules, and sites/all/themes are not present in CVS because they would not appear in the .tar archive Drupal creates from CVS if those directories don't contain anything.

The use of such directories is already documented; if even somebody would have the doubt the archive he downloaded could be corrupt, it would not take too much to understand that is not the case, and that he simply needs to create the directories.
After all, he has been able to install Drupal; he could resolve the issue easily enough.

sites/all/modules, and sites/all/themes are not present in CVS because they would not appear in the .tar archive Drupal creates from CVS if those directories don't contain anything.

Well, that's what this patch is about, isn't it? It puts a README.txt into these directories to make them appear in the tarball.

After all, he has been able to install Drupal; he could resolve the issue easily enough.

Our goal should always be to make installation, usage and updating of Drupal as easy and straightforward as possible. This is why we created the installer, after all. So +1 from me for this patch.

Moving the current core modules/themes will be very difficult since users would have to delete the folders manually when upgrading. Although I guess we could add a check in the D6->D7 update.php to make sure the /modules and /themes folders do not exist in root, but rather in the proper location.

I agree that the installation must be as easy as possible.

My point is that still the user must do something manually (and read a file to understand what to do). After somebody created an user for the database, after he created a database, and he assigned that user to the database (and gave to that user the right access permission to the database), I find rather rare he would block on the simpler step.

Makes total sense.

Requalifying as a bug report (I agree with catch on this: this should backported to lower versions).

@Kiam, one issue is that the sites/all folder is relatively new - prior to Drupal 5 (I think) we used to recommend putting everything in /modules and /themes - and since they're at the root of the installation, it's easy to assume that's where modules and themes should go.

Do we have an issue with people upgrading on some operating systems if we include these folders? I seem to remember default.settings.php was added instead of settngs.php to avoid Mac users overwriting their settings.php - presumably the same thing could happen for minor version upgrades to the themes and modules folders as well. Or is it only files that overwrite and not folders in that case?

@catch: Mac users have issues with merging anyway. Copying a clean Drupal folder with the Finder over an existing one will wipe the original one completely, regardless of everything.

Cool. I also doubt that many sites are actually managed on Macs via the GUI (and would rather we shipped with settings.php too actually since that'd remove a step for 99% of users) so still completely RTBC.

Looks good to me. What about the snippets provided by Keith?

I think John Albin is right that we should avoid the reference to 'in a multisite installation' - since we just want to make sure people put their modules and themes here when they start , and they can get into the nitty gritty of which modules to share between multisites at a much, much later stage. Also " separates your custom files" - contributed modules and themes aren't custom files, and we have a files directory in sites/default/files by default as well, so while custom modules and themes should go in here, we can't refer to them like that.

Here's a pass at it, which takes Keith's text and just hacks those bits out.

Place downloaded and custom themes in this directory to simplify future updates to Drupal core. (replace modules for the modules version).

@Dries, JohnAlbin addressed that in #6. Essentially, the model he used is that found in /sites/all/README.txt. The model I used is that found in /modules/README.txt, which does mention multisite installations.

I'm fine with the patch being committed as is, as it is consistent with at least one of the existing READMEs. We can always clean this up later by patching all these READMEs so that they all use very similar language (the odd one out, at that point, will be /modules/README.txt).

Or, as catch suggests, we can try to consolidate and conform these various files in this issue. I'm ok with either approach.

YAY! This has been one of my pet peeves forever. :) Committed to HEAD.

Marking back to CNW to make the various README.txt files consistent, which should be much easier now that these files exist in HEAD.

The readme file really was not that far off.. but regardless, here is a slightly updated version.

Lets try this again...

Or again...

Here's a re-rolled patch.

Lets try this again... something is quite odd here.

Committed to CVS HEAD. Thanks all! :)