Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The --local flag disables too much #1340

Closed
wants to merge 2 commits into from
Closed

Conversation

greg-1-anderson
Copy link
Member

The --local flag has a couple of problems.

  1. It disables too much.

It is necessary to disable the global locations for Drush commandfiles when using a site-local Drush; however, it is often harmful, and never (rarely?) helpful to disable a user's global configuration or alias files. Even if the site-local Drush provides standard configuration and aliases for the team, it is too restrictive to prevent individual engineers from using their personal customizations in the team environment. There was some talk about having special "local" global configuration, where engineers could put in the customizations that they think are "safe" for use in teams. However, in absence of any compelling examples of why custom shell aliases, command-specific-options and so on would be harmful in team settings, I think it is simpler and better to allow the configuration options to merge together, as the currently do, and disable only global commandfiles.

  1. It conflicts with existing options.

The 'drupal-directory' (dd) and 'site-aliases' (sa) commands previously defined --local options, to filter out paths and sites that are defined only on remote systems. With the global --local option in place, the alias records used with, for example, sa --local are not read, rendering that feature unusable.

This PR renames the global --local option to --site-local, and removes the conditionals that disable global aliases and configuration when in site-local mode. It shouldn't matter if this option's name is a little longer, as it should rarely be typed by hand -- usually it is included in an alias (e.g. a site-local 'dr' bash alias), or included automatically by Drush when doing a site-local redispatch (when $options['drush-script'] set in a site-local drushrc.php configuration file).

…tion for drupal-directory (dd) and site-alias (sa) commands.
@greg-1-anderson
Copy link
Member Author

We discussed 1) on Friday, and did not come to any specific conclusion. If --local does not turn off user preferences, then in a team setting you run the risk of of encountering a "works on my machine" scenario, where one user's individual config may break -- or fix -- the global team configuration settings.

It seems to me that this concern is slightly different than the original motivation for --local, which was to prevent conflicts between global and local commandfiles. Most of the time, when Drush is sharing an autoloader with Drupal, you do not want Drush to include autoloaders from any other commandfiles; it is better if the user's global commands are also required in the composer.json of the Drupal site, so that all of the dependencies are evaluated together, and used to produce a single autoloader. We have an option for this rather than doing it by default because some users want to assume the risk (of autoloader conflict), and continue to manage their global commands globally. Two options seems a bit heavyweight, but perhaps it is what is needed here.

We did not discuss 2), but this conflict should be resolved one way or another.

…ble searching in global commandfile locations. Make the --site-local option disable global alias and settings locations.
@greg-1-anderson
Copy link
Member Author

The most recent commit on this PR takes a slightly different tack.

Now, Drush auto-detects if it is in site-local Drush mode -- that is, if it shares an autoloader with a Drupal site. If it is, it disables loading commandfiles from global locations. The --local flag is renamed to --site-local, and also disables global locations for configuration files and site aliases.

@@ -386,7 +389,8 @@ function _drush_find_commandfiles_drush() {
}
}

if (!drush_get_option('local')) {
// In site-local Drush mode, we ignore global commandfiles.
if (!SITE_LOCAL_DRUSH && !drush_get_option('site-local')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks OK but its going to annoy folks whose team uses site-local drush but they still want to use global commands. I guess those folks could use configuration to add another search path that points to global commands? if so, this PR looks good.

@greg-1-anderson
Copy link
Member Author

We don't need to fix all of this for Drush 7, but we do want to disambiguate the option.

--local should be the local option.

--local-only should be the command-specific option.

@greg-1-anderson
Copy link
Member Author

The conflict in option names was fixed by #1382. The remaining tasks in this issue are not blockers for Drush 7, so removing the Drush 7 tag.

@greg-1-anderson greg-1-anderson removed this from the Drush7 milestone May 14, 2015
@weitzman weitzman changed the title The --local flag disables too much, and conflicts with other command options The --local flag disables too much May 18, 2015
@weitzman
Copy link
Member

weitzman commented Jan 2, 2018

Closing due to old age.

@weitzman weitzman closed this Jan 2, 2018
@weitzman weitzman deleted the site-local branch January 2, 2018 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants