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 supported range of jackalope packages to composer.json #404

Merged
merged 2 commits into from
Jul 3, 2024

Conversation

alexander-schranz
Copy link
Contributor

Problem the latest version of 2.x PHPCR Bundle also installs the jackalope 2.0 packages:

  - Locking doctrine/phpcr-bundle (2.5.0)
  - Locking jackalope/jackalope (2.0.0)
  - Locking jackalope/jackalope-doctrine-dbal (2.0.1)

https://github.com/sulu/SuluHeadlessBundle/actions/runs/9760192331/job/26938459924?pr=139

But this ends on PHP 8.0 and 8.1 with:

PHP Fatal error: Declaration of Doctrine\Bundle\PHPCRBundle\OptionalCommand\Jackalope\InitDoctrineDbalCommand::configure() must be compatible with Jackalope\Tools\Console\Command\InitDoctrineDbalCommand::configure(): void in /home/runner/work/SuluHeadlessBundle/SuluHeadlessBundle/vendor/doctrine/phpcr-bundle/src/OptionalCommand/Jackalope/InitDoctrineDbalCommand.php on line 19

As the return types are correctly only added on the PHPCR 3.0 branch to avoid bc breaks. See #382.

Still the Sulu Bundles will require to atleast set the new version as min requirement else it could happen that 2.5.0 instead of 2.5.1 (including this PR changes) will be installed.

@alexander-schranz
Copy link
Contributor Author

We may can backport this directly to Sulu: sulu/sulu#7501 currently why PHPCRBundle is downgraded is because Sulu 2.5 requires Dantlech PHPCR Migration bundle and only 2.6 the new one.

@dbu
Copy link
Member

dbu commented Jul 3, 2024

thanks for the report! yeah, dependency management is tricky... (and we should have made those commands not extend but wrap the base commands, i guess. inheritance is prone to these kind of problems, decorating would have avoided the problem).

i agree with the change. why do you mark it as draft, anything left to do? (actually, it would be great if you can add a changelog entry explaining what the problem is)

i will look at the 8.1 build failure separately, that is not related to the jackalope issue.

@dbu
Copy link
Member

dbu commented Jul 3, 2024

at least the composer.json of the doctrine bundle suggest section indeed recommends to require ^1.3 of jackalope, so if people followed the suggestion they are safe.

@dbu
Copy link
Member

dbu commented Jul 3, 2024

i fixed the build in #405 (which indicates we lack tests that would see the conflict with the commands...)

can you please rebase and add to the changelog entry i created?

Comment on lines +33 to +35
"jackalope/jackalope": "< 1.3.1 || >= 2.0.0",
"jackalope/jackalope-doctrine-dbal": "< 1.3.0 || >= 2.0.0",
"jackalope/jackalope-jackrabbit": "< 1.3.0 || >= 2.0.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we need to increase this range when merging this back to 3.x

Copy link
Member

@dbu dbu left a comment

Choose a reason for hiding this comment

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

thanks

@dbu dbu merged commit 89fac6f into doctrine:2.x Jul 3, 2024
6 checks passed
@alexander-schranz alexander-schranz deleted the patch-1 branch July 3, 2024 14:25
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.

2 participants