Image module creates a cache bin then never uses it

um... is head broken or something cause most of those tests have nothing to do with images....

Just for reference as to how this happened git-bisect blames #851878: serve image derivatives from the same url they are generated from 7c57f94315b7

re: the update, i'd say we remove it at least in d8. if we aren't providing the cache_flush, then we don't own the table anymore and we shouldn't leave random tables laying around. I don't know how we'd review if contrib is using it in d7. I guess that would be the litmus for backporting?

Looks good to me, obviously for D8, but I also think that D7 should be safe. That is, with a probability of 99%.

Weird. The patch does not seem to apply.

deimos:drupal-head dries$ git apply ../f.p 
error: patch failed: modules/image/image.module:259
error: modules/image/image.module: patch does not apply

I just realized, shouldn't we actually remove the table?

euhm, indeed. ;)

Thanks!

Awesome, just wanted to make sure we didn't leave a table lying around for people upgrading :)
double plus rtbc now.

Committed to 8.x and moving to 7.x. for webchick. I didn't want to commit the d7 patch yet because the test bot hasn't run yet.

Re-uploading for 7.x testbot.

The additional image_update_7000() function needed to be renamed to image_update_7002().

#20: remove-image_cache-bin-1275684-20.patch queued for re-testing.

Re-rolled.

Bumping update function to 7005.

Hm. Is it safe to do that for D7?

What if a contributed or custom module (re-)uses this cache bin?

Trying to find a way to search through all contrib and custom modules to answer your question, @sun.

So I visited the project/issues page and scraped a list of the node ids for all projects, and now I'm visiting each node page and scraping the project name. Once that's done I'll use drush to dl the 7.x version of each and grep through the whole dirtree for 'cache_image' and let you know.

Should be done in another couple of hours.

Of course, that only tests for contrib modules published through drupal.org, but anything else they can just deal with it, as far as I'm concerned. People who write unpublished code that depends on undocumented unintentional cruft get what they deserve.

Downloading stalled. Maybe d.o is blocking me? Restarted...

Okay, here are all the places that reference cache_image:

Backup and Migrate

• backup_migrate_destination_db::backup_settings_default()
  Return the default tables whose data can be ignored. These tables mostly contain info which can be easily reproducted (such as cache or search index) but also tables which can become quite bloated but are not necessarily extremely important to back up or migrate during development (such ass access log and watchdog)
• _backup_migrate_default_structure_only_tables()
  Return the default tables whose data can be ignored. These tables mostly contain info which can be easily reproducted (such as cache or search index) but also tables which can become quite bloated but are not necessarily extremely important to back up or migrate during development (such ass access log and watchdog)

SimpleTest Clone

• SimpleTestCloneTestCase::$excludeTables
  Tables listed in this array will not have data copied, only table structure.
• SimpleTestCloneTestCase::cloneTable()
  

  protected function cloneTable($name, $source, $schema) {
    db_create_table($name, $schema);
  
    $target = Database::getConnection()->prefixTables('{' . $name . '}');
    if (!in_array($name, $this->excludeTables)) {
      // Explicitly name fields since the order of fields can vary
      // (particularly in CCK) from that specified in the schema array.
      $fields = implode(', ', array_keys($schema['fields']));
      db_query("INSERT INTO $target ($fields) SELECT $fields FROM $source");
    }
  }
  

SimpleTest Fixture

• simpletest_fixture_generate()
  

    $excludeTables = array(
  

  ...

      'cache_image',
  

  ...

      foreach ($schemas as $table => $schema) {    
  

  ...

        if (!in_array($table, $excludeTables)) {
          $source = $source_db_name .'.'. $sources[$table];
          db_query('INSERT INTO ' . $target . ' SELECT * FROM ' . $source);
        }
  

SimpleTest

• DrupalCloneTestCase::$excludeTables
  Tables to exlude during data cloning, only their structure will be cloned.
• DrupalCloneTestCase::cloneTable
  

  protected function cloneTable($name, $source, $schema) {
    db_create_table($name, $schema);
  
    $target = Database::getConnection()->prefixTables('{' . $name . '}');
    if (!in_array($name, $this->excludeTables)) {
      db_query('INSERT INTO ' . $target . ' SELECT * FROM ' . $source);
    }
  }
  

Organic groups

• (7.x-1.x) tests/drupal-7.og.update_7001.database.php
  Filled installation of Drupal 7, for test purposes.
• (7.x-2.x) tests/og-7.x-1.x.database.php
  Filled installation of Drupal 7.0, for test purposes.

@sun, did @pillarsdotnet's response answer your question?

Any way to search if a contributed module (re-)uses this cache bin? Must be an easy way to search for that.

As far as custom modules, that could be announced in the release notes.

I do not think we can remove (any) database tables in D7, since doing so falls under the API change + backwards-compatibility umbrella. We may be able to scan contrib, but we cannot scan custom code.

Ok, in that case, it looks like we can't fix this for D7.