diff --git a/Changelog.md b/Changelog.md index 08a06be75..cf42bcf60 100644 --- a/Changelog.md +++ b/Changelog.md @@ -2,6 +2,7 @@ ## Unreleased +- Pnpm: `optionalDependencies` are now read from the lockfile. A project's own optional dependencies are reported as direct dependencies instead of transitive ones, and a package's optional dependencies are connected to it in the graph instead of appearing as unrelated transitive dependencies. Platform packages such as `fsevents`, `sharp`'s `@img/*` libraries, and the `@esbuild/*` binaries are the usual cases. ([#1766](https://github.com/fossas/fossa-cli/pull/1766)) - Diagnostics: When an error or warning group contains multiple errors, each error's `Traceback:` header is now printed on its own line instead of being glued onto the last line of the preceding error message (e.g. `...none passed validationTraceback:`). ([#1758](https://github.com/fossas/fossa-cli/pull/1758)) ## 3.18.2 diff --git a/src/Strategy/Node/Pnpm/PnpmLock.hs b/src/Strategy/Node/Pnpm/PnpmLock.hs index 8f631c716..3cc6e7fd1 100644 --- a/src/Strategy/Node/Pnpm/PnpmLock.hs +++ b/src/Strategy/Node/Pnpm/PnpmLock.hs @@ -186,7 +186,10 @@ buildGraphCore BuildGraphConfig{bgcGetPkgNameVersion, bgcMkPkgKey, bgcToEnv, bgc run . withLabeling applyLabels $ do -- Direct dependencies from each importer (workspace package). for_ (toList (lockfileImporters base)) $ \(_, projectImporters) -> do - for_ (Map.toList $ directDependencies projectImporters) $ \(depName, ProjectMapDepMetadata depVersion) -> + -- Optional dependencies are production dependencies an install may + -- skip on a platform that cannot use them; see 'ProjectMap'. + let prodDependencies = Map.toList (directDependencies projectImporters) <> Map.toList (directOptionalDependencies projectImporters) + for_ prodDependencies $ \(depName, ProjectMapDepMetadata depVersion) -> let resolvedVersion = resolveCatalogVersion catalogs depName depVersion in for_ (toResolvedDependency toEnv pkgs mkPkgKey depName resolvedVersion) $ \dep -> do direct dep diff --git a/src/Strategy/Node/Pnpm/Types.hs b/src/Strategy/Node/Pnpm/Types.hs index a2a93e73d..6fbd8d6c3 100644 --- a/src/Strategy/Node/Pnpm/Types.hs +++ b/src/Strategy/Node/Pnpm/Types.hs @@ -227,7 +227,8 @@ instance FromJSON PnpmLockFileSnapshots where let readTransitiveDepPairs = withObject "Parse dependencies" $ \ds -> do deps <- ds .:? "dependencies" .!= mempty - pure . HashMap.toList $ deps + optionalDeps <- ds .:? "optionalDependencies" .!= mempty + pure . HashMap.toList $ deps <> optionalDeps snapshots <- traverse readTransitiveDepPairs o -- Remove the peer dependency suffix. It's present in the snapshot entry, but it's not present in packages @@ -240,9 +241,16 @@ instance FromJSON PnpmLockFileSnapshots where -- Project map -- +-- | The direct dependencies of one importer (workspace package). +-- +-- pnpm lists optional dependencies separately only so that an install can skip +-- the ones its platform cannot use; for analysis they are direct dependencies +-- like any other, and leaving them out of the graph is what made platform +-- packages such as fsevents appear as unrelated transitive dependencies. data ProjectMap = ProjectMap { directDependencies :: Map Text ProjectMapDepMetadata , directDevDependencies :: Map Text ProjectMapDepMetadata + , directOptionalDependencies :: Map Text ProjectMapDepMetadata } deriving (Show, Eq, Ord) @@ -251,6 +259,7 @@ instance FromJSON ProjectMap where ProjectMap <$> obj .:? "dependencies" .!= mempty <*> obj .:? "devDependencies" .!= mempty + <*> obj .:? "optionalDependencies" .!= mempty newtype ProjectMapDepMetadata = ProjectMapDepMetadata { version :: Text @@ -283,7 +292,9 @@ instance FromJSON PackageData where <$> (obj .:? "dev" .!= False) <*> obj .:? "name" <*> obj .: "resolution" - <*> (obj .:? "dependencies" .!= mempty) + -- A package's optional dependencies are edges like any other; see + -- 'ProjectMap'. + <*> ((<>) <$> (obj .:? "dependencies" .!= mempty) <*> (obj .:? "optionalDependencies" .!= mempty)) <*> (obj .:? "peerDependencies" .!= mempty) data Resolution @@ -362,7 +373,8 @@ parseBaseLockfile (TextLike rawVer) obj = do packages <- obj .:? "packages" .!= mempty dependencies <- obj .:? "dependencies" .!= mempty devDependencies <- obj .:? "devDependencies" .!= mempty - let virtualRootWs = ProjectMap dependencies devDependencies + optionalDependencies <- obj .:? "optionalDependencies" .!= mempty + let virtualRootWs = ProjectMap dependencies devDependencies optionalDependencies let refinedImporters = if Map.null importers then Map.insert "." virtualRootWs importers diff --git a/test/Pnpm/PnpmLockSpec.hs b/test/Pnpm/PnpmLockSpec.hs index e6d37cf21..137ac8756 100644 --- a/test/Pnpm/PnpmLockSpec.hs +++ b/test/Pnpm/PnpmLockSpec.hs @@ -143,6 +143,27 @@ spec = do describe "works with pnpm v11 multi-document lockfile" $ checkGraph pnpmLockV11MultiDoc pnpmLockV9LocalDepSpec + -- Both fixtures declare sharp as an optional dependency of the project and + -- fsevents as an optional dependency of chokidar. + let pnpmLockV9Optional = currentDir $(mkRelFile "test/Pnpm/testdata/pnpm-9-optional-deps/pnpm-lock.yaml") + let pnpmLockV6Optional = currentDir $(mkRelFile "test/Pnpm/testdata/pnpm-lock-v6-optional.yaml") + + describe "optional dependencies in a v9 lockfile" $ + checkGraph pnpmLockV9Optional optionalDepsSpec + + describe "optional dependencies in a v6 lockfile" $ + checkGraph pnpmLockV6Optional optionalDepsSpec + +optionalDepsSpec :: Graphing Dependency -> Spec +optionalDepsSpec graph = do + it "should report the project's own optional dependencies as direct" $ + expectDirect [mkProdDep "chokidar@3.6.0", mkProdDep "sharp@0.33.0"] graph + + it "should connect a package's optional dependencies to it" $ do + expectEdge graph (mkProdDep "chokidar@3.6.0") (mkProdDep "readdirp@3.6.0") + expectEdge graph (mkProdDep "chokidar@3.6.0") (mkProdDep "fsevents@2.3.3") + expectDep (mkProdDep "fsevents@2.3.3") graph + pnpmLockGraphSpec :: Graphing Dependency -> Spec pnpmLockGraphSpec graph = do let hasEdge :: Dependency -> Dependency -> Expectation diff --git a/test/Pnpm/testdata/pnpm-9-optional-deps/pnpm-lock.yaml b/test/Pnpm/testdata/pnpm-9-optional-deps/pnpm-lock.yaml new file mode 100644 index 000000000..34ba1aa03 --- /dev/null +++ b/test/Pnpm/testdata/pnpm-9-optional-deps/pnpm-lock.yaml @@ -0,0 +1,56 @@ +lockfileVersion: '9.0' + +settings: + autoInstallPeers: true + excludeLinksFromLockfile: false + +# Optional dependencies at both levels: the importer declares sharp as an +# optionalDependency, and chokidar's snapshot lists fsevents under +# optionalDependencies. + +importers: + + .: + dependencies: + chokidar: + specifier: ^3.6.0 + version: 3.6.0 + optionalDependencies: + sharp: + specifier: ^0.33.0 + version: 0.33.0 + +packages: + + chokidar@3.6.0: + resolution: {integrity: sha512-7VT13fmjotKpGipCW9JEQAusEPE+Ei8nl6/g4FBAmIm0GOOLMua9NDDo/DWp0ZAxCr3cPq5ZpBqmPAQXDda2XQ==} + engines: {node: '>= 8.10.0'} + + fsevents@2.3.3: + resolution: {integrity: sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw==} + engines: {node: ^8.16.0 || ^10.6.0 || >=11.0.0} + os: [darwin] + + readdirp@3.6.0: + resolution: {integrity: sha512-hOS089on8RduqdbhvQ5Z37A0ESjsqz6qnRcffsMU3495FuTdqSm+7bhJ29JvIOsBDEEnan5DPu9t3To9VRlMzA==} + engines: {node: '>=8.10.0'} + + sharp@0.33.0: + resolution: {integrity: sha512-99qq0YpfgZUSaJ3wF0LDvDSg2fQVBH2PCLZIpimlzeHVAY4/2FuUJp8mCOEAiEnfy/zX9NHUyF7T4FblWQ/n9w==} + engines: {node: ^18.17.0 || ^20.3.0 || >=21.0.0} + +snapshots: + + chokidar@3.6.0: + dependencies: + readdirp: 3.6.0 + optionalDependencies: + fsevents: 2.3.3 + + fsevents@2.3.3: + optional: true + + readdirp@3.6.0: {} + + sharp@0.33.0: + optional: true diff --git a/test/Pnpm/testdata/pnpm-lock-v6-optional.yaml b/test/Pnpm/testdata/pnpm-lock-v6-optional.yaml new file mode 100644 index 000000000..4149f043a --- /dev/null +++ b/test/Pnpm/testdata/pnpm-lock-v6-optional.yaml @@ -0,0 +1,46 @@ +lockfileVersion: '6.0' + +# Optional dependencies in the v6 format: the root importer declares sharp as +# an optionalDependency, and chokidar's package entry lists fsevents under +# optionalDependencies. + +dependencies: + chokidar: + specifier: ^3.6.0 + version: 3.6.0 + +optionalDependencies: + sharp: + specifier: ^0.33.0 + version: 0.33.0 + +packages: + + /chokidar@3.6.0: + resolution: {integrity: sha512-7VT13fmjotKpGipCW9JEQAusEPE+Ei8nl6/g4FBAmIm0GOOLMua9NDDo/DWp0ZAxCr3cPq5ZpBqmPAQXDda2XQ==} + engines: {node: '>= 8.10.0'} + dependencies: + readdirp: 3.6.0 + optionalDependencies: + fsevents: 2.3.3 + dev: false + + /fsevents@2.3.3: + resolution: {integrity: sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw==} + engines: {node: ^8.16.0 || ^10.6.0 || >=11.0.0} + os: [darwin] + requiresBuild: true + dev: false + optional: true + + /readdirp@3.6.0: + resolution: {integrity: sha512-hOS089on8RduqdbhvQ5Z37A0ESjsqz6qnRcffsMU3495FuTdqSm+7bhJ29JvIOsBDEEnan5DPu9t3To9VRlMzA==} + engines: {node: '>=8.10.0'} + dev: false + + /sharp@0.33.0: + resolution: {integrity: sha512-99qq0YpfgZUSaJ3wF0LDvDSg2fQVBH2PCLZIpimlzeHVAY4/2FuUJp8mCOEAiEnfy/zX9NHUyF7T4FblWQ/n9w==} + engines: {node: ^18.17.0 || ^20.3.0 || >=21.0.0} + requiresBuild: true + dev: false + optional: true