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

PNPM support? #684

Closed
AndrewCraswell opened this issue Jun 28, 2023 · 17 comments
Closed

PNPM support? #684

AndrewCraswell opened this issue Jun 28, 2023 · 17 comments
Labels
bug Something isn't working

Comments

@AndrewCraswell
Copy link

Dependabot has released an update to support pnpm package manager, and the associated feature request was closed as completed. The docs have also been updated to show the new package manager support matrix.

I've been trying to verify the changes in our Azure Devops repo, where we use pnpm v8.6.5 and Dependabot via this extension. I'm seeing the package.json file is having the proper version bump, but the pnpm-lock.yaml (using version 6.0) is not being updated. This lead me to wonder if maybe the Dependabot Azure Devops extension is not yet using the latest version of Dependabot?

If it is, do you have any ideas about increasing logging to see why the lockfile is not being updated and included in the PRs that are opened?

@mburumaxwell
Copy link
Contributor

@AndrewCraswell it is easier for me to work on this if you can provide a repro. I have zero experience with pnpm

@hbendev
Copy link

hbendev commented Jul 10, 2023

In my case, even though I use the most recent version (pnpm -v outputs 8.6.7) and the lockfile contains lockfileVersion: '6.0', the job fails with:

Parsing dependencies information
/home/dependabot/dependabot-updater/vendor/ruby/3.1.0/bundler/gems/dependabot-core-8b4e7c7c59ff/npm_and_yarn/lib/dependabot/npm_and_yarn/file_parser/pnpm_lock.rb:23:in `rescue in block in parsed': /pnpm-lock.yaml not parseable (Dependabot::DependencyFileNotParseable)

The project is a workspace with a single lockfile.

Edit:
I've tested this same monorepo locally, with the most recent commit on dependabot-core, and the parsing method returned no errors: https://github.com/dependabot/dependabot-core/tree/8b4e7c7c59ff502de5fe6d2a9f20807aa8710cdd. So I'm also assuming some version mismatch here.

Edit2:
No issues locally on https://github.com/dependabot/dependabot-core/tree/de36c26692ed9bb0d73beda417efc252a4c6d75d either. This is where pnpm support was first added, and the diff between the two are only dependency updates inside the helpers package.

Edit3:
I see this package is using the most recent version of dependabot-core, so there might be something wrong with my configuration instead.

Edit4:
Indeed, it was a misconfiguration from my end. In the dependabot configuration I needed to list packages seperately (by directory). Now, however I was able to reproduce the original issue, where the lockfile in the root was not updated in the PRs. I'll try to add a public example for you in the upcoming days.

Another issue I found is that update-types based ignore matchers are not working. Even though I have this in dependabot.yml:

- dependency-name: "*"
     update-types: ["version-update:semver-major"]

PRs are opened that increase a package's version by major steps. This was previously working well with separate npm packages inside my project.

@AndrewCraswell
Copy link
Author

@AndrewCraswell it is easier for me to work on this if you can provide a repro. I have zero experience with pnpm

My apologies, I can try to put together a repro but it would need to be over a weekend. Assembling a minimal reproduction would be a good first step, and I need to do this anyways to provide an example for how to implement Dependabot in our org.

@mburumaxwell
Copy link
Contributor

@AndrewCraswell it is easier for me to work on this if you can provide a repro. I have zero experience with pnpm

My apologies, I can try to put together a repro but it would need to be over a weekend. Assembling a minimal reproduction would be a good first step, and I need to do this anyways to provide an example for how to implement Dependabot in our org.

Yes, this is the way. 🙂

@hbendev
Copy link

hbendev commented Jul 15, 2023

Hi @mburumaxwell! Here's a very minimal example: https://dev.azure.com/hajnalbendeguz/pnpm-dependabot. Unfortunately, it hasn't received access to use Azure runner agents just yet, so the pipeline can't start, but a request doesn't take more than a few days to fulfill as I read.

@AndrewCraswell
Copy link
Author

Okay, I've put together a minimal reproduction and dropped the same repo in Github and in DevOps so we can verify it works in Github.

Github Repo

DevOps Repo
DevOps Pipeline

Interestingly, in the Azure DevOps repro the pipeline has started failing with parse errors. Two weeks back the pipeline would succeed but the PRs that would open would only include changes to the package.json file, and not lockfile changes. Now the pipeline is just failing.

image

@AndrewCraswell
Copy link
Author

I was able to look at our private repo build history and see what the output was when it was successfully creating the PRs (albeit without the changes to lockfile). Interestingly it seems that the lockfile was not being detected. Only the package.json is being found:

image

This contrasts with the minimal reproduction that I put together today, where the lockfile is found but then the parse error is encountered.

image

@mburumaxwell
Copy link
Contributor

HI @AndrewCraswell
Thanks for offering the repro. I see the issue is parsing the lock file.
This happens in dependabot-core.
The only known way to troubleshoot this is by debugging the Ruby code. You can do this using either this repo or the dependabot-core one.

I should get to this in the course of the week if you will not have managed to get a solution.

@AndrewCraswell
Copy link
Author

This unfortunately is outside of my comfort zone to provide a PR in Ruby. But in case it helps, I've put up a small bounty on the work item.
https://app.bountysource.com/issues/123710224-pnpm-support

@mburumaxwell mburumaxwell added the bug Something isn't working label Jul 25, 2023
@mburumaxwell
Copy link
Contributor

Just an update, I have managed to repro the issue. Hopefully, it does not take too long to resolve.

@mburumaxwell
Copy link
Contributor

@AndrewCraswell

In the latest version, this has been addressed.
The release is major and I hope it does not affect your other setups (if any).
Release notes: 1.20.0
Validation build: https://dev.azure.com/tingle/dependabot/_build/results?buildId=63019&view=results

@AndrewCraswell
Copy link
Author

Just verified it's working! I think it's safe to close the issue :)

@hbendev
Copy link

hbendev commented Jul 31, 2023

Thanks a lot! It does appear to be working, but sadly I bumped into errors which didn't happen while I was still using npm.

  1. Azure DevOps has a 4000 character limit on the description, and I think it's now not, or incorrectly truncated. See example pipeline here: https://dev.azure.com/hajnalbendeguz/pnpm-dependabot/_build/results?buildId=22&view=logs&j=12f1170f-54f2-53f3-20dd-22fc7dff55f9&t=1f93c1a3-255b-505a-53c9-c382114aabf5&l=296
    Edit: separate ticket already created: PR's description characters limit won't work #723

  2. The pipeline is breaking on workspace dependencies, but it has to be tweaked in dependabot core I think.

/home/dependabot/dependabot-updater/vendor/ruby/3.1.0/gems/dependabot-npm_and_yarn-0.224.0/lib/dependabot/npm_and_yarn/requirement.rb:26:in `parse': Illformed requirement ["workspace:*"] (Gem::Requirement::BadRequirementError)`
{
  "dependencies": {
    "@private/package": "workspace:*",
  }
}

This latter error even happens when package updates are explicitly ignored by dependency-name rules in dependabot.yml.

@mburumaxwell
Copy link
Contributor

@AndrewCraswell great! I will proceed

@hbendev ,

  1. Let' use PR's description characters limit won't work #723 to track the 4k limit issue. I know there was a major change in dependabot-core recently, see Add option to control PR description max length + set reasonable defaults for each platform dependabot/dependabot-core#7487
  2. Indeed, this looks like an issue to track in dependabot-core. Given a repro and a separate issue, I can confirm and maybe fix it over there.

@mburumaxwell
Copy link
Contributor

@AndrewCraswell I recently realized bountysource has issues, starting to look like a Ponzi scheme. No responses from their support and it appears that's been going on for a while.

bountysource/core#1539

I recommend that you seek a refund, via PayPal.

GH sponsors is the better alternative.

@AndrewCraswell
Copy link
Author

AndrewCraswell commented Aug 16, 2023

Ahh that's too bad. It used to be great Pre-covid. Sounds like things have gone downhill. I will look into GH Sponsors, unless you would prefer something more direct. Feel free to shoot me an email

@mburumaxwell
Copy link
Contributor

@hbendev Your second issue where you get the error message Illformed requirement ["workspace:*"] indeed occurs. when you have a workspace referencing another in a monorepo. I think dependabot/dependabot-core#7501 tracks that and related issues. If not, you could open a new issue and tag me along.

@AndrewCraswell , GH sponsors is okay and I believe more transparent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants