Skip to content

Conversation

@cobya
Copy link
Contributor

@cobya cobya commented May 15, 2024

After discussing with @CraigMacomber on #1028 we decided to take an experimental approach during rollout of PNPM lockfile v6 scanning. This PR is a modified version of the fork created with the experimental detector setup added along with some minor changes.

Original PR description:

This addresses #503 by doing the following:

  • Explicitly mark code specific to version 5 as "V5"
  • Add new code for V6.
  • Add version detection logic to select which code to run.
  • Add tests for new code.

The old code wasn't clear about what file formats it actually worked with, and a lot of its tests seems to be from versions earlier than 5 which it didn't really parse correctly (ex: the version it parses isn't present in those versions as it had a different name). Thus to preserve existing behavior I pointed cases which failed to detect a version through the old V5 code.

The included verification tests didn't cover a lot of cases (link and file references, aliased packages, shared shrink-wrap format, disambiguated deps with peer deps) so I also tested on the package lock from https://github.com/microsoft/FluidFramework which is 44k lines and has all the mentioned edge cases. If desired, I can add the v5 and v6 versions of that file to the verification tests directory.

This should be in a working state but I need to actually understand how the verification tests results are verifies before this should be merged.

This is my first time working in this codebase, as well as the first time in several years that I'm working in C#, so please be extra careful reviewing this: don't trust that I know what I'm doing.

@cobya cobya added version:minor type:feature Feature (new functionality) detector:pnpm The pnpm detector .NET Pull requests that update .net code labels May 15, 2024
@cobya cobya requested a review from a team as a code owner May 15, 2024 17:16
@cobya cobya requested a review from jcfiorenzano May 15, 2024 17:16
@codecov
Copy link

codecov bot commented May 15, 2024

Codecov Report

Attention: Patch coverage is 76.30058% with 41 lines in your changes are missing coverage. Please review.

Project coverage is 75.6%. Comparing base (d7118c4) to head (f45da9d).

Files Patch % Lines
...Detection.Detectors/pnpm/Pnpm6ComponentDetector.cs 71.4% 18 Missing and 6 partials ⚠️
...onentDetection.Detectors/pnpm/Contracts/Package.cs 37.5% 5 Missing ⚠️
...tDetection.Detectors/pnpm/PnpmComponentDetector.cs 73.6% 3 Missing and 2 partials ⚠️
...ntDetection.Detectors/pnpm/PnpmParsingUtilities.cs 90.6% 2 Missing and 2 partials ⚠️
...ponentDetection.Contracts/FileComponentDetector.cs 0.0% 1 Missing ⚠️
...ntDetection.Detectors/pnpm/Contracts/PnpmYamlV5.cs 66.6% 1 Missing ⚠️
...tDetection.Detectors/ruby/RubyComponentDetector.cs 0.0% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1110     +/-   ##
=======================================
- Coverage   75.7%   75.6%   -0.1%     
=======================================
  Files        239     245      +6     
  Lines      10822   10958    +136     
  Branches    1086    1100     +14     
=======================================
+ Hits        8193    8295    +102     
- Misses      2317    2343     +26     
- Partials     312     320      +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cobya cobya self-assigned this May 15, 2024
@github-actions
Copy link

github-actions bot commented May 15, 2024

👋 Hi! It looks like you modified some files in the Detectors folder.
You may need to bump the detector versions if any of the following scenarios apply:

  • The detector detects more or fewer components than before
  • The detector generates different parent/child graph relationships than before
  • The detector generates different devDependencies values than before

If none of the above scenarios apply, feel free to ignore this comment 🙂

@cobya cobya merged commit 95c12a8 into main May 15, 2024
@cobya cobya deleted the cobya/PNPM6 branch May 15, 2024 19:57
CraigMacomber added a commit to microsoft/FluidFramework that referenced this pull request May 21, 2024
## Description

Update pnpm to version 8. That means lock-file format version 6.

This relies on
microsoft/component-detection#1110 which seems
to be deployed and working now.

This gets us onto a supported version of pnpm according to
https://github.com/pnpm/pnpm/security.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

detector:pnpm The pnpm detector .NET Pull requests that update .net code type:feature Feature (new functionality) version:minor

Development

Successfully merging this pull request may close these issues.

2 participants