renovate icon indicating copy to clipboard operation
renovate copied to clipboard

Renovate attempts to downgrade a Composer package

Open beerendlauwers opened this issue 3 years ago • 14 comments

What Renovate type are you using?

Self-hosted Renovate, version 22.5.0.

Describe the bug

Renovate attempts to downgrade a Composer package, e.g. drupal/media_bulk_upload at version ~1.0.0-alpha24 in composer.json is suggested to be "upgraded" to ~1.0.0-alpha9. (Newest version at the time of writing is actually alpha27).

Relevant debug logs

/

To Reproduce

{
  "name": "renovate/example",
  "description": "Project template",
  "type": "project",
  "license": "proprietary",
  "authors": [
    {
      "name": "Example"
    }
  ],
  "support": {},
  "repositories": [
    {
      "type": "composer",
      "url": "https://packages.drupal.org/8"
    },
    {
      "type": "composer",
      "url": "https://asset-packagist.org"
    }
  ],
  "require": {
    "drupal/core": "~8.9.0",
    "drupal/media_bulk_upload": "~1.0.0-alpha24"
  },
  "autoload": {
    "psr-0": {
      "": "./scripts/composer/"
    }
  },
  "minimum-stability": "dev",
  "prefer-stable": true,
  "config": {
    "preferred-install": "dist",
    "platform": {
      "php": "7.3"
    }
  }
}

beerendlauwers avatar Aug 03 '20 09:08 beerendlauwers

As far as I'm aware, this is the correct sorting order according to the SemVer 2.0.0 spec:

Precedence for two pre-release versions with the same major, minor, and patch version MUST be determined by comparing each dot separated identifier from left to right until a difference is found as follows:

  1. Identifiers consisting of only digits are compared numerically.

  2. Identifiers with letters or hyphens are compared lexically in ASCII sort order.

rarkins avatar Aug 03 '20 11:08 rarkins

The package should have added a dot between the alpha and the number so that the numbers would be sorted numerically and not alphabetically. E.g. -alpha.9

rarkins avatar Aug 03 '20 11:08 rarkins

Next question though: does Composer itself have a non-compliant sorting of such versions? In which case we'll treat this as a bug as our goal is to emulate Composer's sorting regardless of whether it's compliant with semver or not.

rarkins avatar Aug 03 '20 11:08 rarkins

@rarkins In the specific case of the aforementioned package, alpha27appears to be regarded as the latest version:

composer show "drupal/media_bulk_upload" --all
name     : drupal/media_bulk_upload
descrip. : Allows uploading files in bulk and converting them to media entities.
keywords : 
versions : 1.x-dev, * 1.0.0-alpha27, 1.0.0-alpha26, 1.0.0-alpha25, 1.0.0-alpha24, 1.0.0-alpha23, 1.0.0-alpha22, 1.0.0-alpha21, 1.0.0-alpha20, 1.0.0-alpha19, 1.0.0-alpha18, 1.0.0-alpha17, 1.0.0-alpha12, 1.0.0-alpha11, 1.0.0-alpha10, 1.0.0-alpha9, 1.0.0-alpha8, 1.0.0-alpha7, 1.0.0-alpha6, 1.0.0-alpha5, 1.0.0-alpha4, 1.0.0-alpha3, 1.0.0-alpha2, 1.0.0-alpha1, dev-1.x
type     : drupal-module
license  : GNU General Public License v2.0 or later (GPL-2.0+) (OSI approved) https://spdx.org/licenses/GPL-2.0+.html#licenseText
source   : [git] https://git.drupalcode.org/project/media_bulk_upload.git 8.x-1.0-alpha27
dist     : [zip] https://ftp.drupal.org/files/projects/media_bulk_upload-8.x-1.0-alpha27.zip 8.x-1.0-alpha27
path     : /home/blauwers/projects/grando-retail/webroot/modules/contrib/media_bulk_upload
names    : drupal/media_bulk_upload

support
source : git://git.drupal.org/project/media_bulk_upload.git
issues : https://www.drupal.org/project/issues/media_bulk_upload

requires
drupal/core ^8 || ^9
drupal/dropzonejs ^2.0

Composer uses this code to sort the versions: https://github.com/composer/composer/blob/37b1e0fffdac1967da9b9f5e6ae5d83514a6b2ef/src/Composer/Command/ShowCommand.php#L708

The Composer\SemVer namespace is available here https://github.com/composer/semver/tree/master, but the actual parsing appears to be done in VersionParser.php (https://github.com/composer/composer/blob/37b1e0fffdac1967da9b9f5e6ae5d83514a6b2ef/src/Composer/Package/Version/VersionParser.php)

beerendlauwers avatar Aug 03 '20 13:08 beerendlauwers

Let's confirm with a php script that imports the semver package and sorts the two. I know for npm it sorts alpha9 as highest.

rarkins avatar Aug 03 '20 13:08 rarkins

The way I'm reading this issue, is that we need to figure out what the rules are for the Composer version sorting, before we can proceed:

does Composer itself have a non-compliant sorting of such versions? In which case we'll treat this as a bug as our goal is to emulate Composer's sorting regardless of whether it's compliant with semver or not.

So it seems we still need to do this:

Let's confirm with a php script that imports the semver package and sorts the two. I know for npm it sorts alpha9 as highest.

HonkingGoose avatar Mar 08 '21 11:03 HonkingGoose

Next step is to prove directly that Composer sorts the list of versions differently to how we do.

rarkins avatar Mar 08 '21 11:03 rarkins

Composer uses PHP's version_compare which compares 2.0.0.beta.9 vs 2.0.0.beta.16. See magento at packagist for example.

see https://github.com/composer/composer/blob/0021758e0bcf964237a56cb22e2f0309118b6a3f/src/Composer/Package/Version/VersionSelector.php#L126

JorisVanEijden avatar May 10 '21 13:05 JorisVanEijden

Any update on this?

Got this kind of grouped PR:

CleanShot 2021-08-16 at 06 34 14

back-2-95 avatar Aug 16 '21 03:08 back-2-95

Next step is to prove directly that Composer sorts the list of versions differently to how we do.

@back-2-95 ☝️ Somebody needs to confirm composer sorting, so we can fix renovate if it's wrong

viceice avatar Aug 16 '21 15:08 viceice

Next step is to prove directly that Composer sorts the list of versions differently to how we do.

@back-2-95 ☝️ Somebody needs to confirm composer sorting, so we can fix renovate if it's wrong

I linked to an example of composer sorting and to the source code of the composer sorting. I can confirm it differs from what renovate does.

JorisVanEijden avatar Aug 16 '21 15:08 JorisVanEijden

So it seems php compares versions on a strange thing

image

it will split 1.0.0-beta.9 to [1,0,0,'beta',9], so it compares numbers numerically. This can be done in javascript with localCompare numeric option.

image

viceice avatar Aug 16 '21 16:08 viceice

reproduction repo: https://github.com/MaronHatoum/renovate-6899

MaronHatoum avatar Aug 10 '22 08:08 MaronHatoum

Hi @beerendlauwers I tried to reproduce the issue you reported, please take a look at the comment above. I ran the latest Renovate on the reproduction repository, PRs have been created and the dependencies has been updated to the latest releases:

image

can you please confirm if the reproduction repository contains the same scenario you described?

MaronHatoum avatar Aug 10 '22 08:08 MaronHatoum

@MaronHatoum

The problem is the sort of 19 < 9 .

Replace

${['1.2.3-beta', '2.0.1', '1.3.4', '1.2.3']} | ${['1.2.3-beta', '1.2.3', '1.3.4', '2.0.1']}

with:

${['1.2.3-beta', '1.0.0-alpha24', '2.0.1', '1.3.4', '1.0.0-alpha9','1.2.3']} | ${['1.0.0-alpha9', '1.0.0-alpha24', '1.2.3-beta', '1.2.3', '1.3.4', '2.0.1']}

in https://github.com/renovatebot/renovate/blob/main/lib/modules/versioning/composer/index.spec.ts#L170

tobiasbaehr avatar Aug 10 '22 11:08 tobiasbaehr

Does the reproduction need updating or can this be solved using unit tests alone?

rarkins avatar Aug 10 '22 14:08 rarkins

I tried to use allowedVersions in the reproduction repo to allow versions < 1.0.0-alpha28 but it is not working for some reason. we can reproduce the issue using the test that @tobiasbaehr suggested (I already tried it). the expected result : '1.0.0-alpha9', '1.0.0-alpha24', '1.2.3-beta', '1.2.3', '1.3.4', '2.0.1' but Received: '1.0.0-alpha24', 1.0.0-alpha9', '1.2.3-beta', '1.2.3', '1.3.4', '2.0.1'

  test.each`
    versions                                                                      | expected
    ${['1.2.3-beta', '1.0.0-alpha24', '2.0.1', '1.3.4', '1.0.0-alpha9', '1.2.3']} | ${['1.0.0-alpha9', '1.0.0-alpha24', '1.2.3-beta', '1.2.3', '1.3.4', '2.0.1']}
  `('$versions -> sortVersions -> $expected ', ({versions, expected}) => {
    expect(versions.sort(semver.sortVersions)).toEqual(expected);
  });

MaronHatoum avatar Aug 10 '22 15:08 MaronHatoum

I tried to fix the bug, but I just to stupid for js :D https://github.com/tobiasbaehr/renovate/commit/d13c012c921b7220ae47f96da97d064a9ac4e951

tobiasbaehr avatar Aug 10 '22 16:08 tobiasbaehr

:tada: This issue has been resolved in version 35.57.3 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

renovate-release avatar Apr 23 '23 10:04 renovate-release