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

DOM extension reflection version is incorrect #8038

Open
AndrolGenhald opened this issue Feb 4, 2022 · 6 comments · May be fixed by #17702
Open

DOM extension reflection version is incorrect #8038

AndrolGenhald opened this issue Feb 4, 2022 · 6 comments · May be fixed by #17702

Comments

@AndrolGenhald
Copy link

Description

Getting the DOM extension version from the reflection API results in "20031129" no matter which PHP version is used: https://3v4l.org/VIJKY
I would expect it to behave like other bundled extensions and track the PHP version: https://3v4l.org/iNi1B

The following code:

<?php

$domExtReflection = new ReflectionExtension("dom");
var_dump($domExtReflection->getVersion());

Resulted in this output:

string(8) "20031129"

But I expected this output instead:

string(5) "8.1.2"

PHP Version

8.1.2

Operating System

No response

@cmb69
Copy link
Member

cmb69 commented Feb 6, 2022

Oh, right, looks like that version hasn't been bumped for a long time.

@cmb69
Copy link
Member

cmb69 commented Feb 7, 2022

Well, on a closer look, it seems that using the DOM_API_VERSION as extension version isn't the best idea. The extension version should be the same as PHP_VERSION, but DOM_API_VERSION might have some value on its own. On the other hand, it hasn't been bumped for so many years, and I doubt that there have been no API changes, so maybe we should just drop it altogether in favor of PHP_DOM_VERSION. That change cannot happen before PHP 9, though. Not sure what to do.

@alecpl
Copy link

alecpl commented Feb 16, 2022

My 2cents. Expecting it to return the PHP version does not make sense to me. The method is supposed to return version of the extension and various extensions might use it's own versioning, isn't it? But I agree that using a number that wasn't updated since 2003 is not good.

@cmb69 why it should be the same as PHP_VERSION? what's the reason to have this method if it is expected to return PHP_VERSION? why it cannot happen in 8.2?

@cmb69
Copy link
Member

cmb69 commented Feb 16, 2022

@alecpl, few years ago most bundled extensions switched to using the PHP_VERSION, because it is hard to track their own versioning. Still it makes sense to have an own version for all extensions, just in case they will be unbundled (and moved to PECL) in the future, so they would need their own versioning.

Maybe we could remove DOM_API_VERSION in PHP 8.2, but that would be a BC break, and I'm not sure whether that would be appropriate for a minor version, given the small gain. And it wouldn't solve the issue we're having now (in PHP 8.0 and 8.1).

@SigmaFlame
Copy link

Hello, this bug is still not fixed in PHP 8.4.2, it is currently impossible to retrieve the version of an extension.
phpversion("tidy") and (new \ReflectionExtension('tidy')->getVersion()) both return the version of PHP itself.
Only xdebug correctly returns it version number: 3.4.0.

@cmb69
Copy link
Member

cmb69 commented Jan 4, 2025

phpversion("tidy") and (new \ReflectionExtension('tidy')->getVersion()) both return the version of PHP itself.
Only xdebug correctly returns it version number: 3.4.0.

XDebug has its own release cycle and thus its own versioning; bundled extensions (like ext/tidy) share the release cycle with PHP, and as such (most) have the same version as PHP – I can't see anything wrong with that.

cmb69 added a commit to cmb69/php-src that referenced this issue Feb 4, 2025
It doesn't appear to be reasonable to pretend that nothing in the DOM
API would have changed in the last 21 years, and it's also not that
reasonable to pretend that we would thoroughly bump that version in the
future.  Thus we go with the common convention to use the `PHP_VERSION`
for bundled extensions.
@cmb69 cmb69 linked a pull request Feb 4, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants