Skip to content

Change targetplatform version check to match full CMS version#5546

Closed
mbabker wants to merge 1 commit intojoomla:stagingfrom
mbabker:updateVersion
Closed

Change targetplatform version check to match full CMS version#5546
mbabker wants to merge 1 commit intojoomla:stagingfrom
mbabker:updateVersion

Conversation

@mbabker
Copy link
Copy Markdown
Contributor

@mbabker mbabker commented Dec 28, 2014

The manner in which updates check their supported platform is extremely subpar. The primary version attribute is only checked down to the minor version number (i.e. 3.4) which prevents using the advanced regex possibilities of the <targetplatform> tag in an update XML file to match a specific version number. A "band-aid" style fix was applied in the manner of the min_dev_level and max_dev_level attributes which further demonstrates the weakness in this check.

This PR proposes to change the check to enable matching a full version number instead of only the major and minor versions.

Use Case

Had 2.5 supported this, in extensions where versions older than 2.5.6 are unsupported, it would have been possible to use a regexp similar to 2.5.([6-9]|1[0-9]|2[0-8]) to not display updates to 2.5.0 thru 2.5.5 users. As 2.5 didn't even support the min_dev_level and max_dev_level attributes, users on those dated versions would always see an extension update that they could not install.

Testing Instructions

For general testing, the simplest tests are to apply the patch, install older versions of extensions which support Joomla's update platform (as Joomla won't display an update if you're on the latest version), and make sure those extensions still properly load updates. The language installer can be a good way to check this too.

For advanced use cases, developers are welcome to manipulate the Joomla version number (found in libraries/cms/version/version.php and an update XML to create test cases where a regexp would or would not match a version number. Using my example above, 2.5.0 thru 2.5.5 and anything above 2.5.29 should not match but 2.5.6 thru 2.5.28 should.

@Hackwar
Copy link
Copy Markdown
Member

Hackwar commented Jan 1, 2015

Shouldn't we simply support minimum requirements instead of complex regex version numbers? We could have a minimum version and then simply check with version_compare()...

@mbabker
Copy link
Copy Markdown
Contributor Author

mbabker commented Jan 1, 2015

Right now, the internal check is a regex, so for B/C it has to stay that way. For example, the language XMLs use this to check against version 3.[012345], and that would break. I agree a version_compare would be much simpler.

@Hackwar
Copy link
Copy Markdown
Member

Hackwar commented Jan 1, 2015

Could we maybe start a list of things (small/tiny things) somewhere, that we want to do for 4.0? Otherwise stuff like this would drop through and we have to drag this along until 5.0...

@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented Jan 2, 2015

Could we maybe start a list of things (small/tiny things) somewhere, that we want to do for 4.0? Otherwise stuff like this would drop through and we have to drag this along until 5.0...

When we close PRs due to B/C but want to look at it again for 4.0, we already add the label "reevaluate for 4.0".

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Apr 11, 2015

@test successful tested with an outdated german language file.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5546.

@mbabker
Copy link
Copy Markdown
Contributor Author

mbabker commented May 29, 2015

@nikosdion Test please :-)

@nikosdion
Copy link
Copy Markdown
Contributor

@test Successful test with Akeeba Backup Core and the German language pack.

Since this doesn't really affect b/c in any way I'd propose adding this to Joomla! 3.5 or, if the feature list is frozen, in 3.6.


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5546.

@Bakual Bakual added this to the Joomla! 3.5.0 milestone May 30, 2015
@Bakual Bakual added the RTC This Pull Request is Ready To Commit label May 30, 2015
@Bakual
Copy link
Copy Markdown
Contributor

Bakual commented May 30, 2015

It's fine for 3.5

@Kubik-Rubik
Copy link
Copy Markdown
Member

Thank you @mbabker! Merged.

@zero-24 zero-24 removed the RTC This Pull Request is Ready To Commit label Oct 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants