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

Can't install due to conflicting requirements #800

Closed
andremartinez opened this issue Mar 2, 2023 · 25 comments
Closed

Can't install due to conflicting requirements #800

andremartinez opened this issue Mar 2, 2023 · 25 comments

Comments

@andremartinez
Copy link

I'm trying to require DoctrineModule using:

composer require doctrine/doctrine-module:5.3.0

Pretty standard, but I'm getting the following:

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires doctrine/doctrine-module 5.3.0 -> satisfiable by doctrine/doctrine-module[5.3.0].
    - doctrine/doctrine-module 5.3.0 requires doctrine/annotations ^1.13.3 -> found doctrine/annotations[1.13.3, ..., 1.14.3] but the package is fixed to 2.0.1 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.

The conflicting package is PHP-CS-Fixer, that requires this:

        "doctrine/annotations": "^2",

While the composer.json in DoctrineModule requires this:

        "doctrine/annotations": "^1.13.3",

I'm not sure whether doctrine/annotations has a stable release for version 2, and this repo should upgrade the requirement or not

What do you guys suggest?

@demiankatz
Copy link
Contributor

I'm seeing the same problem and would be interested to hear a solution! I'm not familiar enough with Doctrine to know whether this module could add || ^2 to the annotations dependency, or if the backward-incompatible changes in annotations v2 will break something.

@andremartinez
Copy link
Author

andremartinez commented Mar 3, 2023

I was thinking of forking this myself and making some tests, or even creating a pull request, but I really would like to hear someone else's opinion before committing to implementing anything

@greg0ire
Copy link
Member

greg0ire commented Mar 3, 2023

Please go ahead. v2 shouldn't be too hard to upgrade to.

@demiankatz
Copy link
Contributor

Thanks, @andremartinez! If I can do anything to help you, please let me know. I'm not an expert in any of this (I discovered this problem because I'm in the very early stages of migrating my project from Laminas\Db to Doctrine), but still happy to help with testing, troubleshooting, etc. to the best of my ability.

@andremartinez
Copy link
Author

Ok this is gonna be one of those...

The require-dev dependency of doctrine/mongodb-odm also requires "doctrine/annotations": "^1.*", on their require meaning an upgrade should happen there as well.

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires doctrine/annotations ^2, found doctrine/annotations[2.0.0, 2.0.1] but these were not loaded, likely because it conflicts with another require.
  Problem 2
    - doctrine/mongodb-odm is locked to version 2.4.3 and an update of this package was not requested.
    - doctrine/mongodb-odm 2.4.3 requires doctrine/annotations ^1.12 -> found doctrine/annotations[1.12.0, ..., 1.14.3] but it conflicts with your root composer.json require (^2).

Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions.

Also phpstan analyse is returning this nonsense:

> phpstan analyse
Note: Using configuration file /home/andre/workspace/DoctrineModule/phpstan.neon.
 71/71 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ --------------------------------------------------------------------------------------------------------------------------------------- 
  Line   tests/Service/CacheFactoryTest.php                                                                                                     
 ------ --------------------------------------------------------------------------------------------------------------------------------------- 
  84     Parameter #1 $config of method                                                                                                         
         Laminas\ServiceManager\AbstractPluginManager<Laminas\Cache\Storage\StorageInterface>::configure()                                      
         expects array{abstract_factories?:                                                                                                     
         array<class-string<Laminas\ServiceManager\Factory\AbstractFactoryInterface>|Laminas\ServiceManager\Factory\AbstractFactoryInterface>,  
         aliases?: array<string, string>, delegators?:                                                                                          
         Laminas\ServiceManager\DelegatorsConfigurationType,                                                                                    
         factories?:                                                                                                                            
         Laminas\ServiceManager\FactoriesConfigurationType,                                                                                     
         initializers?:                                                                                                                         
         Laminas\ServiceManager\InitializersConfigurationType,                                                                                  
         invokables?: array<string, string>, lazy_services?:                                                                                    
         array{class_map?: array<string, class-string>,                                                                                         
         proxies_namespace?: non-empty-string, proxies_target_dir?:                                                                             
         non-empty-string, write_proxy_files?: bool}, services?:                                                                                
         array<string, array|object>, ...}, array{factories:                                                                                    
         array{Laminas\Cache\Storage\Adapter\BlackHole:                                                                                         
         'Laminas\\ServiceManager\\Factory\\InvokableFactory'},                                                                                 
         aliases: array{blackhole:                                                                                                              
         'Laminas\\Cache\\Storage\\Adapter\\BlackHole'}} given.                                                                                 
         💡 Offset 'factories'                                                                                                                  
            (Laminas\ServiceManager\FactoriesConfigurationType) does not                                                                        
            accept type array<string, string>.                                                                                                  
 ------ --------------------------------------------------------------------------------------------------------------------------------------- 


                                                                        
 [ERROR] Found 1 error                                                  
                                                                        

Script phpstan analyse handling the phpstan event returned with error code 1
Script @phpstan was called via check

I just cloned this, how come?

@demiankatz
Copy link
Contributor

I suppose another consideration is to approach this from the opposite direction -- here's the commit where php-cs-fixer dropped annotations v1, and I can't really see a compelling reason for the change. I doubt we can persuade them to revert it (after all, forward progress is generally a good thing), but hopefully it's a sign that changes required won't be too terrible.

Do you get the phpstan error after a composer install of the current dependencies, before changing anything related to the annotations? Have you compared the way phpstan runs in continuous integration here with your local approach? (Sorry if these are obvious questions -- just trying to be as helpful as I can with limited project familiarity :-) ).

@andremartinez
Copy link
Author

...but hopefully it's a sign that changes required won't be too terrible.

I think it will be annoying, but just annoying

Do you get the phpstan error after a composer install of the current dependencies, before changing anything related to the annotations? Have you compared the way phpstan runs in continuous integration here with your local approach? (Sorry if these are obvious questions -- just trying to be as helpful as I can with limited project familiarity :-) ).

Yup the Most recent job apparently failed like this well

@demiankatz
Copy link
Contributor

Interesting. I can't see anything obviously wrong with this, but the formatting of the phpstan output doesn't exactly make it easy to parse what's going on! I imagine it's possible that a new release of phpstan has broken something. Since there is no committed composer.lock file in this project (for good reason), it might make sense to pin the dev tools in composer.json to more specific versions so that there's finer-grained control over when they get updated. Otherwise, a new tool release can cause a mystery broken build like this. I've learned of this problem the hard way, and now I just pin everything to specific versions so I can update tools on my own terms and schedule. :-)

@greg0ire
Copy link
Member

greg0ire commented Mar 4, 2023

it might make sense to pin the dev tools

FTR, that's exactly what we do on some other Doctrine repositories where this situation happens too often.

Note that mongodb-odm 2.5.0 was released yesterday, and that it now supports annotations v2: https://github.com/doctrine/mongodb-odm/blob/b8ff84dacb3b4926de346919211bf0fabe86e31a/composer.json#L26

You might want to give this another try.

@demiankatz
Copy link
Contributor

Thanks, @greg0ire, that's all good to know! Things still won't resolve as long as this project's composer.json lacks the "|| ^2.0" option, so the question is, would adding that break anything?

@greg0ire
Copy link
Member

greg0ire commented Mar 4, 2023

Well there's only one way to find out…

@demiankatz
Copy link
Contributor

@greg0ire, I did a quick experiment and found that the next domino in the chain appears to be https://github.com/doctrine/mongodb-odm -- I think that will need to be updated to support annotations v2 before we can make progress on this PR. It looks like there's work in progress on that in doctrine/mongodb-odm#2498, though it looks like there hasn't been a lot of activity there for more than a month.

@greg0ire
Copy link
Member

greg0ire commented Mar 6, 2023

@demiankatz what are you talking about? As per my message above, mongodb-odm 2.5.0 has support for v2. The PR you are pointing at is only about making annotations optional, it's something orthogonal to our issue.

I did a quick experiment

Please make a PR, it will be easier to discuss any hurdles you are having. Right now I don't know what you try, and can't tell why you would be blocked.

@demiankatz
Copy link
Contributor

Sorry, @greg0ire, I was confused because the default branch of the mongodb-odm Git repo still points to 2.4.x, which does not support annotations v2. I'll try again and open a PR for further discussion.

@greg0ire
Copy link
Member

greg0ire commented Mar 6, 2023

Thank you!

@demiankatz
Copy link
Contributor

@greg0ire, looks like this is not going to be a simple drop-in replacement. Here are the changes I had to make to get dependencies to install (note that it required me to raise the mongodb extension version in the platform section, along with bumping annotations and mongodb-odm):

5.4.x...demiankatz:DoctrineModule:annotations-v2-compatibility

Unfortunately, tests fail after making this change, so it appears that the backward-incompatible changes in annotations v2 DO cause problems.

There are a total of 32 test failures, but they seem to fall into two basic categories, which are demonstrated by these two examples:

2) DoctrineModuleTest\Service\DriverFactoryTest::testCreateMongoDBODMAnnotationDriver
Error: Class "Doctrine\Common\Annotations\CachedReader" not found

/home/dkatz/DoctrineModule/src/Service/DriverFactory.php:84
/home/dkatz/DoctrineModule/src/Service/DriverFactory.php:48
/home/dkatz/DoctrineModule/tests/Service/DriverFactoryTest.php:164

3) DoctrineModuleTest\ServiceFactory\ModuleDefinedServicesTest::testModuleDefinedServices with data set #0 ('doctrine.cache.array', true)
Error: Call to undefined method Doctrine\Common\Annotations\AnnotationRegistry::registerLoader()

