[RFC] Refactoring phpbb_version_compare

Discuss general development subjects that are not specific to a particular version like the versioning control system we use or other infrastructure.
Post Reply
User avatar
3Di
Registered User
Posts: 694
Joined: Tue Nov 01, 2005 9:50 pm
Location: Milano (I) Frankfurt (D)
Contact:

[RFC] Refactoring phpbb_version_compare

Post by 3Di » Tue Mar 29, 2016 11:07 pm

Hi all,
the purpose is to refactor the function phpbb_version_compare in order to check if at least also the files (constants.php) are accordingly to the DB's config['version'].

Gives a more constructive way to check, at least, if we are allowing to install something onto a not ready to be used Board that gives the supporters more troubles than nothing else, nonetheless.

Making it mandatory the ext.php for extensions isn't, IMO, a bad idea.. also.

Comments and suggestions are welcome.
Thanks. :)
Want to compensate me for my interest? Don't ask me, just do it
I'm available for custom phpBB jobs, you can PM me the details.
New: my Live Board, Extensions and Tools for phpBB - Men at works.

User avatar
david63
Registered User
Posts: 215
Joined: Mon Feb 07, 2005 7:23 am
Location: Lancashire, UK

Re: [RFC] Refactoring phpbb_version_compare

Post by david63 » Wed Mar 30, 2016 7:04 am

I would agree with using the ext file for this - it can be customised to give more meaningful, and helpful, error messages to the user.
David
Remember: You only know what you know -
and you do not know what you do not know!

User avatar
3Di
Registered User
Posts: 694
Joined: Tue Nov 01, 2005 9:50 pm
Location: Milano (I) Frankfurt (D)
Contact:

Re: [RFC] Refactoring phpbb_version_compare

Post by 3Di » Wed Mar 30, 2016 9:22 am

At the present time and since has been introduced (phpBB 3.0.10-RC1) this function is just a wrapper to the PHP version_compare. Only difference is that is case-insensitive, AFAIK.

The ext.php file also uses the phpbb_version_compare, that's why the present RFC.
This leads also me to think if it could be better to refactor the function is_enableable, instead.

Code: Select all

	public function is_enableable()
	{
		$config = $this->container->get('config');
		return phpbb_version_compare($config['version'], '3.1.8-RC1', '>=');
	}
Want to compensate me for my interest? Don't ask me, just do it
I'm available for custom phpBB jobs, you can PM me the details.
New: my Live Board, Extensions and Tools for phpBB - Men at works.

User avatar
Marc
Development Team Leader
Development Team Leader
Posts: 123
Joined: Thu Sep 09, 2010 11:36 am
Location: Munich, Germany

Re: [RFC] Refactoring phpbb_version_compare

Post by Marc » Sat Apr 02, 2016 8:17 am

A change for comparing these two versions has been proposed for 3.1.x already:
https://github.com/phpbb/phpbb/pull/3901

The PR is currently waiting small fixes to it's initial proposal though.

User avatar
VSE
Extension Customisations
Extension Customisations
Posts: 670
Joined: Mon Mar 08, 2010 9:18 am

Re: [RFC] Refactoring phpbb_version_compare

Post by VSE » Sun Apr 03, 2016 1:28 am

phpbb_version_compare should not be changed. It doesn't care what you're comparing. It's just a case-insensitive wrapper for version_compare.

And refactoring is_enableable is also not an likely, as it is meant for any possible reason an extension can be allowed to enable or not. It is purposely ambiguous by default and up to ext authors to decide how they want to use it, if they need it.

Also, the eventual goal is for extensions to respect the composer.json version constraints when been installed. Using is_enableable() to compare the phpbb version is just a stop-gap measure I came up with until such time.
david63 wrote:
Wed Mar 30, 2016 7:04 am
I would agree with using the ext file for this - it can be customised to give more meaningful, and helpful, error messages to the user.
It already does. It basically says something about the board not meeting the requirements of the extension.
Has an irascible disposition.

User avatar
david63
Registered User
Posts: 215
Joined: Mon Feb 07, 2005 7:23 am
Location: Lancashire, UK

Re: [RFC] Refactoring phpbb_version_compare

Post by david63 » Sun Apr 03, 2016 6:54 am

VSE wrote:
Sun Apr 03, 2016 1:28 am
It basically says something about the board not meeting the requirements of the extension
The problem with that message is is that it is too generic - by using the ext file you can have multiple tests each with their own error message.

