Take account of drupal_hash_salt during migration path from 7.x

Yeah, I definitely think we do, unless it's absolutely impossible.

You can't write a test that changes settings.php.

you wanted if (!empty($settings)) drupal_rewrite_settings($settings); to avoid a notice and needless work.

+++ b/core/modules/system/system.installundefined
@@ -2195,6 +2195,38 @@ function system_update_8051() {
+    $variable = update_variable_get($variable_name, '');

update_variable_get() does not consider $conf overrides. So this doesn't work for things that were defined in $conf only. There will also be things that aren't $conf at all, e.g. $drupal_hash_salt.

We either need to change that or check it ourself. Also not sure what to do with the existing $conf snippets in settings.php that we're replacing with this.

Hmm I actually think we don't need an upgrade path for this - if you're running a site which needs them, then you're capable of updating them yourself - they're code rather than user data.

Not changing status and others might feel differently, but fwiw.

I'm not sure myself, but at least *some* of them need an upgrade path. For example, drupal_hash_salt and $databases, if we change them to $settings too.

OK those two do indeed and it'd be great to move those two to $settings as well so well kill off globals altogether (except maybe $conf).

settings.d7.php you mean, but yes, I might take this one if noone else does; the database upgrade will need special casing in the new supersimple drupal_rewrite_settings function ;)

http://drupal.org/node/1427826 has instructions for updating issue summaries and using the issue summary template

using the template might be helpful, also looks like some good comments to incorporate there.

Issue summary updated @ Drupalcon Portland

Updated issue summary.

removing needs tag

#11: 1921822-settings-upgrade-path-11.patch queued for re-testing.

Tagging all critical D8 upgrade path issues as "beta blocker."

Since the migration based API will use a fresh 8.x install to migrate into, there's no need to migrate $databases, and anything else in settings.php can be configured by the person setting up the new site.

Except for drupal_hash_salt - it's absolutely necessary that old passwords continue to work etc. so that needs to be an explicit step. I think we can either require that it's copied across manually, or rewrite it if we can - but one or the other has to happen.

Postponed on at least the initial framework from #1052692: New import API for major version upgrades (was migrate module).

Why not keep automated upgrades as much as possible? Not all site should require manual settings and manual database migration.

I still believe that many Drupal sites upgrading to Drupal 8 would still benefits from using the regular upgrade steps and use update.php instead of doing everything manually.

formatting

Still important to figure out, but no longer blocks release since migratation path doesn't.

I think this has been fixed with #2125717: Migrate in core: patch #1

Are folks agreed that this is fixed with #2125717: Migrate in core: patch #1?

Migration from Drupal 7 has not yet been committed to core - there's been some work on it in the IMP sandbox, but I don't see anything involving drupal_hash_salt. And I don't see how it could do anything, since drupal_hash_salt is defined in the legacy site's settings.php, not in the database we're migrating from - the D8 instance has no way to automatically obtain the value. There's going to need to be a manual step documented, methinks... At the very least, perhaps instructions to the upgrader to copy the D7 drupal_hash_salt and paste into the migrate_upgrade form.

See #2397849: Export migration configuration via services for a possible approach - a contrib-implemented service on D7 to provide drupal_hash_salt (and any other code-based configuration).

This is probably no longer postponed.

No need for the upgrade path tag here.

This is not part of the configuration system.

Rerolled #11.

This issue needs a completely different direction from #41 - that was based on a time when we upgrade d7 to d8 in place.

Except for drupal_hash_salt - it's absolutely necessary that old passwords continue to work etc. so that needs to be an explicit step. I think we can either require that it's copied across manually, or rewrite it if we can - but one or the other has to happen.

Discussed with @alexpott, @weal, @mikeryan, and @benjifisher. This is actually a duplicate of #2598038: Invalid passwords after D7 to D8 migration -- that issue cannot be resolved without this, and it doesn't make sense to discuss the solution separately.

The rerolled patch in #41 is not relevant as @alexpott says.

We need to work out if we care if a Drupal 7 site that is being migrated to Drupal 8 has a different hash salt in settings.php. (Note that user passwords do not use this salt). This salt is used to generate things like CSRF tokens - or things like this:

function google_analytics_user_id_hash($uid) {
  return Crypt::hmacBase64($uid, \Drupal::service('private_key')->get() . Settings::getHashSalt());
}

function _update_manager_unique_identifier() {
  $id = &drupal_static(__FUNCTION__, '');
  if (empty($id)) {
    $id = substr(hash('sha256', Settings::getHashSalt()), 0, 8);
  }
  return $id;
}

If a site migrates it's hash salt then we can determine who is moving from 7 to 8 on d.o...

However given the difficulties of doing anything automatic - a migration cannot have access to both settings.php - I think this issue might be a docs issue.

Adding tag for Drupal source and destination version.

@alexpott: Any updated thoughts on this? Just docs, or is there anything else to do here?

Thanks.

Postponing on needing more info to proceed.

Seems like a duplicate. If it isn't, please re-open and provide next steps.