Problems with 'path-aliases' when an alias has a 'parent' item associate

In the original patch I try to correct a behavior related to the following property of the array_merge() function:

If the input arrays have the same string keys, then the later value for that key will overwrite the previous one.

In this new version of the patch, now also taking advantage of the difference between the functions array_key_exist() and isset()

isset() does not return TRUE for array keys that correspond to a NULL value, while array_key_exists() does.

Both quotes are from the manual of php.net.

I hope this helps someone.
Regards.

Hi Greg.
If there are common keys, array_merge_recursive() does not replace the values, instead transforms the common keys in arrays, where all the values of the above mentioned common keys are stored.

For example, if (in patch):

- $aliases[$alias_name]['path-aliases'] = array_merge($parent_record['path-aliases'], $alias_value_path_aliases);
+ $aliases[$alias_name]['path-aliases'] = array_merge_recursive($parent_record['path-aliases'], $alias_value_path_aliases);

and in @site2-live replace '%other-custom' by '%custom', then:

shell> dr sa @site2-live
$aliases['site2-live'] = array (
  'remote-host' => 'myserver.com.ar',
  'remote-user' => 'myuser',
  'root' => '/path/to/remote/drupal/root',
  'path-aliases' => 
  array (
    '%drush-script' => 'php /path/to/drush/drush.php',
    '%custom' => 
    array (
      0 => '/path/to/custom-server',
      1 => '/path/to/other/custom',
    ),
    '%dump' => '/path/to/live/sql_dump_site2.sql',
    '%files' => '/path/to/site2.com.ar/files',
  ),
  'uri' => 'site2.com.ar',
);

Therefore, this is not viable either:

- $aliases[$alias_name] = array_merge($parent_record, $aliases[$alias_name]);
+ $aliases[$alias_name] = array_merge_recursive($parent_record, $aliases[$alias_name]);

Which might be other arrays to which you refer?

If there might be other arrays in the alias record that need to be merged together, some options:
1.- wait for it: #791860: array_merge_recursive() is never what we want in Drupal: add a drupal_array_merge_recursive() function instead. and his backported,
2.- inspired by #791860 in 1, create drush_array_merge_recursive(),

Maybe useful function also in drush_command_invoke_all_ref().

Thanks for the explanation.

Now the patch also contains some other minor modifications (don't correspond specifically to this topic, but I don't believe that they deserve a new one) to avoid this:

// mygroup.aliases.drushrc.php is empty, but the group-alias is create
$aliases['mygroup'] = array (
  'site-list' => 
  array (
    0 => '',
  ),
);

// if %drush-script refers to drush.php and %drush isn't set, 
// the item '%drush' generated is incorrect 
shell> drush sa --with-optional @server
$options['site-aliases']['server'] = array (
  'remote-host' => 'myserver.com.ar',
  'remote-user' => 'myuser',
  'root' => '/path/to/remote/drupal/root',
  'path-aliases' => 
  array (
    '%drush-script' => 'php /path/to/drush/drush.php',
    '%custom' => '/path/to/custom-server',
    '%drush' => 'php /path/to/drush',
  ),
);

Ignore the patch in #6 please. It wasn't so simple :).

Is really neccesary special handling also for 'site-aliases' and 'command-specific' in the patch?
The parameters passed to the function _drush_sitealias_add_inherited_values() are preprocessed by _drush_sitealias_find_and_load_alias():

shell> grep --color -rn -B 5 _drush_sitealias_add_inherited_values .
./includes/sitealias.inc-334-      drush_log(dt('Load alias !alias', array('!alias' => $alias)));
./includes/sitealias.inc-335-      $aliasname = substr($alias,1);
./includes/sitealias.inc-336-      $result = _drush_sitealias_find_and_load_alias($aliasname, $alias_path_context);
./includes/sitealias.inc-337-      if (!empty($result)) {
./includes/sitealias.inc-338-        $alias_options = array('site-aliases' => array($aliasname => $result));
./includes/sitealias.inc:339:        _drush_sitealias_add_inherited_values($alias_options['site-aliases']);
--
./includes/sitealias.inc-353-  $result = _drush_sitealias_find_and_load_alias(NULL);
./includes/sitealias.inc-354-  if (!empty($result) && ($resolve_parent == TRUE)) {
./includes/sitealias.inc-355-    // If any aliases were returned, then check for
./includes/sitealias.inc-356-    // inheritance and then store the aliases into the
./includes/sitealias.inc-357-    // alias cache
./includes/sitealias.inc:358:    _drush_sitealias_add_inherited_values($result);
--

Extract of _drush_sitealias_find_and_load_alias():

        // If aliasname is NULL, then we will store
        // all $aliases into the alias cache
        if ($aliasname == NULL) {
          if (!empty($aliases)) {
            if (!empty($options)) {
              foreach ($aliases as $name => $value) {
                $aliases[$name] = array_merge($options, $value);
              }
              $options = array();
            }

            foreach ($aliases as $name => $value) {
              _drush_sitealias_initialize_alias_record($aliases[$name]);
            }

            $result = array_merge($result, $aliases);
          }
        }
        // If aliasname is not NULL, then we will store
        // only the named alias into the alias cache
        elseif ((isset($aliases)) && array_key_exists($aliasname, $aliases)) {
          drush_set_config_special_contexts($options); // maybe unnecessary
          $result = array_merge($options, $aliases[$aliasname]);
          _drush_sitealias_initialize_alias_record($result);
        }

And _drush_sitealias_find_and_load_alias() use drush_set_config_special_contexts(), an extract from the latter:

    // Copy site aliases and command-specific options into their
    // appropriate caches.
    foreach (array('site-aliases', 'command-specific') as $option_name) {
      if (array_key_exists($option_name, $options)) {
        $cache =& drush_get_context($option_name);
        $cache = array_merge($cache, $options[$option_name]);
        unset($options[$option_name]);
      }

The special handling is currently done, but the problem seems to be again the use of array_merge() on multidimensional associative arrays.

Mmmm, now ignore all in #7 please, except the conclusion.

Before the parent-child combination takes place, both 'command-specific' and 'site-aliases' are in their appropriate context.
This happens (in _drush_sitealias_add_inherited_values()) here:

$parent_record = drush_sitealias_get_record($parent);

I could verify it doing the following replace:

- $parent_record = drush_sitealias_get_record($parent);
+ $parent_record = _drush_sitealias_get_record($parent);

(drush_sitealias_get_record() calls drush_set_config_special_contexts(), but _drush_sitealias_get_record() does not) and then comparing the return of drush_get_context('command-specific') for each case.

The wrong combination of 'command-specific' values (when the alias has an 'parent') is apparently caused by the array_merge() in drush_set_config_special_contexts().

Greg, have you some opinion about this?
If the original patch fixes the original issue I really would appreciate to have the solution in HEAD.

Ok, I'll see what I can do about it in the coming days, with more time available.

A new and flexible helper function (drush_get_special_keys()) is added.

Some minor improvements in the documentation block.

On second thought, the $with_defaults parameter does not make much sense.
Changes in this patch:
- Remove the parameter $with_defaults, and
- Change the name of the parameter $more_keys by $extra_keys.

Commited with a slight modification, right?

Since there must be a special function, why not use it to avoid also this $array_based_keys = array_merge(drush_get_special_keys(), array('path-aliases')); in favour of $array_based_keys = drush_get_special_keys(array('path-aliases'));?

Thanks anyway.

Update: 'favour' instead of 'favor'.