Skip to content

Relax the version comparison for NPM uploads

David Fernandez requested to merge 472262-npm-relax-versions-comparison into master

🔭 Context

In the NPM package registry, when a package is uploaded, a background job will "process" it. At this time, different checks are execution.

One of them is called the manifest coherence check. In short words, when a NPM client uploads a package file, it sends:

  1. a tarball file (that's the file that is uploaded).
  2. a set of upload parameters (the upload payload).

The coherence check is there to make sure that (1.) and (2.) is coherent. Among the different fields, we check the version.

The check is currently implemented as version in (1.) has to be the exact same than in (2.).

Npm clients (in particular, $ npm) will use semver. The problem we discovered in gitlab-com/gl-infra/production#18253 (closed) is that in their usage, they will auto correct what is in (1.) to build (2.).

Example, when considering version precedence, the build identifier part is not taken into account. In short, 1.2.3+first.build and 1.2.3+second.build has strictly the exact same precedence. Thus, when (2.) is built, 1.2.3+first.build is auto corrected to 1.2.3.

Now, what happens with our coherence check? (1.2.3+first.build vs 1.2.3) Yes, 💥

This MR aims to relax this version comparison using https://gitlab.com/gitlab-org/ruby/gems/semver_dialects which can understand that 1.2.3+first.build and 1.2.3 is the exact same thing.

This is issue Relax the validation of versions for npm packages (#472262 - closed).

🤔 What does this MR do and why?

  • Relax the version comparison in the NPM manifest coherence check using semver_dialects
  • Update the related spec.

🏁 MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

🦄 Screenshots or screen recordings

No UI changes.

How to set up and validate locally

  1. Get a project and a PAT ready.
  • Create a folder with:
    1. package.json :
      {
        "name": "@scope/test",
        "version": "5.0.5",
        "main": "index.js"
      }
    2. .npmrc (replace <pat> and <project_id>):
    @scope:registry=http://gdk.test:8000/api/v4/projects/<project_id>/packages/npm/
    //gdk.test:8000/api/v4/projects/<project_id>/packages/npm/:_authToken="<PAT>"
  • run $ npm publish

🕹 Play with the version string and check the difference with master:

version On master With this MR Version displayed in the UI
5.0.5 (obvious) (obvious) 5.0.5 (obvious)
5.0.5+my.build 💥 5.0.5 (version string cleaned up)
v5.0.5 💥 5.0.5 (version string cleaned up)
v5.0.5+my.build 💥 5.0.5 (version string cleaned up)
5.0.5-dev+my.build 💥 5.0.5-dev (version string cleaned up)
v5.0.5-dev+my.build 💥 5.0.5-dev (version string cleaned up)
5.0.5+my.build-dev 💥 5.0.5 (version string cleaned up)
v5.0.5+my.build-dev 💥 5.0.5 (version string cleaned up)
=5.0.3 💥 💥 (expected) new error message saying that version is not semver compliant
05.01.03 💥 5.1.3 (version string cleaned up)
5.1.3-beta.01 💥 5.1.3-beta.1 (version string cleaned up)
=5.0.3 💥 💥 (expected) new error message saying that version is not semver compliant
5.1.3beta 💥 💥 (expected) new error message saying that version is not semver compliant

The majority of master 💥 cases are handled properly. The remaining ones have a new error message to point out users in the correct direction.

To be complete, here are the results for $ yarn publish (alternative to npm). Note that $ yarn does not auto cleanup version strings.

version On master With this MR Version displayed in the UI
5.0.5 (obvious) (obvious) 5.0.5 (obvious)
5.0.5+my.build 5.0.5+my.build
v5.0.5 💥 (not valid for yarn) 💥 (not valid for yarn) -
v5.0.5+my.build 💥 (not valid for yarn) 💥 (not valid for yarn) -
5.0.5-dev+my.build 5.0.5-dev+my.build
v5.0.5-dev+my.build 💥 (not valid for yarn) 💥 (not valid for yarn) -
5.0.5+my.build-dev 5.0.5+my.build-dev
v5.0.5+my.build-dev 💥 (not valid for yarn) 💥 (not valid for yarn) -

All good, no changes 🎉

Edited by David Fernandez

Merge request reports

Loading