PNPM lockfileVersion reverted back from 9.0 to 6.0

[wJoenn]

Is there an existing issue for this?

• I have searched the existing issues

Package ecosystem

npm

Package manager version

PNPM v9

Language version

Node.js 20

dependabot.yml content

version: 2
updates:
  - package-ecosystem: 'npm'
    directory: '/'
    schedule:
      interval: 'daily'
      time: "15:00"

What you expected to see, versus what you actually saw

Since #9668 I assumed updating a package that relies on a 9.0 pnpm.lock.yaml file should just update the given file without issue.

Instead the lockfileVersion is reverted back to 6.0 which rewrites the whole lockfile from scratch

Images of the diff or a link to the PR, issue, or logs

wJoenn/vue-template#237

[pboling]

The logic in the conditional statement is not great, and falls back to a default version of 6. I suggested an improvement which went unnoticed, so perhaps now it will get noticed!

#9668 (comment)

[wJoenn]

Tbh I don't see how this condition could provoke such bug 🤔

sig { params(pnpm_lock: DependencyFile).returns(Integer) }
def self.pnpm_version_numeric(pnpm_lock)
  if pnpm_lockfile_version(pnpm_lock).to_f >= 9.0
    9
  elsif pnpm_lockfile_version(pnpm_lock).to_f >= 6.0
    8
  elsif pnpm_lockfile_version(pnpm_lock).to_f >= 5.4
    7
  else
    6
  end
end

If pnpm_lockfile_version(pnpm_lock).to_f did end up in the default 6 returned value, the the lockfile version wouldn't be reverted to "6.0" but to an even older version as, as you can see, PNPM's v7 used to use a 5.4 lockfileVersion

In my case because my lockfileVersion ends up at 6.0 it would mean the pnpm version used by Dependabot is v8 for some reason

[pboling]

The existing logic is bizarre. If the sig defines it as returning an Integer, why call .to_f on it? If it starts as an integer it can never result in something like 5.4.

This code has never been mentally processed, merely changed over and over again, and passed a CI.

This is really sad to see.

[pboling]

Additionally, if the parser can't parse a v9 lock file in the first place, then we shouldn't expect it to work. Perhaps it returns nil which is converted to 0.0, which falls through to the 6 default.

If there is no fixture for a v9 lock file in the test suite, then it is impossible that a test written to pass returning v9 is properly testing the code.

[wJoenn]

If it starts as an integer it can never result in something like 5.4.

If we have a look we can see that the method used to get the lockfileVersion is:

def self.pnpm_lockfile_version(pnpm_lock)
  pnpm_lock.content.match(/^lockfileVersion: ['"]?(?<version>[\d.]+)/)[:version]
end

Which results in the proper version float number

irb(main):001> "lockfileVersion: '9.0'".match(/^lockfileVersion: ['"]?(?<version>[\d.]+)/)[:version].to_f
=> 9.0
irb(main):002> "lockfileVersion: '5.4'".match(/^lockfileVersion: ['"]?(?<version>[\d.]+)/)[:version].to_f
=> 5.4

If the sig defines it as returning an Integer, why call .to_f on it?

to_f is not called on the returned value but on a variable inside the method.
The method does indeed return an Integer, it being 9, 8, 7 or 6

Additionally, if the parser can't parse a v9 lock file in the first place, then we shouldn't expect it to work. Perhaps it returns nil which is converted to 0.0, which falls through to the 6 default.

First of all, in the same PR that you linked we can see that a test has been added to test parsing a v9 file so there's a fare chance that parsing the file works.
The test is not extensive enough that it checks that the version is accurate but there's a good chance ::pnpm_lockfile_version does not return nil.

Even if that was the case it wouldn't be the problem. You're confusing the lockfileVersion and pnpm's version.

When the lockfileVersion is less that 5.4 the the pnpm's version used is 6
But in my case the lockfileVersion was changes to 6.0 which means the pnpm version used by Dependabot was 8

Chill out man
You're free to create a PR if you think you have a solution 🙂

[fallemand]

Same issue happening for me: #9682

[pboling]

Chill out man
You're free to create a PR if you think you have a solution 🙂

I hear you, but...

Dependabot is GitHub, and GitHub is Microsoft. They also own NPM, a competitor project of pnpm.

I'd rather hold Microsoft Github's feet to the fire so they fix their products than do it for them for free.

Microsoft funnels millions to anti-human causes. They claim to have stopped funding election deniers (1, 2), but after they thought scrutiny had passed, they started again (3 (page 5)). They also fund climate-change-deniers in the US, while their AI chatbot preaches conspiracy and election denialism... so the suggestion to help them freely is absurd to me, as is the idea of being "chill" about their "mistakes", and slow response to fix issues which are actively harming their competition (pnpm in this case). Happy to help them help them help themselves though.

Dependabot is the only reason I still use GitHub for any private repos. I'll be happy to discard it if they don't fix this.

[tusbar]

From the rebase logs:

  proxy | 2024/05/10 07:09:48 [009] GET https://registry.npmjs.org:443/pnpm
  proxy | 2024/05/10 07:09:48 [010] GET https://registry.npmjs.org:443/pnpm
  proxy | 2024/05/10 07:09:48 [010] 200 https://registry.npmjs.org:443/pnpm
  proxy | 2024/05/10 07:09:48 [009] 200 https://registry.npmjs.org:443/pnpm
  proxy | 2024/05/10 07:09:48 [012] GET https://registry.npmjs.org:443/pnpm/-/pnpm-9.1.0.tgz
  proxy | 2024/05/10 07:09:48 [012] 200 https://registry.npmjs.org:443/pnpm/-/pnpm-9.1.0.tgz

pnpm v9 seems to be used… though I’m seeing the same behavior: lockfile re-generated to v6.

[fallemand]

From my logs, what's using PNPM 8 is: /home/dependabot/dependabot-updater/repo

updater | Your pnpm version is incompatible with "/home/dependabot/dependabot-updater/repo".
updater | 
updater | Expected version: ^9.0.4
updater | Got: 8.15.6
updater | 
updater | This is happening because the package's manifest has an engines.pnpm field specified.
updater | To fix this issue, install the required pnpm version globally.
updater | 
updater | To install the latest version of pnpm, run "pnpm i -g pnpm".
updater | To check your pnpm version, run "pnpm -v".

[tusbar]

Yes: #9687 should fix that.

[pboling]

It still does not use pnpm v9, for me I am getting pnpm v8 still, and additionally the outdated packages are not parsed properly (which I expect is because the format is different from v9).

updater | Dependabot encountered '2' error(s) during execution, please check the logs for more details.
updater | +-------------------------------+
updater | | Dependencies failed to update |
updater | +---------------+---------------+
updater | | drizzle-kit   | unknown_error |
updater | | tsx           | unknown_error |
updater | +---------------+---------------+

The two listed are correctly identified as out of date packages, but it fails to identify the version?