As was mentioned in #1216824: drush @alias ublk/uublk commands do not work, the implicit 'name' attribute for site aliases is too generic, and can collide with command options that want to be called --name. It should be renamed to something more distinctive, like 'alias-name'.

CommentFileSizeAuthor
#9 name-for-aegir.patch1.85 KBgreg.1.anderson
#1 alias-name-drush5.patch5.13 KBgreg.1.anderson
#1 alias-name-drush4.patch4.98 KBgreg.1.anderson

Comments

greg.1.anderson’s picture

greg.1.anderson commented
Status: Active » Needs review
StatusFileSize
new alias-name-drush4.patch4.98 KB
new alias-name-drush5.patch5.13 KB

The attached patches rename 'name' to '#name' for drush4 and drush5. I picked '#name' instead of 'alias-name' because the site alias system already uses # to hide elements that contain metadata.

moshe weitzman’s picture

moshe weitzman commented
Title: Implicit site alias 'name' attribute should be renamed 'alias-name'. » Implicit site alias 'name' attribute should be renamed '#name'.
Status: Needs review » Reviewed & tested by the community

If the tests pass, this looks RTBC to me.

greg.1.anderson’s picture

greg.1.anderson commented
Status: Reviewed & tested by the community » Fixed

Committed to master and 7.x-4.x HEAD.

Status: Fixed » Closed (fixed)

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

omega8cc’s picture

omega8cc commented
Status: Closed (fixed) » Needs work

This breaks Aegir completely - see #1250068: Drush 4.5 breaks Aegir completely

Not sure what trick should we use for backward compatibility, so we can still support upgrades gracefully?

greg.1.anderson’s picture

greg.1.anderson commented

Yes, I admit it was against standard drush policy to backport this change to drush-4; however, the bug was fairly severe in that the internal-use 'name' attribute was polluting the environment of all drush commands, and causing problems with a number of them. I don't think there was a good way for us to fix this problem and maintain compatibility, and I think the change really needed to be made.

Could you revise aegir to test first for '#name' in the alias record, and then check 'name' if '#name' does not exist? I'm sorry for the inconvenience.

greg.1.anderson’s picture

greg.1.anderson commented

If this was found in drush-4.x (before drush-4.5 shipped), then I might also suggest that drush-4.x could write both #name and name into the alias record, but special-case 'name' so that it is not written into the environment ('alias' context). We could still do that now, but I don't know if it helps you if Aegir works with drush-4.4 and drush-4.6, but not drush-4.5, does it?

omega8cc’s picture

omega8cc commented

We didn't change anything yet and latest Aegir release (and HEAD) still requires drush-4.4. This means that if that could be 'fixed' in drush-4.6 for us, that would help for sure. In the meantime we will try to introduce some workaround as we did before for drush 3->4 upgrade path. Thank you for your help!

greg.1.anderson’s picture

greg.1.anderson commented
Version: » All-versions-4.x-dev
Status: Needs work » Needs review
StatusFileSize
new name-for-aegir.patch1.85 KB

Try this out and see if it works for Aegir. I expect this would be for drush-4.x only, and at some point in the future you would need to migrate to '#name' and leave drush-4.4 and earlier unsupported.

omega8cc’s picture

omega8cc commented
Status: Needs review » Needs work

Just tested this patch, but it doesn't help - install fails with the same errors, so probably it will not work due to this:

+  // 'name' is only here for Aegir compatibility; do not apply it to the alias context.
+  $skip_list[] = 'name';

It appears we rely on 'name' existing in the environment.

Full debug output: https://gist.github.com/1150262

So we probably need to introduce some workaround in Provision, as you suggested in #6 above.

greg.1.anderson’s picture

greg.1.anderson commented
Version: All-versions-4.x-dev »
Status: Needs work » Fixed

Yeah; we can't write 'name' into the alias context, because then it breaks a bunch of commands that take --name as an option. Sorry this wasn't fixed prior to drush-4.0.

Setting this back to 'fixed', as there's nothing more that we can do here.

Status: Fixed » Closed (fixed)

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