$ drush pm-enable devel zen garland
garland is already enabled.                                                                                        [ok]
The following projects will be enabled: devel, zen
Do you really want to continue? (y/n): y
devel was enabled successfully.                                                                                    [ok]
zen was enabled successfully.                                                                                      [ok]

$ drush pm-disable devel zen garland
garland is the default theme and can't be disabled.                                                                [ok]
The following projects will be disabled: devel, zen
Do you really want to continue? (y/n): y
devel was disabled successfully.                                                                                   [ok]
zen was disabled successfully.                                                                                     [ok]

Changes:
1. Now drush_get_projects(), drush_get_themes() and drush_get_modules() features project caching with a static variable and a new argument `$refresh = FALSE`.
2. drush_get_themes() is the same in all drupal versions. moved to environment.inc

New functions:
3. drush_theme_enable() and drush_theme_disable()
4. drush_theme_get_default().

Other cosmetic changes:
5. 

<       drush_die('Aborting.');
---
>       drush_log(dt('Aborting.'));
>       return;

notes:

I've been unable to make this work although the query is correct (at least for mysql):
db_query("UPDATE {system} SET status = 0 WHERE type = 'theme' AND name IN ('%s')", array(implode("','", $themes)));
so I use a loop and run the query for each theme.

for D7, PDO likes ->condition('name', $themes, 'IN').

CommentFileSizeAuthor
#6 693958.patch25.09 KBjonhattan
#4 pm-themes.patch31 KBjonhattan
#3 pm-themes.patch31.01 KBjonhattan
#2 pm-themes.patch30.4 KBjonhattan

Comments

moshe weitzman’s picture

moshe weitzman commented

That query needs http://api.drupal.org/api/function/db_placeholders/6. you need a similar technique in d5, but that function is not available.

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
StatusFileSize
new pm-themes.patch30.4 KB

rerolled with:
1. db_placeholders()
2. remove now unneeded 2nd argument of update_check_incompatibility()
3. no use a static for drush_get_projects() as suggested on irc by adrian

<Vertice> our big issue in drush is memory usage
<Vertice> so i'd support a drush_get_project($refresh)
<Vertice> and then pass the $refresh parameter down
<Vertice> to drush_get_modules and drush_get_themes
<Vertice> so basically
<Vertice> make drush_get_projects :
<Vertice> return array_merge(drush_get_modules($refresh), drush_get_themes($refresh));
jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
StatusFileSize
new pm-themes.patch31.01 KB

New patch include two lines to drush status:

$drush status theme
 Default theme          :  garland                 
 Administration theme   :  seven
jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
StatusFileSize
new pm-themes.patch31 KB

I've found theme_enable() and theme_disable() for D7. Fixes also a typo in pm-list.

moshe weitzman’s picture

moshe weitzman commented

I'm a bit worried about those statics getting stale in drupal_get_modules and drupal_get_themes. drupal already caches this info pretty well. i'd like to understand why we can't just call into drupal every time. it sounds like adrian ses this as a memory win but I don't know why.

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented
StatusFileSize
new 693958.patch25.09 KB

`drush_get_themes()` can use native drupal cache with `list_themes($refresh)` across all drupal versions with no overhead.

For modules: I don't see any caching (with a static variable) for `module_rebuild_cache()` (D5,6) or `system_rebuild_module_data()` (D7). There is `module_list()` for all versions but it only list *enabled* modules *name*.

I introduced this cache thinking of performance/speed. I don't think adrian see this as a memory improvement. He just told me to not use also a static for drush_get_projects(), and this is a memory win between my patchs in #0 and #1.

The rationale of this cache is we dont need a rebuild each time we want the list of projects and also using a cache for `drush_get_modules()` allow us to decouple several functions within pm.drush.inc and environment_x.inc by avoid passing $module_info as an argument.
Note there was the same caching mechanism already present in `update_check_incompatibility()`. Patch in #4 shorten this function with no drawback.

Also issue #535788: Check sites/all/themes for *.drush.inc files uses `drush_get_projects()` in `_drush_find_commandfiles()` when DRUSH_BOOTSTRAP_DRUPAL_FULL. It was not the best choice btw.

This is the result of running drush pm-list with and without cache 3 consecutive times for each:

without cache:
Peak memory usage was 11.71 MB [1.34 sec, 10.93 MB]
Peak memory usage was 11.71 MB [1.39 sec, 10.93 MB]
Peak memory usage was 11.71 MB [1.33 sec, 10.93 MB]

with cache:
Peak memory usage was 11.67 MB [0.95 sec, 11.29 MB]
Peak memory usage was 11.67 MB [0.91 sec, 11.29 MB]
Peak memory usage was 11.67 MB [0.98 sec, 11.29 MB]

As spected without cache it run faster. It's also spected that with-cache memory usage at the end is bigger (11.29 > 10.93). Still don't understand why peak usage is bigger. I'll do my tests in deeper and open other issues just in case.

Current patch does not include any, as it is not the target of the issue.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Fixed

Committed. Thx.

Feel free to submit follow up patches. Your cache explanation makes good sense.

Status: Fixed » Closed (fixed)

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