SelectQueryExtender->hasAllTags method uses syntax not accessible to PHP 5.2.x installations, which violates
Drupal 7 base requirement for PHP version. Simple change fixes the problem. Patch attached.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dadikof’s picture

FileSize
478 bytes

Generated patch used filesystem absolute paths, updated. No experience with Drupal patches, sorry :)

aspilicious’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, select.patch, failed testing.

aspilicious’s picture

See http://drupal.org/node/707484 and have fun :)

aspilicious’s picture

Priority: Critical » Major

Btw this is not critical...

chx’s picture

Issue tags: +Needs tests

The patch is good but needs tests.

xjm’s picture

Version: 7.10 » 8.x-dev
Issue tags: +Needs backport to D7

Also, let's create a D8 version. Thanks!

Niklas Fiekas’s picture

Issue tags: -Needs tests

We already have test coverage:

  /**  
   * Test query tagging "has all of these tags" functionality.
   */
  function testHasAllTags() {
    $query = db_select('test');
    $query->addField('test', 'name');
    $query->addField('test', 'age', 'age');

    $query->addTag('test');
    $query->addTag('other');

    $this->assertTrue($query->hasAllTags('test', 'other'), t('hasAllTags() returned true.'));
    $this->assertFalse($query->hasAllTags('test', 'stuff'), t('hasAllTags() returned false.'));
  }

Only, we didn't notice because the testbots run PHP > 5.3.

 

Obviously there is more usage of func_get_args(). Not so obviously, but also not surprising: Some more wrong usage.

./includes/form.inc: $args = func_get_args();
./includes/form.inc: $args = func_get_args();
./includes/cache.inc: if (func_get_args()) {
./includes/bootstrap.inc: return drupal_array_merge_deep_array(func_get_args());
./includes/bootstrap.inc: $args = func_get_args();
./includes/module.inc: $args = func_get_args();
./includes/module.inc: $args = func_get_args();
./includes/file.inc: $args = func_get_args();
./includes/database/select.inc: return call_user_func_array(array($this->query, 'hasAllTags'), func_get_args());
./includes/database/select.inc: return call_user_func_array(array($this->query, 'hasAnyTags'), func_get_args());
./includes/database/select.inc: return !(boolean)array_diff(func_get_args(), array_keys($this->alterTags));
./includes/database/select.inc: return (boolean)array_intersect(func_get_args(), array_keys($this->alterTags));

./includes/database/schema.inc: $args = func_get_args();
./includes/database/sqlite/database.inc: $args = func_get_args();
./includes/database/sqlite/database.inc: $args = func_get_args();
./modules/system/system.tar.inc: $v_att_list = &func_get_args();
./modules/simpletest/tests/database_test.test: $modules = func_get_args();
./modules/simpletest/tests/ajax.test: $modules = func_get_args();
./modules/simpletest/tests/test.php: call_user_func_array('dump', func_get_args());
./modules/simpletest/tests/menu.test: $modules = func_get_args();
./modules/simpletest/tests/menu.test: $modules = func_get_args();
./modules/simpletest/tests/menu.test: $modules = func_get_args();
./modules/simpletest/drupal_web_test_case.php: $modules = func_get_args();
./modules/field/tests/field.test: $modules = func_get_args();
./modules/field/tests/field.test: $modules = func_get_args();
./modules/field/tests/field_test.field.inc: $args = func_get_args();
./modules/field/tests/field_test.field.inc: $args = func_get_args();
./modules/field/tests/field_test.field.inc: $args = func_get_args();
./modules/field/tests/field_test.field.inc: $args = func_get_args();
./modules/field/tests/field_test.field.inc: $args = func_get_args();
./modules/field/tests/field_test.module: $args = func_get_args();
./modules/field_ui/field_ui.test: $modules = func_get_args();
./modules/field_ui/field_ui.module: $args = array_slice(func_get_args(), 4);
./modules/image/image.module: $args = func_get_args();
./modules/file/file.module: $form_parents = func_get_args();
./modules/file/tests/file.test: $modules = func_get_args();

 

This sucks. Should we really fix that for D8? What PHP version will it require?

xjm’s picture

@Niklas Fiekas pointed out that this is specific to PHP 5.2 only, so a D8 patch may not make sense. Doh! Sorry about that. Whether we change the syntax in D7 only or in both branches for all this? Tough call.

jthorson pointed out it's not technically feasible to test patches on PHP 5.2 at present, either, so tough to sort out what to do about testing.

webchick’s picture

Version: 8.x-dev » 7.x-dev
Issue tags: -Needs backport to D7

I think it's fine to fix these for D7 as long as the fixes are also PHP 5.3 compatible. No reason to do this in D8 though, unless Crell really wants to keep the code bases in sync.

Niklas Fiekas’s picture

Title: PHP Fatal error: func_get_args(): Can't be used as a function parameter in hasAllTags » func_get_args() can't be used as a function parameter before PHP 5.3
Status: Needs work » Needs review
Issue tags: +Needs manual testing
FileSize
2.41 KB

The attached patch converts all occurences of a wrong func_get_args(). Setting to a more general title. Let's see what the testbot says, but this shouldn't break anything.

To verify that it actually fixes the problem, we need manual testing, as jthorson pointed out. Running the core testsuite on a PHP 5.2 machine should be sufficient but painful.

droplet’s picture

Status: Needs review » Reviewed & tested by the community

Looks good. PHP 5.2 and below don't support Closure.

Is there any plan / ref about how to use closure/anonymous function in D8 ? (http://php.net/manual/en/functions.anonymous.php)

chx’s picture

While #12 looks a bit off topic to me, http://php.net/manual/en/function.func-get-args.php indeed says

5.3.0 This function can now be used in parameter lists.

So the patch is good / unnecessary for D8.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Thanks folks! Committed and pushed to 7.x.

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