See the blog post on this subject:

http://blog.riff.org/2013_01_25_when_drush_make_fails_to_apply_patches

It seems that Drush should either recognize the short-form version option when extended-format options are also used, or it should fail with an error. I would guess that it would be common for folks to start out with makefiles where all package declarations are in the short format, and later add more options that refer to the existing package. It is mysterious that adding to a working declaration should change its behavior.

CommentFileSizeAuthor
#2 merge-mixed-type-ini-items.patch1.19 KBgreg.1.anderson

Comments

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

This seems like a limitation of the .ini format. It's basically trying to mix strings and arrays, similar logic in php results in garbage:

php > $foo = 'bar';
php > $foo['baz'] = TRUE;
PHP Warning:  Illegal string offset 'baz' in php shell code on line 1
PHP Stack trace:
PHP   1. {main}() php shell code:0
php > var_dump($foo);
string(3) "1ar"
greg.1.anderson’s picture

greg.1.anderson commented
Status: Active » Needs work
StatusFileSize
new merge-mixed-type-ini-items.patch1.19 KB

Hm, yeah, it looks like make is using _drush_drupal_parse_info_file() to do the work of parsing a makefile, so this would be a little bit inconvenient to fix. Since we already have our own copy of this function, though, it would be possible to fix it if we desired to. We could consider adding a parameter to _drush_drupal_parse_info_file() that indicated the name of the field that overlapping data items would be merged into. See attached patch.

Then:

$ drush ev 'return _drush_drupal_parse_info_file("projects[domain] = 3.7\nprojects[domain][patch][] = http", "version");'
array (
  'projects' => 
  array (
    'domain' => 
    array (
      'version' => '3.7',
      'patch' => 
      array (
        0 => 'http',
      ),
    ),
  ),
)

Of course, if we did this, then make files wouldn't really be .info files any longer. On the other hand, one could argue that make files already are not, strictly speaking, info files, since Drush make has already introduced the concept that a given data item might be an array or a simple string.

To continue with this patch, we could either trivially modify make_parse_info_file(), like so:

    $info = _drush_drupal_parse_info_file($data, 'version');

Or, more robustly, we could use something else, such as '__DRUSH_MAKE_MERGE__' to hold the merged items, and write extra code to map this constant to 'version' when it appears in the correct context, or fail with an error when it does not. It might also be necessary to adjust the code that fills in the default values; I didn't check this.

It'd also be okay to "won't fix" this, if you prefer, although I suspect it would be very helpful to folks if make handled this edge case correctly.

andreiashu’s picture

andreiashu commented

There's also the option for drush to throw an error in this case explaining what is happening. This way people will know what they have to do in order to not get an unexpected behaviour.

greg.1.anderson’s picture

greg.1.anderson commented

Without a change similar to #2, an error would be impossible to detect, as conflicts are silently overwritten by the .info parsing code. #2 could be simplified to detect but not correct conflicts, though.

greg.1.anderson’s picture

greg.1.anderson commented
Status: Needs work » Closed (won't fix)
Issue tags: +Needs migration

This issue was marked closed (won't fix) because Drush has moved to Github.

If desired, you may copy this bug to our Github project and then post a link here to the new issue. Please also change the status of this issue to closed (duplicate).

Please ask support questions on Drupal Answers.