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

Composer require doctrine/doctrine-bundle=~1.12.0 #9382

Merged
merged 1 commit into from
Dec 9, 2020

Conversation

maxlawton
Copy link
Contributor

@maxlawton maxlawton commented Nov 9, 2020

Q A
Branch? 3.2
Bug fix? yes
New feature? no
Deprecations? no
BC breaks? no
Automated tests included? no
Related user documentation PR URL N/A
Related developer documentation PR URL N/A
Issue(s) addressed N/A

Description:

Fixes exception when running bin/console due to compatibility issue between doctrine/doctrine-bundle and doctrine/dbal: doctrine/DoctrineBundle#1168 .

With doctrine/dbal being an inherited requirement, the version constraint is such that the incompatible version is installed. The solution is to either add an explicit restriction of the doctrine/dbal version to a release that pre-dates this bug; or, to upgrade to the newer doctrine/doctrine-bundle within the 1.x line, which still retains compatibility with Mautic's other requirements (e.g. symfony/* at ~3.4.0).

NOTE

Upon further testing of this with a clean, standalone copy of Mautic @ 3.2, I realize the issue will not be present when installing with the distributed composer.lock file present, as the version of doctrine/dbal specified there in (v2.10.1) does not contain the connection option in the RunSqlCommand. I encountered this issue when requiring Mautic as a dependency of another Composer project having its own top-level composer.json to allow for the management of private internal plugins as packages without patching Mautic's composer file. Thus, this PR is addressing an issue that is unlikely to affect standard Mautic installations.

Steps to reproduce the issue:

  1. Run composer install without composer.lock present, which will install doctrine/doctrine-bundle at 1.11.2
  2. Run bin/console with or without any subcommand specified
  3. See the exception mentioned in the linked PR above

Steps to test this PR:

  1. Load up this PR
  2. Run composer install, updating doctrine/doctrine-bundle to 1.12.11
  3. Run bin/console
  4. Observe that no exceptions are thrown

@cla-bot cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Nov 9, 2020
escopecz
escopecz previously approved these changes Dec 9, 2020
Copy link
Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

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

We also use "doctrine/doctrine-bundle": "~1.11.0", in our cloud, so I approve this PR.

@maxlawton could you please rebase to latest 3.2 branch?

@escopecz escopecz added needs-rebase PR's that need to be rebased pending-test-confirmation PR's that require one test before they can be merged labels Dec 9, 2020
Fixes exception when running bin/console, encountered when Mautic is
installed as a library (and the lock file is thus ignored):

In InputDefinition.php line 232:

  [Symfony\Component\Console\Exception\LogicException]
  An option named "connection" already exists.

Exception trace:
  at /var/www/vendor/symfony/console/Input/InputDefinition.php:232
 Symfony\Component\Console\Input\InputDefinition->addOption() at /var/www/vendor/symfony/console/Command/Command.php:406
 Symfony\Component\Console\Command\Command->addOption() at /var/www/vendor/doctrine/doctrine-bundle/Command/Proxy/RunSqlDoctrineCommand.php:24
 Doctrine\Bundle\DoctrineBundle\Command\Proxy\RunSqlDoctrineCommand->configure() at /var/www/vendor/symfony/console/Command/Command.php:77
 Symfony\Component\Console\Command\Command->__construct() at /var/www/vendor/doctrine/dbal/lib/Doctrine/DBAL/Tools/Console/Command/RunSqlCommand.php:36
 Doctrine\DBAL\Tools\Console\Command\RunSqlCommand->__construct() at /var/www/html/var/cache/prod/ContainerH108zlf/getDoctrine_QuerySqlCommandService.php:8
 require() at /var/www/html/var/cache/prod/ContainerH108zlf/appProdProjectContainer.php:6117
 ContainerH108zlf\appProdProjectContainer->load() at /var/www/html/var/cache/prod/ContainerH108zlf/getConsole_CommandLoaderService.php:77
 ContainerH108zlf\appProdProjectContainer->{closure}() at /var/www/vendor/symfony/dependency-injection/ServiceLocator.php:64
 Symfony\Component\DependencyInjection\ServiceLocator->get() at /var/www/vendor/symfony/console/CommandLoader/ContainerCommandLoader.php:46
 Symfony\Component\Console\CommandLoader\ContainerCommandLoader->get() at /var/www/vendor/symfony/console/Application.php:523
 Symfony\Component\Console\Application->has() at /var/www/vendor/symfony/console/Application.php:715
 Symfony\Component\Console\Application->all() at /var/www/vendor/symfony/framework-bundle/Console/Application.php:122
 Symfony\Bundle\FrameworkBundle\Console\Application->all() at /var/www/vendor/symfony/console/Descriptor/ApplicationDescription.php:102
 Symfony\Component\Console\Descriptor\ApplicationDescription->inspectApplication() at /var/www/vendor/symfony/console/Descriptor/ApplicationDescription.php:75
 Symfony\Component\Console\Descriptor\ApplicationDescription->getCommands() at /var/www/vendor/symfony/console/Descriptor/TextDescriptor.php:194
 Symfony\Component\Console\Descriptor\TextDescriptor->describeApplication() at /var/www/vendor/symfony/console/Descriptor/Descriptor.php:55
 Symfony\Component\Console\Descriptor\Descriptor->describe() at /var/www/vendor/symfony/console/Helper/DescriptorHelper.php:67
 Symfony\Component\Console\Helper\DescriptorHelper->describe() at /var/www/vendor/symfony/console/Command/ListCommand.php:75
 Symfony\Component\Console\Command\ListCommand->execute() at /var/www/vendor/symfony/console/Command/Command.php:255
 Symfony\Component\Console\Command\Command->run() at /var/www/vendor/symfony/console/Application.php:1010
 Symfony\Component\Console\Application->doRunCommand() at /var/www/vendor/symfony/framework-bundle/Console/Application.php:86
 Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand() at /var/www/vendor/symfony/console/Application.php:255
 Symfony\Component\Console\Application->doRun() at /var/www/vendor/symfony/framework-bundle/Console/Application.php:74
 Symfony\Bundle\FrameworkBundle\Console\Application->doRun() at /var/www/vendor/symfony/console/Application.php:148
 Symfony\Component\Console\Application->run() at /var/www/html/bin/console:43
@maxlawton
Copy link
Contributor Author

We also use "doctrine/doctrine-bundle": "~1.11.0", in our cloud, so I approve this PR.

@maxlawton could you please rebase to latest 3.2 branch?

@escopecz Did you mean you use ~1.12.0 in your cloud? Because that's the version this PR specifies (was previously ~1.11.0).

I've rebased onto 3.2, updated the composer.lock during rebase, and dropped the irrelevant version/asset bump commit.

@escopecz
Copy link
Member

escopecz commented Dec 9, 2020

My bad. I read the diff wrong. I don't see a reason why not to bump to the latest version. The diff looks like nothing should break. https://github.com/doctrine/DoctrineBundle/releases/tag/1.12.0

Copy link
Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

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

👍

@npracht npracht merged commit 9197692 into mautic:3.2 Dec 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The PR contributors have signed the contributors agreement needs-rebase PR's that need to be rebased pending-test-confirmation PR's that require one test before they can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants