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

Add doctrine/common 3 support #6127

Merged
merged 1 commit into from
Aug 14, 2020

Conversation

jaikdean
Copy link
Contributor

@jaikdean jaikdean commented Jun 1, 2020

Subject

This PR adds support for doctrine/common 3. It follows the upgrade instructions in Doctrine's release notes.

I am targeting this branch, because the change is back-compatible.

Closes #6121.

Changelog

### Added
- Added support for `doctrine/common` 3.

core23
core23 previously approved these changes Jun 1, 2020
Copy link
Member

@phansys phansys left a comment

Choose a reason for hiding this comment

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

@jaikdean
Copy link
Contributor Author

jaikdean commented Jun 2, 2020

It looks like doctrine/common is restricted to 2.x by SonataBlockBundle. Should I create a PR to support 3.x in that package too? I'm not sure what's going on with the versioning and which branch to target, as SonataAdminBundle doesn't use the current major SonataBlockBundle.

@jordisala1991
Copy link
Member

SonataBlock 3.19 released but doctrine common is still on 2.x, maybe there are more dependencies that block the install? @jaikdean

@jaikdean
Copy link
Contributor Author

jaikdean commented Jun 4, 2020

It seems the dependency tree here is a little more complex than I'd realised and will need updates to some third-party packages.

@greg0ire
Copy link
Contributor

greg0ire commented Jun 4, 2020

composer why-not folks 😉

composer why-not doctrine/common 3
sonata-project/admin-bundle  3.x-dev  requires          doctrine/common (^2.7)                         
doctrine/common              3.0.0    requires          doctrine/persistence (^2.0)                    
sonata-project/admin-bundle  3.x-dev  does not require  doctrine/persistence (but 1.3.7 is installed)  


composer why-not doctrine/persistence 2
doctrine/common                     2.13.2  requires  doctrine/persistence (^1.3.3)  
sonata-project/doctrine-extensions  1.6.0   requires  doctrine/persistence (^1.3.6)  
symfony/doctrine-bridge             v4.4.9  requires  doctrine/persistence (^1.3)

@phansys
Copy link
Member

phansys commented Jun 27, 2020

I'm allowing doctrine/persistence:^2.0 here: sonata-project/sonata-doctrine-extensions#203.

@jordisala1991
Copy link
Member

The symfony doctrine bridge already has support for this, can you rebase to fix phpstan? Maybe we can merge this now

@jordisala1991
Copy link
Member

Well, now the build uses the doctrine common 3 and the build fails. Can you fix it @jaikdean ?

@jaikdean
Copy link
Contributor Author

I've fixed the compatibility issues with doctrine/persistence 2.0. The remaining issue is an incompatibility between symfony/security-acl (which receives minimal maintenance) and doctrine/common/doctrine/persistence. This package doesn't have a requirement or conflict for either listed in its composer.json, which is why it's allowing an incompatible version to be installed.

@SonataCI
Copy link
Collaborator

SonataCI commented Aug 8, 2020

Could you please rebase your PR and fix merge conflicts?

@jordisala1991
Copy link
Member

@jaikdean the security acl is already fixed

@jaikdean
Copy link
Contributor Author

Thanks @jordisala1991.

This branch has been rebased and conflicts resolved. doctrine/common is now at version 3.

$ composer info doctrine/common
name     : doctrine/common
descrip. : PHP Doctrine Common project is a library that provides additional functionality that other Doctrine projects depend on such as better reflection support, persistence interfaces, proxies, event system and much more.
keywords : common, doctrine, php
versions : * 3.0.2
type     : library
license  : MIT License (MIT) (OSI approved) https://spdx.org/licenses/MIT.html#licenseText
homepage : https://www.doctrine-project.org/projects/common.html
source   : [git] https://github.com/doctrine/common.git a3c6479858989e242a2465972b4f7a8642baf0d4
dist     : [zip] https://api.github.com/repos/doctrine/common/zipball/a3c6479858989e242a2465972b4f7a8642baf0d4 a3c6479858989e242a2465972b4f7a8642baf0d4
path     : /Users/jaikdean/Documents/SonataAdminBundle/vendor/doctrine/common
names    : doctrine/common

@jordisala1991
Copy link
Member

PHPStan is failing, are you sure you did a rebase with latest 3.x changes?

@jordisala1991
Copy link
Member

jordisala1991 commented Aug 10, 2020

ping @vladyslavstartsev any clues why it is failing? seems like the PR is rebased correctly.

@franmomu
Copy link
Member

franmomu commented Aug 11, 2020

Looks like it's installing block-bundle 3.x in the phpstan action and the phpstan baseline errors are based on block-bundle4.x

@jordisala1991
Copy link
Member

Oh so we need first to add support for doctrine common 3 on sonata block 4?

@franmomu
Copy link
Member

It already has support, but I have no idea why in this action is installing block bundle 3:
https://github.com/sonata-project/SonataAdminBundle/pull/6127/checks?check_run_id=971548249#step:4:81

And in the last commit merged is installing block bundle 4:
https://github.com/sonata-project/SonataAdminBundle/runs/965815491#step:4:84

composer.json Outdated Show resolved Hide resolved
composer.json Outdated Show resolved Hide resolved
composer.json Show resolved Hide resolved
@jordisala1991
Copy link
Member

Build is green and installing common 3, but before merge we need to know if those dependencies added are correct

@jaikdean jaikdean requested a review from phansys August 12, 2020 08:44
franmomu
franmomu previously approved these changes Aug 12, 2020
jordisala1991
jordisala1991 previously approved these changes Aug 12, 2020
@jordisala1991 jordisala1991 requested a review from a team August 12, 2020 09:56
VincentLanglet
VincentLanglet previously approved these changes Aug 12, 2020
@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

Fix support for doctrine/persistence 1.3 and 2.0

Remove unnecessary Doctrine dependencies
@jaikdean
Copy link
Contributor Author

Rebased and merge conflicts resolved.

@phansys phansys added the minor label Aug 14, 2020
@jordisala1991 jordisala1991 merged commit 46e8f56 into sonata-project:3.x Aug 14, 2020
@jordisala1991
Copy link
Member

Thank you @jaikdean

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support doctrine/common 3
8 participants