/home/dkatz/DoctrineModule/src/Module.php:21
/home/dkatz/DoctrineModule/vendor/laminas/laminas-modulemanager/src/Listener/InitTrigger.php:25
/home/dkatz/DoctrineModule/vendor/laminas/laminas-eventmanager/src/EventManager.php:320
/home/dkatz/DoctrineModule/vendor/laminas/laminas-eventmanager/src/EventManager.php:170
/home/dkatz/DoctrineModule/vendor/laminas/laminas-modulemanager/src/ModuleManager.php:160
/home/dkatz/DoctrineModule/vendor/laminas/laminas-modulemanager/src/ModuleManager.php:76
/home/dkatz/DoctrineModule/vendor/laminas/laminas-eventmanager/src/EventManager.php:320
/home/dkatz/DoctrineModule/vendor/laminas/laminas-eventmanager/src/EventManager.php:170
/home/dkatz/DoctrineModule/vendor/laminas/laminas-modulemanager/src/ModuleManager.php:99
/home/dkatz/DoctrineModule/tests/ServiceManagerFactory.php:45
/home/dkatz/DoctrineModule/tests/ServiceFactory/ModuleDefinedServicesTest.php:20

I'm probably not the best-qualified person to fix this, though I can try to carve out some time to dig deeper if nobody better-equipped is able to look into it.

I haven't opened a PR since I didn't want to create a broken PR without asking first. I'm happy to do it if you think it would save you time, but if somebody else would prefer to just cherry-pick my commit from above and build on it, that's fine with me too. Whatever is most helpful! :-)

@greg0ire
Copy link
Member

greg0ire commented Mar 6, 2023

I haven't opened a PR since I didn't want to create a broken PR without asking first.

You can open a draft PR, I think that's fine. Regarding the call to registerLoader, I think it might be useless, given the comment here: https://github.com/doctrine/annotations/blob/fb0d71a7393298a7b232cbf4c8b1f73f3ec3d5af/lib/Doctrine/Common/Annotations/AnnotationRegistry.php#L102-L111

You could try to remove it entirely. Regarding CachedReader, you should use PsrCachedReader instead, as per https://github.com/doctrine/annotations/blob/fb0d71a7393298a7b232cbf4c8b1f73f3ec3d5af/lib/Doctrine/Common/Annotations/CachedReader.php#L20-L22

Note that both points could be handled separately without upgrading I think, making this upgrade a "drop-in replacement"

@demiankatz
Copy link
Contributor

demiankatz commented Mar 6, 2023

Thanks for the pointers, @greg0ire. Removing registerLoader does indeed clear up most of the errors. I have opened #801 as a draft, and I've added a TODO note to test with annotations v1 after the work is done to be sure we haven't broken backward compatibility. Unfortunately, it looks to me like the one remaining piece, the CachedReader --> PsrCachedReader modification, is going to be a non-trivial change. The doctrine/cache library is deprecated in favor of PSR-6/PSR-16, and I suspect we need to do some refactoring to get rid of it before we can easily finish the rest of the work. That's probably a task for a separate PR, but unfortunately I think it's more than my time will permit me to figure out at the moment. Of course, if nobody else is willing or able, I can try to make time -- but it's going to require me to learn more details that I don't currently know, so it will take me a while to work through, and my plate is already very full.

@demiankatz
Copy link
Contributor

@andremartinez, I've made some more progress on addressing this in #801; you might want to take a look. Feedback would be greatly appreciated! I'm not confident that I've fully solved the problem (indeed, I'm pretty sure more work on upgrading the cache code is necessary to avoid some loss of functionality), but I think I've at least gotten things to a point where the packages can resolve, and things shouldn't be completely broken (I just expect that caching functionality may be lost under certain circumstances).

@andremartinez
Copy link
Author

Thank you very much @demiankatz !

I'll take a look now, I've been busy lately so I didn't touch this issue lately.

Except for some warnings, that should be addressed, the P.R. looks good to me

@TomHAnderson
Copy link
Member

@demiankatz
Copy link
Contributor

Thanks, @TomHAnderson! I can't fully test this until there's a new release of DoctrineORMModule as well. I've started work on updating DoctrineORMModule for compatibility with DoctrineModule 6 in doctrine/DoctrineORMModule#734, but I'm a bit stuck on next steps and could use some help/guidance. I also opened #805 in response to a problem detected by Psalm during my work on DoctrineORMModule.

@demiankatz
Copy link
Contributor

@TomHAnderson / @andremartinez -- I spent a little more time on doctrine/DoctrineORMModule#734 and cracked the problem that was causing tests to fail. I believe it is now ready for review. Are either of you (or anyone else) available to take a closer look at that and also #805 (which I think should be a very easy thing to merge)? I'd love to get all of this merged so a new release of DoctrineORMModule can be merged -- it's currently blocking progress on my project, so I'm motivated to help if there's anything else you need from me. :-)

@demiankatz
Copy link
Contributor

I've also figured out the phpstan problems on DoctrineORMModule -- see doctrine/DoctrineORMModule#736. So I think if these three PRs can be reviewed/merged and new releases of DoctrineModule and DoctrineORMModule minted, we'll be all up to date!

@driehle
Copy link
Member

driehle commented Oct 18, 2023

Closed with #801 I think?

@driehle driehle closed this as completed Oct 18, 2023
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

No branches or pull requests

5 participants