Evil hack: admin/modules without memory restrictions
Hi there! There are quite a few people who'd like to run a lot of Drupal modules and have problems on admin/modules when they hit the php memory limit. Moshe pointed out that this has changed/ will change with the install files, but I needed something quick and dirty now. Attached is a patch that will make Drupal 1) read the module files 2) looking for help, init, exit hooks 3) write these hooks to a file 4) include that file 5) delete the file for all modules found. The patch makes a few assumptions about how a module file looks like (namely coding conventions). A few contrib modules do not respect these or they do call some other functions from their help hooks. These are: adsense contextlinks cursor g2 smartypants textile xstatic (all taken from contrib HEAD). I am surprised that the list is so short. The patch isn't proposed to be included in Drupal, also people who don't know PHP should not use it. I will not support it. It is intended to be run in development setups where you might want to test a lot of modules. Cheers, Gerhard
The patch makes a few assumptions about how a module file looks like (namely coding conventions). A few contrib modules do not respect these or they do call some other functions from their help hooks.
These are:
adsense
adsense.module calls a function in help so that it can display ad formats which are maintained as an array so it is easy to change.
On 21 Jan 2006, at 7:26 AM, Gerhard Killesreiter wrote:
Attached is a patch that will make Drupal
1) read the module files 2) looking for help, init, exit hooks 3) write these hooks to a file 4) include that file 5) delete the file Or you could skip all that and have a simple text file in the module directory that is parseable.
I don't believe there's a single good reason to have all the modules loaded just to get their descriptions. -- Adrian Rossouw Drupal developer and Bryght Guy http://drupal.org | http://bryght.com
Adrian Rossouw wrote:
On 21 Jan 2006, at 7:26 AM, Gerhard Killesreiter wrote:
Attached is a patch that will make Drupal
1) read the module files 2) looking for help, init, exit hooks 3) write these hooks to a file 4) include that file 5) delete the file
Or you could skip all that and have a simple text file in the module directory that is parseable.
I don't believe there's a single good reason to have all the modules loaded just to get their descriptions.
That is certainly what the future should be. This patch is really just a hack that can be used Right Now to make admin/modules use less memory. That's why killes said this is not in consideration to actually be used by Drupal.
Adrian Rossouw wrote:
On 21 Jan 2006, at 7:26 AM, Gerhard Killesreiter wrote:
Attached is a patch that will make Drupal
1) read the module files 2) looking for help, init, exit hooks 3) write these hooks to a file 4) include that file 5) delete the file
Or you could skip all that and have a simple text file in the module directory that is parseable.
Right. But as I said this is an evil hack.
I don't believe there's a single good reason to have all the modules loaded just to get their descriptions.
I fully agree. The problem with that approach seems to be that once you say something like this, people also want a dependency system and fully automatic database installs. While I agree that these things would be nice to have, it would make more sense to start simple... Cheers, Gerhard
-- Adrian Rossouw Drupal developer and Bryght Guy http://drupal.org | http://bryght.com
Op zaterdag 21 januari 2006 07:18, schreef Gerhard Killesreiter:
While I agree that these things would be nice to have, it would make more sense to start simple...
+1 A thought I recently had was to load the modulles smarter. Instead of loading them all we should think of a way to: * load a module * add the hook_help description output to $descritions[$modulename] * de-include that file (this is the hard part :) * whipe the file from memory (can PHP do that) I think that this can be done by freading and evalling the modules. Bèr -- [ Bèr Kessels | Drupal services www.webschuur.com ]
Bèr Kessels wrote:
Op zaterdag 21 januari 2006 07:18, schreef Gerhard Killesreiter:
While I agree that these things would be nice to have, it would make more sense to start simple...
+1 A thought I recently had was to load the modulles smarter. Instead of loading them all we should think of a way to: * load a module * add the hook_help description output to $descritions[$modulename] * de-include that file (this is the hard part :) * whipe the file from memory (can PHP do that)
I think that this can be done by freading and evalling the modules.
Don't post before the first coffee. :p I don't think that your approach would work. Small text files that contain a description of a module will be much easier to handle. We currently only need to read the module file to extract the description of the module and to check whether it has an init or exit hook. That's all info we use on admin/modules. Cheers, Gerhard
While I agree that these things would be nice to have, it would make more sense to start simple...
+1 A thought I recently had was to load the modulles smarter. Instead of loading them all we should think of a way to: * load a module * add the hook_help description output to $descritions[$modulename] * de-include that file (this is the hard part :) * whipe the file from memory (can PHP do that)
I think that this can be done by freading and evalling the modules. Another possibility is to have php files with the module descriptions, so you can just load those. They hould containg 'generic' information about the modules.
If you guys agree on what should be contained in the files - php or plain text I can generate you those for conrib and/or core very quickly - I have the code for that as part of the install/packaging thingie I'm doing.
On 1/21/06, Adrian Rossouw <adrian@bryght.com> wrote:
Or you could skip all that and have a simple text file in the module directory that is parseable.
I don't believe there's a single good reason to have all the modules loaded just to get their descriptions.
Agreed - a text file in the module directory would definitely be a big step forward. As well as improving performance for the admin/modules page, it would also provide a concise description of the module, for inquisitive end-users installing that are installing it. I think a file with a particular extension would work well (e.g. modulename.desc - for 'description file'). But adding a .desc file for every core module would be bad - would clutter up the core /modules directory, and is not needed anyway, since there aren't that many core modules, and any decent server can cope with loading all core modules. A better solution would be to make the .desc files optional (i.e. to be encouraged for use by all contrib modules), and to keep the 'admin/modules#description' but in hook_help for core modules. When loading the admin/modules page, Drupal can first look for a .desc file for each module, and if one is not found, then load the whole module file and call hook_help. Jaza.
On Saturday 21 January 2006 05:04, Karoly Negyesi wrote:
every core module would be bad - would clutter up the core /modules
There has been talk about giving every module its own dir, to put the module and all its assorted stuff there.
I currently install all contributed modules into their own respective directories, and leave the core modules in the main modules directory. In that way, I can easily tell which modules are core and which are contributed. -- Jason Flatt http://www.oadae.net/ jason@oadae.net
A better solution would be to make the .desc files optional (i.e. to be encouraged for use by all contrib modules), and to keep the 'admin/modules#description' but in hook_help for core modules. When loading the admin/modules page, Drupal can first look for a .desc file for each module, and if one is not found, then load the whole module file and call hook_help.
+1 This a path of least resistance appraoch. In the future, we could change the .desc files to .info, and have a lot more stuff in there (description, author, version, dependancies, ...etc.) Right now, a .desc scheme for contrib will solve the memory issue and avoid the need to load the module. It can even be scripted to generate the .desc from the existing _help now to give everyone a headstart.
On 21 Jan 2006, at 2:49 PM, Jeremy Epstein wrote:
I think a file with a particular extension would work well (e.g. modulename.desc - for 'description file'). But adding a .desc file for every core module would be bad - would clutter up the core /modules directory, and is not needed anyway, since there aren't that many core modules, and any decent server can cope with loading all core modules.
Wrong. All modules should be in their own directories to begin with. -- Adrian Rossouw Drupal developer and Bryght Guy http://drupal.org | http://bryght.com
I think a file with a particular extension would work well (e.g. modulename.desc - for 'description file'). But adding a .desc file for every core module would be bad - would clutter up the core /modules directory, and is not needed anyway, since there aren't that many core modules, and any decent server can cope with loading all core modules.
Wrong.
All modules should be in their own directories to begin with. I agree that all modules should have their own directory.
As far as the info file goes, we can have one for each module, starting with: DESC: This is the module description to show in admin/modules This can even be autogenerated for every existing module by running a one time script to extract that info from the module itself, similar to what Gerhard did. Later, we can add: AUTHOR: blah VERSION: blah DEPENDENCY: blat1 DEPENDENCY: blat2 DBCREATE: blah.sql ...etc. You get the idea.
On Jan 22, 2006, at 9:55 AM, Khalid B wrote:
As far as the info file goes, we can have one for each module, starting with:
DESC: This is the module description to show in admin/modules
This can even be autogenerated for every existing module by running a one time script to extract that info from the module itself, similar to what Gerhard did.
Later, we can add:
AUTHOR: blah VERSION: blah DEPENDENCY: blat1 DEPENDENCY: blat2 DBCREATE: blah.sql
...etc.
You get the idea.
This seems like a sensible direction. But consider using an ini file format: author = blah version - blah dependency = "blat1, blat2" (or all caps, if you like) parse_ini_file($filename) will make short work of converting this into a useful array. And it's already set up for different line endings, comment formatting (# or ;), and it matches what folks are already used to. Allie Micka pajunas interactive, inc. http://www.pajunas.com scalable web hosting and open source strategies
On 22 Jan 2006, at 6:10 PM, Allie Micka wrote:
This seems like a sensible direction. But consider using an ini file format:
author = blah version - blah dependency = "blat1, blat2"
We want to base it on the debian control file format variable: value I don't for instance see use using sections, nor do I think we should need to have comments, as users should NEVER edit this file. ever ever ever. It's meta-information, not configuration settings. And caps are bad. =) -- Adrian Rossouw Drupal developer and Bryght Guy http://drupal.org | http://bryght.com
On Jan 22, 2006, at 10:20 AM, Adrian Rossouw wrote:
We want to base it on the debian control file format
variable: value
I don't for instance see use using sections, nor do I think we should need to have comments, as users should NEVER edit this file. ever ever ever.
It's meta-information, not configuration settings.
But why would you start from scratch when a single function can do all the heavy lifting for you? The preference for a single character? ( : versus = ). It seems that development time could be put to better use than writing another file parsing routine. What are users? Aren't module developers users too? Why not give them a documented function to work against? Keep in mind that each file included adds cost. Usually 3-5msec, sometimes 10 or more per file. This is OS and configuration- dependent. If we're just talking about the modules page then that's fine, but files aren't free. Allie Micka pajunas interactive, inc. http://www.pajunas.com scalable web hosting and open source strategies
But why would you start from scratch when a single function can do all the heavy lifting for you? The preference for a single character? ( : versus = ). It seems that development time could be put to better use than writing another file parsing routine.
What are users? Aren't module developers users too? Why not give them a documented function to work against? My sentiments are similar. But, in the long run a non-exetuable meta-data file is a bit simpler - you can distribute only that file, and no trust between the parties is required to read it in.
I've implemented both kinds of meta-data file type, where the plain text is the origin. The php file is generated off the text, can be concatenated into one big thing and can be used uniformely.
On Jan 22, 2006, at 9:47 AM, Adrian Rossouw wrote:
On 21 Jan 2006, at 2:49 PM, Jeremy Epstein wrote:
I think a file with a particular extension would work well (e.g. modulename.desc - for 'description file'). But adding a .desc file for every core module would be bad - would clutter up the core /modules directory, and is not needed anyway, since there aren't that many core modules, and any decent server can cope with loading all core modules.
Wrong.
All modules should be in their own directories to begin with.
With or without separate directories, modules can share a desc/info file with: [user] desc = "Manages the user registration and login system." [watchdog] desc = "Logs and records system events." And so-on. Allie Micka pajunas interactive, inc. http://www.pajunas.com scalable web hosting and open source strategies
On 22 Jan 2006, at 6:15 PM, Allie Micka wrote:
[user] desc = "Manages the user registration and login system."
[watchdog] desc = "Logs and records system events."
And so-on.
Regardless of that. Modules should be in their own directories. -- Adrian Rossouw Drupal developer and Bryght Guy http://drupal.org | http://bryght.com
On 1/23/06, Piotr Krukowiecki <piotr@mallorn.ii.uj.edu.pl> wrote:
On Sun, Jan 22, 2006 at 06:23:40PM +0200, Adrian Rossouw wrote:
Regardless of that. Modules should be in their own directories.
Why?
Yeah... good question, I think I'll ask it too! Why?
Allie I agree with you that an ini format may be better than the hypothetical format I used, let us go for that instead. But I have to disagree that modules should share files. This brings us back to a centralized file that has to be updated (reminiscent of Windows registry). If we mandate that each module should have its own directory, then let each module get its own ini file (modulename.ini), and then we can have in it anything we like in the future. Module maintainers still distribute something that is self contained. The only snag is that core has to be reorganized to have directories. Now, all this would have to tie into the automated install that is under development (was it Vlado who was last working on it?). Are we discussing something that is opposed to what he is doing, or something that overlaps it, or something that compliments it?
On Jan 22, 2006, at 10:38 AM, Karoly Negyesi wrote:
If we mandate that each module should have its own directory
Yes, we do. So far, it seems only Steven does not agree with us in this.
Has anybody talked about using "functional" folders? All the required/core modules could live in "core" (block, filter, user, system; all the stuff drupal ships with) All the modules you download separately have their own folders Contrib/sub-modules live in directories *inside* of their parent module's folders (cck, ecommerce, etc.) I have had the hardest time supporting the e-commerce module, because you inevitably forget to enable cart, shipping, etc. Matt tells me that these used to be grouped on the modules page, and it wasn't always so confusing. CCK is doing similar sub-moduling, where you must scan up and down the modules page to find all the requisite field types. It would be useful to have these all appear on the modules page as grouped by the folders they appear in. As I'm thinking about this, it's getting down to two questions: 1) putting groups of modules into collective folders, and presenting them as grouped in such a manner 2) with or without these sub-folders, whether each module itself gets a folder. I guess my feelings on this aren't strong one way or the other. Allie Micka pajunas interactive, inc. http://www.pajunas.com scalable web hosting and open source strategies
On 1/22/06, Allie Micka <allie@pajunas.com> wrote:
On Jan 22, 2006, at 10:38 AM, Karoly Negyesi wrote:
If we mandate that each module should have its own directory
Yes, we do. So far, it seems only Steven does not agree with us in this.
Has anybody talked about using "functional" folders?
If you mean on the admin/modules and admin/settings pages, then yes, this is something I thought of. If we have "system" or "drupal", then groups that are categoriezed (much like RPMs on Red Hat for example: Internet, Games, Development, Office, ...etc.) So, if we have drupal, community features, ecommerce, admin tools, misc, ...etc. Again, this can be a settings in the modulename.ini file category = 'Site tools' The trick is what hierarchy we put in place, and agreeing on such a basic hierarchy. If you mean we group the directories to be like that, then I disagree. Modules already have a place under sites/default/modules, sites/sitehostname/modules or (if we implement it) sites/all/modules, each in its own directory. Things that depend on other modules (e.g. ecommerce contrib) can be under their main module.
On Sunday 22 January 2006 11:28, Khalid B wrote:
Has anybody talked about using "functional" folders?
If you mean on the admin/modules and admin/settings pages, then yes, this is something I thought of. If we have "system" or "drupal", then groups that are categoriezed (much like RPMs on Red Hat for example: Internet, Games, Development, Office, ...etc.)
So, if we have drupal, community features, ecommerce, admin tools, misc, ...etc.
Again, this can be a settings in the modulename.ini file
category = 'Site tools'
The trick is what hierarchy we put in place, and agreeing on such a basic hierarchy.
If you mean we group the directories to be like that, then I disagree. Modules already have a place under sites/default/modules, sites/sitehostname/modules or (if we implement it) sites/all/modules, each in its own directory. Things that depend on other modules (e.g. ecommerce contrib) can be under their main module.
Just an FYI, the sites/all suggestion from an earlier thread is already slated for inclusion early in 4.8. http://drupal.org/node/44920 That is step one of fully separating core and admin-added plugins. Step 2 would be the drupal/ or core/ or whatever directory that some have discussed. Since that requires a lot of moving stuff around in CVS, I think that really has to be done by Dries. That would also be the proper time to move all core modules into their own directories, which I agree is what we should be doing. That would then allow us to do a lot of other breakup and refactoring that needs to be done, for various reasons. I'm liking this ini file idea to replace/supplement hook_help(). You only need a little bit of metadata for admin/modules, not any of the functionality of the modules in question. Name, dependency information, description, etc. can all be put in there and parsed in a split second, even if they're then just put straight into the system table and pulled from there thereafter. Currently, as far as I am aware the system will parse any module file anywhere in the modules/ directory (and its kin). The file structure of the directory should reflect the packaging, not the dependencies. If a given tarball comes with modules modA, modB, and modC (like ecommerce), they should all be in the modules/mod/ directory. If another module, modD, is an extension to modB, it should say so in its metadata, not its file location. Consider that mod/ may be in sites/all, but you only want modD available on the sites/myexample.com/ site. You can't do that if it's filesystem dependent. So +1 to an ini file. +1 to every module having its own directory. Assume sites/all/ will happen, since Dries seems to like it as well. :-) -- Larry Garfield AIM: LOLG42 larry@garfieldtech.com ICQ: 6817012 "If nature has made any one thing less susceptible than all others of exclusive property, it is the action of the thinking power called an idea, which an individual may exclusively possess as long as he keeps it to himself; but the moment it is divulged, it forces itself into the possession of every one, and the receiver cannot dispossess himself of it." -- Thomas Jefferson
Karoly Surely it is not a cause worth dying for. Yes, we know that moving CVS in the backend is a pain, but that gets us over the problem of losing CVS history (one more argument for moving to SVN, IMHO). Can you please explain what other serious downsides of that approach? On 1/22/06, Karoly Negyesi <karoly@negyesi.net> wrote:
Step 2 would be the drupal/ or core/ or whatever directory that some have discussed.
And I oppose to death...
I'm not arguing for one approach or another, but do not let "difficulty" with CVS stop any such approach. CVS may seem difficult for the n00b, but I've done lots of moves and restructurings of CVS repositories. It's very possible if you know what you are doing. Khalid B wrote:
Karoly
Surely it is not a cause worth dying for.
Yes, we know that moving CVS in the backend is a pain, but that gets us over the problem of losing CVS history (one more argument for moving to SVN, IMHO).
Can you please explain what other serious downsides of that approach?
On 1/22/06, Karoly Negyesi <karoly@negyesi.net> wrote:
Step 2 would be the drupal/ or core/ or whatever directory that some have discussed.
And I oppose to death...
On 1/22/06, Chris Johnson <chris@tinpixel.com> wrote:
I'm not arguing for one approach or another, but do not let "difficulty" with CVS stop any such approach. CVS may seem difficult for the n00b, but I've done lots of moves and restructurings of CVS repositories. It's very possible if you know what you are doing.
+1.
On Sunday 22 January 2006 22:15, Khalid B wrote:
On 1/22/06, Chris Johnson <chris@tinpixel.com> wrote:
I'm not arguing for one approach or another, but do not let "difficulty" with CVS stop any such approach. CVS may seem difficult for the n00b, but I've done lots of moves and restructurings of CVS repositories. It's very possible if you know what you are doing.
+1.
Besides, only one person needs to know what they're doing with it, Dries or Steven. Everyone else can just pull down the new CVS head when they're done. -- Larry Garfield AIM: LOLG42 larry@garfieldtech.com ICQ: 6817012 "If nature has made any one thing less susceptible than all others of exclusive property, it is the action of the thinking power called an idea, which an individual may exclusively possess as long as he keeps it to himself; but the moment it is divulged, it forces itself into the possession of every one, and the receiver cannot dispossess himself of it." -- Thomas Jefferson
Khalid B wrote:
Allie
I agree with you that an ini format may be better than the hypothetical format I used, let us go for that instead.
Why would a registry style format be good? Why not use a format that native to php, like an associative array? We already do this with settings.php.
But I have to disagree that modules should share files. This brings us back to a centralized file that has to be updated (reminiscent of Windows registry).
+1 --Vernon
If we mandate that each module should have its own directory, then let each module get its own ini file (modulename.ini), and then we can have in it anything we like in the future. Module maintainers still distribute something that is self contained.
The only snag is that core has to be reorganized to have directories.
Now, all this would have to tie into the automated install that is under development (was it Vlado who was last working on it?). Are we discussing something that is opposed to what he is doing, or something that overlaps it, or something that compliments it?
I agree with you that an ini format may be better than the hypothetical format I used, let us go for that instead.
Why would a registry style format be good?
Just because we already have parsing functions, like Allie said. We will not even need the [section] thing. Just plain name = value pairs.
Why not use a format that native to php, like an associative array? We already do this with settings.php.
That is not a bad idea either. However, ini wins in the simplicity contest (readable to non-techie humans for example).
On Friday 20 January 2006 21:04, Adrian Rossouw wrote:
On 21 Jan 2006, at 7:26 AM, Gerhard Killesreiter wrote:
Attached is a patch that will make Drupal
1) read the module files 2) looking for help, init, exit hooks 3) write these hooks to a file 4) include that file 5) delete the file
Or you could skip all that and have a simple text file in the module directory that is parseable.
I don't believe there's a single good reason to have all the modules loaded just to get their descriptions.
There is currently an active discussion (for those of you who are not aware of it) on the documentation list about a similar issue. It might be a good thing to collaborate this effort to produce a single item that would work well for both the modules and the documentation. Here is the start of the thread: http://lists.drupal.org/pipermail/documentation/2006-January/003280.html -- Jason Flatt http://www.oadae.net/ jason@oadae.net
On Jan 20, 2006, at 11:26 PM, Gerhard Killesreiter wrote:
1) read the module files 2) looking for help, init, exit hooks 3) write these hooks to a file 4) include that file 5) delete the file
This would actually do well in core if you could get rid of the intermediate file-writing step. Why not just evaluate the $output after you've found the help/etc function?
Index: modules/system.module =================================================================== RCS file: /cvs/drupal/drupal/modules/system.module,v retrieving revision 1.279 diff -u -p -r1.279 system.module --- modules/system.module 17 Jan 2006 17:35:47 -0000 1.279 +++ modules/system.module 21 Jan 2006 04:31:23 -0000 @@ -898,9 +898,37 @@ function system_modules() {
ksort($files);
+ $tmpdir = file_directory_temp(); + foreach ($files as $filename => $file) { drupal_get_filename('module', $file->name, $file->filename); - drupal_load('module', $file->name); + // drupal_load('module', $file->name); + if (!function_exists($file->name .'_help')) { + $fp = fopen($file->filename, 'r'); + $help_found = FALSE; + $output = "<?php\n"; + while (!feof($fp)) { + $line = fgets($fp, 2048); + if (preg_match('/^function '. $file->name .'_(help|init| exit)[ (]/', $line)) { + $help_found = TRUE; + } + if ($help_found && preg_match("/^}/", $line)) { + $output .= $line; + $help_found = FALSE; + } + if ($help_found) { + $output .= $line; + } + } + $output .= "\n"; + fclose($fp); + $tmpfile = tempnam($tmpdir, $file->name .'.module.'); + $fp = fopen($tmpfile, 'w'); + fwrite($fp, $output); + fclose($fp); + include_once $tmpfile; + unlink($tmpfile); + }
$file->description = module_invoke($file->name, 'help', 'admin/ modules#description');
Allie Micka pajunas interactive, inc. http://www.pajunas.com scalable web hosting and open source strategies
Allie Micka wrote:
On Jan 20, 2006, at 11:26 PM, Gerhard Killesreiter wrote:
1) read the module files 2) looking for help, init, exit hooks 3) write these hooks to a file 4) include that file 5) delete the file
This would actually do well in core if you could get rid of the intermediate file-writing step. Why not just evaluate the $output after you've found the help/etc function?
Mainly because I didn't think that this would work. It appears to work, though. The page now loads with all contrib modules from HEAD (minus the ones I mentioned earlier) with the dreaded 8M memory limit. It is still an evil hack, though. The contrib modules violating the code conventions could be fixed, but those that call some internal functions to beautify their help output could not. Cheers, Gerhard
participants (14)
-
Adrian Rossouw -
Allie Micka -
Bèr Kessels -
Chris Johnson -
Earl Miles -
Gerhard Killesreiter -
Jason Flatt -
Jeremy Epstein -
Karoly Negyesi -
Khalid B -
Larry Garfield -
piotr＠mallorn.ii.uj.edu.pl -
Vernon Mauery -
vlado