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

ContainerInterface reference was changed in a minor release #59

Closed
fabiang opened this issue Dec 1, 2020 · 19 comments
Closed

ContainerInterface reference was changed in a minor release #59

fabiang opened this issue Dec 1, 2020 · 19 comments
Assignees
Labels
BC Break Bug Something isn't working
Milestone

Comments

@fabiang
Copy link
Contributor

fabiang commented Dec 1, 2020

BC Break Report

Q A
Version 3.5.0

Summary

I waited for this day since two years or so, but did not expect this to happen on a minor release. Is it intentional that the Laminas\ServiceManager\Factory\FactoryInterface now uses Psr\Container\ContainerInterface instead of Interop\Container\ContainerInterface and all my factories are broken now, while I've only updated to minor version 3.5.0? Not even a single word in the changelogs. Shouldn't such a small change with huge impact be done in the 4.0 release?

Can we have an upgrade guide please?

@Ocramius
Copy link
Member

Ocramius commented Dec 1, 2020

@fabiang hmm, no, a BC break was not expected.

I did a release due to 3.5.0 being ready to be released and all changes merged thus far: sorry if it caused any issue: means that we have to get better at handling BC.

As for the BC break, since it was released minutes ago, I'd go with 3.5.1 that reverts it.

@Ocramius Ocramius self-assigned this Dec 1, 2020
@Ocramius Ocramius added BC Break Bug Something isn't working and removed BC Break labels Dec 1, 2020
@Ocramius Ocramius added this to the 3.5.1 milestone Dec 1, 2020
@Ocramius
Copy link
Member

Ocramius commented Dec 1, 2020

Indeed, 3.5.0 contained commits that should not have been there: I'm preparing a patch and will crank out a release ASAP

@Ocramius
Copy link
Member

Ocramius commented Dec 1, 2020

Ok, so here's what happened:

  1. I merged Replace container-interop with PSR-11 container interface zendframework/zend-servicemanager#187 into develop 2 years ago: Replace container-interop with PSR-11 container interface zendframework/zend-servicemanager#187 (comment)
  2. package got renamed
  3. at some point, develop became 3.5.x instead of becoming 4.0.x
  4. today, after reading a ping (Add support for PHP 8, fix tests #57 (comment)) and checking the issues that were assigned to the 3.5.0 milestone (https://github.com/laminas/laminas-servicemanager/milestone/1?closed=1) I triggered the release automation (https://github.com/laminas/laminas-servicemanager/actions/runs/393759253)

The problem is that the diff (https://github.com/laminas/laminas-servicemanager/compare/3.4.1..3.5.0) does not match the milestone (https://github.com/laminas/laminas-servicemanager/milestone/1?closed=1)

@Ocramius
Copy link
Member

Ocramius commented Dec 1, 2020

Ok, after evaluating the entire diff, a full revert of 3.5.0 is needed: I will push 3.4.1 as 3.5.1 and move 3.5.x to 4.0.x, and 3.4.x as 3.5.x

@Ocramius Ocramius changed the title ContainerInterface ContainerInterface reference was changed in a minor release Dec 1, 2020
@Ocramius
Copy link
Member

Ocramius commented Dec 1, 2020

Marking this as closed and starting 3.5.1 release automation

@Ocramius
Copy link
Member

Ocramius commented Dec 1, 2020

3.5.1 is now the same as 3.4.1

I will publish 3.5.0 as a new branch, mostly for us to move things into 4.0.x.

@fabiang
Copy link
Contributor Author

fabiang commented Dec 1, 2020

Thanks a lot. I can confirm that it's working with 3.5.1 again.

Would you accept an PR with an unit tests for this interface? I would do it with Prophecy, so we can make sure that such an fundamental interface is always correct for that version?

@Ocramius
Copy link
Member

Ocramius commented Dec 1, 2020

@fabiang the tests are already in place: the diffs changed all related tests too, hence why it looked like everything was ready for a release (and wasn't).

What should probably be built in is something like roave/backward-compatibility-check, which guarantees that a minor release won't contain more backwards-incompatible changes (https://github.com/Roave/BackwardCompatibilityCheck).

Can you perhaps send a patch with that? See https://github.com/Roave/BackwardCompatibilityCheck/blob/20c0e653865822ecf836bd249810dea8f677a864/.github/workflows/backwards-compatibility.yml for an example of how this works there.

@fabiang
Copy link
Contributor Author

fabiang commented Dec 1, 2020

Btw I think 3.5.0 should be removed from Packagist.org. If someone does compose require laminas/laminas-service right now he will get ^3.5 in his composer.json, but 3.5.1 is installed. When he executed his tests with composer install --prefer-lowest (I do often to check the lowest versions required) the tests will break.

@Ocramius
Copy link
Member

Ocramius commented Dec 1, 2020

Packagist does scrub versions after some time: happens overnight though.

Alternatively, @weierophinney can "press zeh button" when online.

@weierophinney
Copy link
Member

Done!

@adamturcsan
Copy link

Thanks for the fast reaction! I've hit the 3.5.0, but it's all good now!

@fabiang
Copy link
Contributor Author

fabiang commented Dec 2, 2020

What should probably be built in is something like roave/backward-compatibility-check, [...]

I've tried. But I can't install any version without needing to update other requie-devs and then loosing PHP support for <7.x.

@Ocramius
Copy link
Member

Ocramius commented Dec 2, 2020

The tool can be installed independently in a job that runs 7.4

@PowerKiKi
Copy link

According to #51, the 3.5.0 version should have brought support for PHP 8.0. But with the cancellation of 3.5.0 and complete revert in 3.5.1, we lost that PHP 8.0 support.

I see there is already a 3.5.2 milestone. Is it planned to (re-)release the non-breaking parts from 3.5.0 as a 3.5.2 ? should we open pull requests for that ? or what should we do ?

@froschdesign
Copy link
Member

@PowerKiKi
Please have a look at #64

@PowerKiKi
Copy link

Thanks ! so I will wait for 3.6.0 for PHP 8

@mrVrAlex
Copy link

mrVrAlex commented Feb 3, 2021

FYI: IMHO: This problem with BC Break can be resolve just like this:

use interop\container\containerinterface as OldContainerInterface;
use Psr\Container\ContainerInterface;

class_alias(ContainerInterface::class, OldContainerInterface::class);

And add this file to composer autoload...

P.S. It's ugly but it worked for 3.5.0 version for me.)

@boesing
Copy link
Member

boesing commented Feb 3, 2021

@shandyDev We reverted 3.5.0 and thus you should upgrade to 3.6.X if you are still using 3.5.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BC Break Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants