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

v189 #461

Merged
merged 15 commits into from
Feb 5, 2021
Merged

v189 #461

merged 15 commits into from
Feb 5, 2021

Conversation

dzuelke
Copy link
Contributor

@dzuelke dzuelke commented Feb 5, 2021

The usual version updates, and some polish for Composer's package's PHP version requirement and associated handling.

mkrepo.sh has to sort all packages by their "heroku-sys/php" version requirement in descending order

this is due to an internal implementation detail of Composer's repository data structure, in which e.g. "ext-memcached" in version "3.1.5" is stored multiple times: once for PHP 8.0, once for PHP 7.4, once for PHP 7.3, and so forth.

If a composer.json now has a requirement for "php: *" and "ext-memcached: *", and "ext-memcached: 3.1.5" in its variant for PHP 7.0 (expressed in turn as a dependency on "heroku-sys/php: 7.0.*") comes first in the list of packages, this would result in PHP 7.0 getting installed instead of a later possible version.

Until now, all packages, if they used a PHP version requirement (that's only the case for extensions, commonly), expressed that as "7.4.*" or similar.

A package might, however, also want to express the version requirement differently. That's not useful for extensions, which are compiled against a particular PHP series, but for e.g. Composer, we want to say "5.3.2 or later", etc - and for a future Composer version, "7.2.0 or later" if it requires that, and then have the solver automatically pick something older for folks stuck on an older PHP.

We use Python's distutils.version.LooseVersion for a rough comparison, since the version strings might contain all sorts of things (e.g. "^7.2.3 || 8.0.0" or whatever), most notably that asterisk.

However, comparing ">=5.3.2" against e.g. "0.0.0" fails with a str vs int TypeError.

Why would one side of the comparison be 0.0.0? Because that's the default if the "php" requirement is absent.

So when changing e.g. the Composer package from no version requirement to ">=5.3.2", the repo couldn't be generated, since there is an older manifest with no requirement at all, and now its "0.0.0" default is compared against ">=5.3.2", which TypeErrors.

The solution is to just replace anything that looks like a version specifier with a zero, since we're not comparing exact versions, we just need to roughly sort.
This is just a nice-to-have, since all of our PHPs satisfy all these requirements, but if that ever changes, this will help catch it.
@dzuelke dzuelke requested a review from a team as a code owner February 5, 2021 00:48
@dzuelke dzuelke merged commit 967a2b9 into main Feb 5, 2021
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.

2 participants