What would make life easier would be if an extension developer could add an optional language variable to the phpbb_version_compare function to override the generic message.
David
Remember: You only know what you know -
and you do not know what you do not know!

User avatar
VSE
Extension Customisations
Extension Customisations
Posts: 670
Joined: Mon Mar 08, 2010 9:18 am

Re: [RFC] Refactoring phpbb_version_compare

Post by VSE » Sun Apr 03, 2016 7:51 am

Umm, look at phpbb_version_compare. It only returns a boolean true/false. It has nothing to do with the messages displayed.

Also, no extension can add any language variables if the extension is not enabled.

But like I said before, the is_enableable() doing a version check is temporary until 3.2/3.3. And of course it's message is a little generic, there's a lot of reasons why one might return false for is_enableable. But in fact the message is right, if is_enableable is false, it means the extension can not be allowed to be enabled on the board. Doesn't matter what tests resulted in false really, the bottom line is that extension fails to meet a requirement for being enableable. And since the extension is not enabled, it can't provide it's own message, so it relies on phpBB's message.

And just to say it a third time, this is all moot, as this will all be a thing of the past soon enough. Thanks to https://github.com/phpbb/phpbb/pull/3909
Has an irascible disposition.

User avatar
3Di
Registered User
Posts: 694
Joined: Tue Nov 01, 2005 9:50 pm
Location: Milano (I) Frankfurt (D)
Contact:

Re: [RFC] Refactoring phpbb_version_compare

Post by 3Di » Mon Apr 04, 2016 3:53 pm

Marc wrote:
Sat Apr 02, 2016 8:17 am
A change for comparing these two versions has been proposed for 3.1.x already:
https://github.com/phpbb/phpbb/pull/3901

The PR is currently waiting small fixes to it's initial proposal though.
Thanks Marc for the link, I am seeing also here the good job done and the fact that's related to Ascraeus, though (for completeness) it could include a check also in cases (like I encountered) that for some reasons the user ran the DB updater but the files weren't updated, believe it or not.

Also this, though, isn't related to what I meant opening this Topic.

I am trying to re-word/explain the whole thing also having read further this topic, till the end.

I will post an exaustive answer to those points once some test has been completed in my loc env, thank you. :)
Want to compensate me for my interest? Don't ask me, just do it
I'm available for custom phpBB jobs, you can PM me the details.
New: my Live Board, Extensions and Tools for phpBB - Men at works.

User avatar
3Di
Registered User
Posts: 694
Joined: Tue Nov 01, 2005 9:50 pm
Location: Milano (I) Frankfurt (D)
Contact:

Re: [RFC] Refactoring phpbb_version_compare

Post by 3Di » Mon Apr 11, 2016 3:20 pm

I am still WIP about the new function to use instead of phpbb_version_compare, that still will remain in place into the core code. Just a one more and accurated function that serves in this cases, after having read all of your comments.

What it is also worth to be mentioned is that a lot of requests for support are related to installed Ascraeus Boards onto a server running PHP7, that shouldn't happen IMO.

There could be also a check into the installer that denies the installation at all in this case.

Just a thought. :)

Reference (just one of a lot) : https://www.phpbb.com/community/viewtop ... #p14404376

Thanks.
Want to compensate me for my interest? Don't ask me, just do it
I'm available for custom phpBB jobs, you can PM me the details.
New: my Live Board, Extensions and Tools for phpBB - Men at works.

User avatar
3Di
Registered User
Posts: 694
Joined: Tue Nov 01, 2005 9:50 pm
Location: Milano (I) Frankfurt (D)
Contact:

Re: [RFC] Refactoring phpbb_version_compare

Post by 3Di » Mon Apr 11, 2016 8:52 pm

3Di wrote:
Mon Apr 11, 2016 3:20 pm
What it is also worth to be mentioned is that a lot of requests for support are related to installed Ascraeus Boards onto a server running PHP7, that shouldn't happen IMO.

There could be also a check into the installer that denies the installation at all in this case.
https://tracker.phpbb.com/browse/PHPBB3-14596
Want to compensate me for my interest? Don't ask me, just do it
I'm available for custom phpBB jobs, you can PM me the details.
New: my Live Board, Extensions and Tools for phpBB - Men at works.

Post Reply