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

Remove phpdbg, migrate to pcov #130

Open
Ocramius opened this issue Oct 11, 2022 · 8 comments · Fixed by #148
Open

Remove phpdbg, migrate to pcov #130

Ocramius opened this issue Oct 11, 2022 · 8 comments · Fixed by #148
Labels
BC Break Bug Something isn't working dependencies Pull requests that update a dependency file Enhancement
Milestone

Comments

@Ocramius
Copy link
Member

See sebastianbergmann/php-code-coverage#945

According to upstream maintainers, phpdbg coverage support is not really maintained, and we should therefore migrate away from it.

We will likely need to ship pcov with our container, and use that for running tests.

We may also need to remove phpdbg, since running phpdbg with pcov installed leads to segfaults.

@Ocramius Ocramius added Bug Something isn't working Enhancement dependencies Pull requests that update a dependency file labels Oct 11, 2022
@Ocramius Ocramius added this to the 1.27.0 milestone Oct 11, 2022
@Ocramius
Copy link
Member Author

PHPDBG support has been removed right now: sebastianbergmann/php-code-coverage@c304be7

@Ocramius
Copy link
Member Author

PHPUnit 10 pulled the plug and switched over: will prepare a patch for this, since we can otherwise no longer run coverage reports (which breaks mutation tests)

Ocramius added a commit to Ocramius/laminas-continuous-integration-action that referenced this issue Feb 22, 2023
This is a potential breaking change, but note that PHPUnit v10 no longer
supports PHPDBG for coverage, which was the only use-case for PHPDBG in
this container, thus far.

We instead install `pcov`, which is considered precise enough for coverage
reports by PHPUnit.

Fixes laminas#130

Ref: laminas#130
Ref: sebastianbergmann/php-code-coverage#945
Ref: sebastianbergmann/php-code-coverage@c304be7
@boesing
Copy link
Member

boesing commented Feb 22, 2023

Careful:
https://github.com/laminas/laminas-ci-matrix-action/blob/ca11cee53aec05d9cc301993585c316d27e3040d/src/tools.ts#L34-L42

I still lack time/mood to combine matrix + action in one repository as I am still not that deep into the whole container registry push stuff, etc.

This change will introduce BC break to almost any project running infection.


We may also need to remove phpdbg, since running phpdbg with pcov installed leads to segfaults.

phpdbg is just an executable while pcov is an extension. I wonder if there is a way to have dedicated phpdbg php.ini which just does not load the pcov extension?

@Ocramius
Copy link
Member Author

I still lack time/mood to combine matrix + action in one repository as I am still not that deep into the whole container registry push stuff, etc.

Same: mostly trying to reduce the dependency upgrade noise caused by PHPUnit 10.

This change will introduce BC break to almost any project running infection.

I will go for 2.0.0 then 👍

phpdbg is just an executable while pcov is an extension. I wonder if there is a way to have dedicated phpdbg php.ini which just does not load the pcov extension?

If we roll 2.0.0 and drop PHPDBG and PHP:<8.0, I think we can get rid of the problem overall: WDYT?

@Ocramius Ocramius added this to the 2.0.0 milestone Feb 22, 2023
@boesing
Copy link
Member

boesing commented Feb 23, 2023

If we roll 2.0.0 and drop PHPDBG and PHP:<8.0, I think we can get rid of the problem overall: WDYT?
Yah, I think that might work.

Only thing I don't like about this is that this will then lead to upgrade maintenance in all repositories, including those which still have support for 7.4 which then have to release with a new version dropping 7.4 just because we dropped it as some extension is not properly installable in docker.

So I know that dropping old PHP versions usually provide "easy fixes" but since we are providing a CI pipeline here, it feels kinda weird that we start dropping "old" PHP versions. I still think it could be possible to have both phpdbg and pcov available, might just be a problem with enabling the pcov extension for the dbg SAPI which can probably be fixed in here:

https://github.com/laminas/laminas-continuous-integration-action/blob/a39482668bb41479b0c8f8cdd2a3369e5a0c8628/scripts/extensions.sh#LL86C13-L86C13

Anyways, thats not the time to discuss this further. I'm fine with a new release - lets see how it will end. Maybe I'm just the panicking dude again while there is no reason to cry. 😅

@Ocramius
Copy link
Member Author

Well, 7.x is gonna be in oldstable anyway, no?

The fact that we keep this to a new major is OK IMO 👍

I just need to find the willpower to coordinate the releases xD

@Slamdunk
Copy link
Contributor

Hello guys, both this issue and #149 seem have been addressed.

Can milestone 2.0.0 be closed then?

gsteel added a commit to gsteel/looker that referenced this issue Oct 16, 2023
gsteel added a commit to gsteel/laminas-escaper that referenced this issue Dec 12, 2023
…-action#130 is finished

Signed-off-by: George Steel <george@net-glue.co.uk>
@fezfez
Copy link

fezfez commented May 8, 2024

Hello, same question as @Slamdunk , can it be release ? thanks

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 dependencies Pull requests that update a dependency file Enhancement
Projects
None yet
4 participants