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

This plugin uses symfony console, but does not require it so a newer version can be included breaking build processes. #46

Closed
2 tasks done
nlighteneddesign opened this issue Oct 17, 2024 · 7 comments

Comments

@nlighteneddesign
Copy link

nlighteneddesign commented Oct 17, 2024

Module version(s) affected

2.0.1

Description

As far as I can tell this project expects symfony/console 6 (recipe plugin too)
but there is no dependency, so if you update you get console 7 and a bunch of errors.

How to reproduce

Include symfony console 7

Possible Solution

No response

Additional Context

No response

Validations

  • Check that there isn't already an issue that reports the same bug
  • Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)

Related issues

PR

@emteknetnz
Copy link
Member

Note that this doesn't seem to be causing any installation issues with CMS 6 which uses symfony 7 (included in framework's composer.json) while things like recipe-cms are still using recipe-plugin 2 (which CMS 5 also uses) - https://github.com/silverstripe/recipe-cms/blob/6/composer.json#L9

@emteknetnz
Copy link
Member

emteknetnz commented Oct 23, 2024

Based on this comment in the related issue it seems like the issue is probably caused by a mis-configured composer.json file at the project level

If this isn't the case and you can point out something wrong at our end then please let me know and I'll reopen this and probably the other issue too

@GuySartorelli
Copy link
Member

This doesn't have anything to do with the composer deps (i.e. composer.json deps) I think. It seems more likely related to what version of symfony/console is used by composer itself.
The most recent version of composer has mixed versions in its constraints, which include the version with the strict typehints.

I think we should update this.

@GuySartorelli GuySartorelli reopened this Oct 23, 2024
@emteknetnz
Copy link
Member

While we could just merge these PRs as they are and they'll work with both versions of symfony via PHP Covariance, we do have a policy of strictly following semver to make decision making easier. Since there's a method signature change, we'd need to make new majors of recipe-plugin / vendor-plugin and update all CMS 6 modules that depend on them accordingly, if you want to do that then we should backlog a new issue for that

@GuySartorelli
Copy link
Member

I'm not saying we should make this change in a patch or minor release.
We can release a major version. That's fine.
I don't see why we'd need a new issue when this issue already explains what's wrong?

@emteknetnz
Copy link
Member

Because we'd basically end up deleting the description of this issue in order to write a new one for doing the work, seems much cleaner to just create a new issue. I've gone ahead and done this - silverstripe/.github#325

@GuySartorelli
Copy link
Member

That feels unnecessary but seeing as it's already been done:
Closing as a duplicate of #46

@GuySartorelli GuySartorelli closed this as not planned Won't fix, can't repro, duplicate, stale Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants