During the Panels upgrade sprint, I noticed that when panels_node.info looked like this:

name = Panel nodes
description = Create nodes that are divided into areas with selectable content.
package = "Panels"
dependencies[] = panels
configure = admin/build/panels
core = 7.x

then entry on the module page looked like this:

But when I added just one little line:

name = Panel nodes
description = Create nodes that are divided into areas with selectable content.
package = "Panels"
dependencies[] = panels
configure = admin/build/panels
core = 7.x
files[] = pooper.module

it works just fine:

Hopefully the problem here is obvious (there's no pooper.module!) - without at least one files[] entry, a module gets marked as invalid somewhere. When the full registry was in, this made sense, but since we've rolled the registry back to only encompasses class autoloading, I don't see why a class-less module ought to have to declare any files. Looking through other core .info files, it seems like we've just silently ignored the question of whether files without classes still ought to be listed in .info files. At the very least, the documentation isn't helpful, since it's really just a whole lotta strikethrough.

So, what's the verdict? Do we still need to list files[] even if there aren't classes in them? Or is it safe to loosen that requirement - in which case, we need to patch core so that modules aren't erroneously listed as incompatible?

CommentFileSizeAuthor
#13 update_remove_files_check.diff565 bytesjmiccolis
#2 info-files.patch1020 bytesbleen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sdboyer’s picture

Category: task » bug

Changing to bug report.

bleen’s picture

Status: Active » Needs review
FileSize
1020 bytes

This patch should fix the issue ... I can't see any reason why we still need the if (empty($info['files'])) here

Lets see what testbot thinks

Status: Needs review » Needs work

The last submitted patch, info-files.patch, failed testing.

Damien Tournoud’s picture

Status: Needs work » Reviewed & tested by the community

Indeed, we don't need that anymore. That's a remainder of the removal of the function registry.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Good catch. Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

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

mattyoung’s picture

Status: Closed (fixed) » Active
marcingy’s picture

Status: Active » Fixed

Only http://drupal.org/node/542202#files needs changing and has been updated to note that files are now optional. The other link is still correct as you still need still need to declare files containing interfaces and classes.

aspilicious’s picture

I thought this was by design LOL

mattyoung’s picture

#8:

I still see in http://drupal.org/node/542202#files:

files (Required)
Module .info files must now specify all loadable code files explicitly. Drupal now supports a dynamic-loading code registry. To support it, all modules must now declare their code files in the .info file like so:

In http://drupal.org/node/394070

Module .info files must now specify all loadable code files explicitly

This is confusing. It's not required and it's not optional. Can we make it plainly clear when "files[]" are necessary? Is "files[] = mymodule.module" even necessary at all if there is no function registry anymore?

I feel this issue should be active until the documentation is fixed. I am beginning to update my modules to D7 and the first problem I run into is what to do with the .info file.

bleen’s picture

Status: Fixed » Active
Issue tags: +Documentation, +.info file

there is no reason not to reopen this for the docs

mattyoung’s picture

One of the thing that confuses me is "loadable code files", what are they? Aren't all php code files loadable? If that are files containing classes and interfaces, then just say that instead. It would make things much clearer.

Also how about removing the strike through text? They don't mean anything anymore and it just add confusion.

jmiccolis’s picture

Status: Active » Needs review
FileSize
565 bytes

It appears that update.inc has a check for the files array as well. You can see this in action as modules with no entries for `files[]` will be disabled by a visit to `update.php`.

yhahn’s picture

Priority: Normal » Major
Status: Needs review » Reviewed & tested by the community

I can confirm that this patch fixes the update.php problem. Marking as major since the current bug causes many contrib modules to be turned off just by running update.php.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

jmiccolis’s picture

Component: base system » documentation
Category: bug » task
Priority: Major » Normal
Status: Fixed » Active

As bleen pointed out in comment 11 - this still needs docs to be updated. Re-opening.

jhodgdon’s picture

Component: documentation » base system
Status: Active » Fixed

Please just go edit those pages. This is not a Drupal Core / documentation issue. It is a Handbook documentation issue. So if you don't want to edit the pages yourself, please file a new issue in the Documentation project, so we can leave this one in Drupal Core for tracking.

Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -Documentation, -.info file

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