Remove unnecessary files[] declarations from core info files

Very good idea. Some .module files need to be kept though, as they contain entity controller classes.

strip_unnecessary_file_declaration.patch queued for re-testing.

Manually went through all module .info files throughout core and contextually double-checked which files[] declarations are actually needed.

Last patch passed.

This speeds up registry building with no ill side effects. And even if it did have a side effect, it is easily remedied.

It is important that core sets a good example here since the parsing will get really expensive if all contrib modules add all their files regardless of contents.

I advocated doing this a long time ago and was shot down, on the grounds that having a full registry table of all compiled code could be useful to contrib. Removing it now would represent a regression from what's been in D7 for the entire life of its release since the registry patch was committed (roughly 2 years ago).

I don't see how we can do this now.

Lets reserve the word regression to refer to a bug that existed, was fixed, and has now returned. This task does not represent a regression. This is a performance fix for drupal, and a low risk one at that. I get that we want to be conservative right before release but low risk performance fixes are always eligible, IMO.

I also don't think that the current information is of any substantial use for anyone. From a technical perspective, it is incomplete and insufficient. And if you'd really need all files, then a manual file_scan_directory() is much more faster than retrieving a manually compiled registry of files per module, which still needs to be completed because it's incomplete. It simply makes no sense. ;)

Also, on slow machines and/or file systems (especially Windows), there should be a larger performance impact, because every single file that is listed in files[] is

- checked for file_exists()
- checked for its file modification date
- possibly read into memory
- grepped for PHP OO constructs through a regular expression

upon registry rebuilds. Admittedly, registry rebuilds don't happen very often, but when they happen, the entire request is very slow anyway already, so it's definitely worth cut off any unnecessary overhead.

I don't totally recall who it was who shot this down before, but it seems like a good idea to get chx to sign off on this patch. If he wasn't the person who shot it down, he at least did a lot of work to put the registry there in the first place.

./field/field.attach.inc:class FieldValidationException extends FieldException {
./field/tests/field_test.entity.inc:class TestEntityBundleController extends DrupalDefaultEntityController {
./search/search.extender.inc:class SearchQuery extends SelectQueryExtender {
./simpletest/drupal_web_test_case.php:class DrupalUnitTestCase extends DrupalTestCase {
./simpletest/drupal_web_test_case.php:class DrupalWebTestCase extends DrupalTestCase {
./system/system.archiver.inc:class ArchiverTar implements ArchiverInterface {
./system/system.archiver.inc:class ArchiverZip implements ArchiverInterface {
./system/system.tar.inc:class Archive_Tar // extends PEAR
./system/system.updater.inc:class ModuleUpdater extends Updater implements DrupalUpdaterInterface {
./system/system.updater.inc:class ThemeUpdater extends Updater implements DrupalUpdaterInterface {
./system/system.queue.inc:class DrupalQueue {
./system/system.queue.inc:class SystemQueue implements DrupalReliableQueueInterface {
./system/system.queue.inc:class MemoryQueue implements DrupalQueueInterface {
./system/system.mail.inc:class DefaultMailSystem implements MailSystemInterface {
./system/system.mail.inc:class TestingMailSystem extends DefaultMailSystem implements MailSystemInterface {

are the files that contain a class. Only system.queue.inc has an interface. I reviewed and these files are present in the info files after the patch.

Ok, I talked to Dries about this given how late it was, and he said:

"for the love of god, please commit that :)"

:D

I noticed at least one problem (dashboard.test is removed and shouldn't be). Could we get one more pass at this and then I can commit it later tonight?

+++ modules/dashboard/dashboard.info	6 Dec 2010 10:41:50 -0000
@@ -4,7 +4,6 @@ description = Provides a dashboard page 
-files[] = dashboard.module
 files[] = dashboard.test

um, but dashboard.test is not removed?

Powered by Dreditor.

Oh. I noticed another critical problem. With my reading comprehension level. I was reading Sam's patch and not yours. ;P~

Committed to HEAD. YAYYYY!!!

Now we need to update the documentation in about 50,000 places that you no longer need this stupid index.

Also tagging as an "API change" because this needs to be announced.

Randy: Change is that you no longer need the stupid files[] declaration in .info files unless you are defining a class. (so .test files, entity controllers, etc.) Someone else can likely come up with a better, more copy/pastable summary.

Yeah, I'd like more clear direction on how to announce:

"All files[] directives can be removed unless there is a class defined within. So you *can't* remove *.test or other files that provide a class definition" ???

But what files[] would have to remain?

The files[] property in module .info files should only contain files that contain any PHP classes and/or interfaces. Most commonly, these are test classes, exception classes, select query extenders, or mail system implementations.

Awesome.

It looks like there are several more we can remove; I looked to see which .module files we had left after this (there are very few) and the attached patch removes the remaining obvious ones that don't have any classes or interfaces in them.

(Come to think of it, we could maybe just remove all .module files from the registry anyway, since they should never need to be autoloaded. But this patch doesn't go that far.)

oh, thanks, @David_Rothstein! Those modules must have been introduced along the way (or my checkout was stale, perhaps).

Committed to HEAD. Thanks!

This also needs to be updated in the module dev guide for D7 in the Handbook, which I'm pretty sure said something about the files array in .info files (though I don't have the page handy).

Right, I guess this is still "needs work" for documentation.

sorry, my bad!

Very late to this one but woo!

In the Module dev guide, the page on .info files already seems to be correct:
http://drupal.org/node/542202
It says you need files[] only for classes/interfaces.

Also, the 6/7 guide looks correct:
http://drupal.org/update/modules/6/7#registry

So, looks like the doc is updated.