Problem/Motivation
Tried a vanilla D6 migration to 8.1.x. No CCK, nothing.
$ drush @d8.local migrate-upgrade --legacy-db-url=mysql://root:root@localhost:3306/d6 --legacy-root=http://d6.local
Returned:
(snipped)
Upgrading d6_node:page
SQLSTATE[42S02]: Base table or view not found: 1146 Table 'd6.content_node_field' doesn't exist: SELECT COUNT(*) AS expression [error]
FROM
(SELECT DISTINCT cnf.field_name AS field_name, cnf.type AS type, cnf.global_settings AS global_settings, cnf.required AS required,
cnf.multiple AS multiple, cnf.db_storage AS db_storage, cnf.module AS module, cnf.db_columns AS db_columns, cnf.active AS active,
cnf.locked AS locked, 1 AS expression
FROM
{content_node_field} cnf
INNER JOIN {content_node_field_instance} cnfi ON cnfi.field_name = cnf.field_name) subquery; Array
(
)
Which I *thought* had been fixed as part of #2561697: Migration should not choke when the content_node_field table isn't available. Enabling CCK's content module fixes the issue, but we shouldn't be assuming it's there.
Proposed resolution
Make sure CCK's content module isn't required for a default migration to run successfully
Remaining tasks
There was possibly a regression after #2561697: Migration should not choke when the content_node_field table isn't available - Review and or git bisect and fix what needs to be fixed.
User interface changes
None.
API changes
None.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#12 | interdiff.txt | 2.46 KB | vasi |
#12 | 2710133-12.patch | 7.31 KB | vasi |
#8 | 2710133-8.patch | 6.84 KB | vasi |
#8 | 2710133-8.fails_.patch | 5.62 KB | vasi |
Comments
Comment #2
anavarreComment #3
catchComment #4
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #5
douggreen CreditAttribution: douggreen at Tag1 Consulting commentedComment #6
vasi CreditAttribution: vasi at Evolving Web commentedI tried this with both migrate_drupal_ui and migrate_upgrade. Strangely, it only fails with migrate_upgrade!
Comment #7
vasi CreditAttribution: vasi at Evolving Web commentedSo this is interesting! I've configured migrations, and written this:
This yields the following:
Why are they different????
Comment #8
vasi CreditAttribution: vasi at Evolving Web commentedThis seems to be happening because of some code in MigrationPluginManager:: buildDependencyMigration():
Our condition at the end depends not on some global property, but on the dependencies of the last migration in the list being passed in. If the last migration has no optional dependencies, then all optional dependencies end up converted to required ones!
In our case, that means the d6_field migration is added to the d6_node:page one, even though it's optional. It looks like the UI first calculates all the migrations and their order, but then when running them loads them one at a time—so it doesn't encounter this problem.
I've added a test to reproduce this misbehaviour of buildDependencyMigration(), as well as just generally testing it, since it's complex and has no tests. This should fail.
Also added a fix.
Comment #10
catchGood find and simple fix.
Bumping to major for now.
Comment #11
mikeryanNot showing in the patch, but this is inside an isset($migration_dependencies['optional']) - it could be hit on an empty 'optional' array. Perhaps change the isset to !empty?
Blank line at start of class.
Comment #12
vasi CreditAttribution: vasi at Evolving Web commentedOk, those should be fixed. I can't add a test for the empty optionals situation, because the only harm it caused was disabling an optimization.
Also changed the test to make sure that ->set() is only called once.
Comment #14
mikeryanComment #15
mikeryanLooks good to me!
Comment #16
alexpottCommitted 1efd63e and pushed to 8.1.x and 8.2.x. Thanks!
Comment #19
mkalkbrennerThis commit introduced a fatal error when running phpunit. See #2725755: Fatal error: Cannot redeclare class Drupal\Tests\migrate\Unit\TestMigration for details.