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

[BUGFIX] Fix handling of chatty upgrade wizards #757

Merged
merged 5 commits into from Oct 21, 2018
Merged

[BUGFIX] Fix handling of chatty upgrade wizards #757

merged 5 commits into from Oct 21, 2018

Conversation

astehlik
Copy link
Contributor

@astehlik astehlik commented Oct 3, 2018

The TYPO3 code introduced a ChattyInterface for upgrade
wizards in d336193d86e3c2b99da6d59870f56116277e89c3.

When one of those wizards tries to output something it crashes
because the output is not initialized.

This is fixed by using a BufferedOutput and attaching the collected
messages to the upgrade wizard result.

@@ -80,6 +87,8 @@ public function executeWizard(string $identifier, array $rawArguments = [], bool
$hasPerformed = $upgradeWizard->performUpdate($dbQueries, $message);
}

$message = trim($message . PHP_EOL . $output->fetch());
Copy link
Member

Choose a reason for hiding this comment

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

This overwrites the messages passed as reference to old upgrade wizards.
This line needs to be moved up in the if statement that applies for wizards implementing the new interface.

@helhum
Copy link
Member

helhum commented Oct 8, 2018

Would also be awesome, if you could add a test that demonstrates the failure and the fix.

Thanks a bunch!

astehlik and others added 5 commits October 8, 2018 21:54
The TYPO3 code introduced a ChattyInterface for upgrade
wizards in d336193d86e3c2b99da6d59870f56116277e89c3.

When one of those wizards tries to output something it crashes
because the output is not initialized.

This is fixed by using a BufferedOutput and attaching the collected
messages to the upgrade wizard result.
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.

None yet

3 participants