symfony / security-acl

Symfony Security ACL Component
https://symfony.com/components/Security
MIT License
363 stars 48 forks source link

Declare incompatibility with `symfony/security-core` 6 #97

Closed derrabus closed 2 years ago

derrabus commented 2 years ago

On the 6.0 branch, symfony/security-core has added return type declarations to the VoterInterface. Our implementation AclVoter is incompatible which is why our high-deps CI job is currently failing.

Since this incompatibility is not trivial to fix in a backwards-compatible manner, I propose to remove the 6.0 branch from the list of compatible versions.

wouterj commented 2 years ago

Unfortunately, doing this will break the symfony/symfony CI as it requires this component.

(we did this just before we made the previous release, which you corrected swiftly in https://github.com/symfony/security-acl/pull/92 )

nicolas-grekas commented 2 years ago

We'd better remove the return types on branch 6.0 I suppose.

wouterj commented 2 years ago

Maybe I'm missing something, but doesn't that mean that we must remove return types from all methods that are in classes that are not internal or final? That would be a real bummer...

nicolas-grekas commented 2 years ago

See my monolog on the topic at https://github.com/symfony/symfony/pull/42496 :)

derrabus commented 2 years ago

Well, for this library, it's actually one (!) method that causes us trouble. I'm convinced we can manage that without reverting every return type we've added so far. 😱

nicolas-grekas commented 2 years ago

We'd remove the return type only from VoterInterface::vote() then. The pain we feel now is the one the community will also feel very soon. Removing a few return types to reduce the migration cost looks worth it to me.

derrabus commented 2 years ago

We'd remove the return type only from VoterInterface::vote() then.

symfony/symfony#42901

derrabus commented 2 years ago

Closing because this has been resolved in Symfony.

nicolas-grekas commented 2 years ago

Linking to https://github.com/symfony/symfony/issues/43021 for history.