Drush bug boiled down revealing drupal core bug

Further explanation:

problem only in MAINTENANCE_MODE

$ drush updatedb

bootstrap...
  ./includes/theme.inc: list_themes()
    if MAINTENANCE_MODE $themes = _system_theme_data();
    if (!isset($theme->status)) $theme[*]->status = 0;

calling ./modules/system/system.module: system_theme_data()
   // or is that that not allowed in MAINTENANCE_MODE, in which case the error would be in drush


---------------------- from ./includes/theme.inc (version 6.16) ------------------------------
function list_themes($refresh = FALSE) {
  static $list = array();
[...]
    if (db_is_active() && !defined('MAINTENANCE_MODE')) {
[...]
    }
    else {
      // Scan the installation when the database should not be read.
      $themes = _system_theme_data();
    }

    foreach ($themes as $theme) {
[...]
### This is the ***MAIN PROBLEM***
### Despite the 'MAINTENANCE_MODE' handling inconsitency
### If I comment out that bit of code, then the issue is non-existant
      // Status is normally retrieved from the database. Add zero values when
      // read from the installation directory to prevent notices.
      if (!isset($theme->status)) {
        $theme->status = 0;
      }
[...]
    }
[...]
  
  return $list;
}

--------------------------------------------------------------------------------------------------------

---------------------- from ./modules/system/system.module (version 6.16) ------------------------------
function system_theme_data() {
  // Scan the installation theme .info files and their engines.
  ### second call, now the static array is initialized and status=0
  $themes = _system_theme_data();     
  
  // Extract current files from database.
  ### PROBLEM: reads status from the database (despite MAINTAINANCE_MODE ... ???)
  ### but does NOT assign $theme[*]->status, as it's already set to zero
  system_get_files_database($themes, 'theme'); 
  
  db_query("DELETE FROM {system} WHERE type = 'theme'");
  
  foreach ($themes as $theme) {
    if (!isset($theme->owner)) {
      $theme->owner = '';
    }

    db_query("INSERT INTO {system} (name, owner, info, type, filename, status, throttle, bootstrap) VALUES ('%s', '%s', '%s', '%s', '%s', %d, %d, %d)", $theme->name, $theme->owner, serialize($theme->info), 'theme', $theme->filename, isset($theme->status) ? $theme->status : 0, 0, 0);
  }

  return $themes;
}

[...]

function system_get_files_database(&$files, $type) {
  // Extract current files from database.
### doesn't care about MAINTENANCE_MODE ???
  $result = db_query("SELECT filename, name, type, status, throttle, schema_version FROM {system} WHERE type = '%s'", $type);
  while ($file = db_fetch_object($result)) {
    if (isset($files[$file->name]) && is_object($files[$file->name])) {
      $file->old_filename = $file->filename;
      foreach ($file as $key => $value) {
### IS already set to 0 and doesn't get assigned with the (correct) value from the database
        if (!isset($files[$file->name]) || !isset($files[$file->name]->$key)) {
          $files[$file->name]->$key = $value;
        }
      }
    }
  }
}

[...]

function _system_theme_data() {
### STATIC !!!
  static $themes_info = array();

  if (empty($themes_info)) {
    [...]
    [fill in $themes_info from .info files]
    [...]
  }
  
  return $themes_info;
}
-------------------------------------------------------------------------------------------------------------------------------

fine ... the end of the day - the problematic code - imho - is
- in ./includes/theme.inc:list_themes()
    if the database is in MAINTENANCE MODE, the themes status gets initialized to zero
- in ./modules/system/system.module:system_get_files_database()
    here - in contrast to list_themes - MAINTENANCE MODE is not handled specially
- in ./modules/system/system.module:system_theme_data()
    that function, of course, also doesn't care about maintenance mode

so basically: what's that MAINTENANCE_MODE all about anyway - anybody knows the definition ? are you still alowwed to touch the databse? or just readonly?

besides: what about themed maintenance mode notifications to visitors. I think I read something about that somewhere.
Is it actually necessary to go back to garland theme before putting the site offline (==MAINTENANCE_MODE ???) or can i stick with my theme?

:-) ... i'm a bit lost

greets

patrick

http://drupalcode.org/viewvc/drupal/drupal/includes/theme.inc?r1=1.399&r...

Now I'm getting a little mixed up with the meaning and function of "maintenance mode" (php runtime define()d constant set in update.php and install.php) and "site offline" (databse variable set by the admin thru the web at "q=admin/settings/site-maintenance" - in drupal 7 renamed to "q=admin/settings/maintenance-mode" (not really making me less confused also)

Well ... learning ... :-)

Sorry to be so noisy!

found an interresting comment in ./includes/common.inc:drupal_flush_all_caches()

  // If invoked from update.php, we must not update the theme information in the
  // database, or this will result in all themes being disabled.
  if (defined('MAINTENANCE_MODE') && MAINTENANCE_MODE == 'update') {
    _system_theme_data();
  }
  else {
    system_theme_data();
  }

So maybe drush should do the same... !!?

lets see...

Well ... of course it should! That would solve the problem.

The only thing is that this won't be very easy to do in drush since system_theme_data is not called directly from the update_main() function but thru some other ("high-level", "drush library") function, which doesn't distinguish between MAINTENANCE_MODE and not ~.

So either drush will have to check there (in it's "library function" (drush_get_projects(),drush_get_themes())) again, or, and that's what I'd suggest - out of the blue,

the test for MAINTENANCE_MODE could go into system_theme_data() so the callers wouldn't have to bother about calling _system_theme_data() in case of drupal beeing in MAINTENANCE_MODE.

What does anybody else think ???

Suggesting this patch (for drupal 6.16)

Would fix (tested) the drush issue but I haven't checked what consequences (performance overhead calling theme data ... ? - if at all, then probably minimal - don't think the test takes very long) it might have on other drupal core parts or modules.

patch -p0 -adjusted

Ok - maybe that's the right format also ... I'm desperately sorry and am going the read the introduction/manual/documentation right NOW ...

Read http://drupal.org/patch/create ... wonder what I might have missed ...

diff example

1. Make sure you have the latest copy of the file(s) by downloading the latest tarball.
2. Create a working copy of the file you are editing in the same directory as the original (e.g. example.module copied to exampleNew.module).
3. Edit the working (exampleNew.module) copy, and save it.
4. When you are ready to roll your patch, go to the command line and change (cd) to the proper "root" directory.
5. Enter this command: diff -up path/to/file/example.module path/to/file/exampleNew.module > mypatchname.patch. This creates a new patch file in the current directory that you can now attach to a reply in a Drupal project's issue thread.

Another try...

qa.drupal.org only tests against HEAD, doesn't care about version - right ?

last try