Yarn 1.x: Parse packages from yarn.lock #693

taylormadore · 2024-10-21T13:41:37Z

This PR creates "Packages" for a Yarn 1.x project and all project dependencies. These "Packages" will later be resolved into SBOM Components in future PRs.

The yarn.lock file is the principal source for the fully resolved set of direct project dependencies and transitive dependencies (Yarn workspaces are an exception to this and must be parsed from package.json).

The pyarn library will be used to parse yarn.lock. There is a related PR in that repository that must be merged and released first. It adds handling for additional styles of path-based version specifiers.

Maintainers will complete the following section

Commit messages are descriptive enough
Code coverage from testing does not decrease and new code is covered
Docs updated (if applicable)
Docs links in the code are still valid (if docs were updated)

Note: if the contribution is external (not from an organization member), the CI
pipeline will not run automatically. After verifying that the CI is safe to run:

approve GitHub Actions workflows by clicking a button
approve the Red Hat Trusted App Pipeline container build by commenting /ok-to-test
(as is the standard for Pipelines as Code)

a-ovchinnikov

Mostly LGTM, a couple of small nitpicks.

cachi2/core/package_managers/yarn_classic/resolver.py

tests/unit/package_managers/yarn_classic/test_resolver.py

cachi2/core/package_managers/yarn_classic/main.py

cachi2/core/package_managers/yarn_classic/resolver.py

tests/unit/package_managers/yarn_classic/test_resolver.py

requirements-extras.txt

cachi2/core/package_managers/yarn_classic/main.py

eskultety · 2024-10-24T09:09:48Z

pyproject.toml

@@ -28,6 +28,7 @@ dependencies = [
  "gitpython",
  "packageurl-python",
  "packaging",
+  "pyarn@https://github.com/taylormadore/pyarn/archive/refs/heads/add-alternate-file-dep-style.tar.gz",


Only raising a comment here so that we don't forget to update this before merging.

Updated to the released version

pyproject.toml

requirements-extras.txt

eskultety · 2024-10-24T09:13:30Z

cachi2/core/package_managers/yarn_classic/resolver.py

@@ -186,6 +187,22 @@ def _get_packages_from_lockfile(source_dir: RootedPath) -> list[YarnClassicPacka
    return [_classify_pyarn_package(source_dir, package) for package in pyarn_packages]


+def _get_main_package(source_dir: RootedPath) -> WorkspacePackage:
+    """Return a WorkspacePackage for the main package in package.json."""
+    # TODO: Use yarn classic project files once available


I'm lacking context with this message, can you please elaborate for me?

I pulled in the PackageJson class from the yarn module here. It should eventually be replaced by whatever the result of #705 is once merged

Uhm, apparently github failed to include my response in this thread when submitting the review, so repeating:

I pulled in the PackageJson class from the yarn module here. It should eventually be replaced by whatever the result of #705 is once merged

Oh, but this means it's going to be super easy to forget to update this accordingly given there's already working logic. This creates a direct dependency on #705 then.

eskultety · 2024-10-24T09:16:10Z

cachi2/core/package_managers/yarn_classic/resolver.py

+def _classify_pyarn_package(source_dir: RootedPath, package: PYarnPackage) -> YarnClassicPackage:
+    """Return a YarnClassicPackage corresponding to a PYarnPackage."""
+
+    def assert_package_has_relative_path(package: PYarnPackage) -> None:
+        if package.path and Path(package.path).is_absolute():
+            raise PackageRejected(
+                (
+                    f"The package {package.name}@{package.version} has an absolute path "
+                    f"({package.path}), which is not permitted."
+                ),
+                solution="Ensure that file/link packages in yarn.lock do not have absolute paths.",
+            )
+
+    if _is_from_npm_registry(package.url):
+        return RegistryPackage(
+            name=package.name,
+            version=package.version,
+            integrity=package.checksum,
+        )
+    elif package.path is not None:
+        # Ensure path is not absolute
+        assert_package_has_relative_path(package)
+        # Ensure path is within the repository root
+        path = source_dir.join_within_root(package.path)
+        # File packages have a url, whereas link packages do not
+        if package.url:
+            return FilePackage(
+                name=package.name,
+                version=package.version,
+                relpath=path.subpath_from_root,
+                integrity=package.checksum,
+            )
+        return LinkPackage(
+            name=package.name,
+            version=package.version,
+            relpath=path.subpath_from_root,
+        )
+    elif _is_git_url(package.url):
+        return GitPackage(
+            name=package.name,
+            version=package.version,
+            url=package.url,
+        )
+    elif _is_tarball_url(package.url):
+        return UrlPackage(
+            name=package.name,
+            version=package.version,
+            url=package.url,
+            integrity=package.checksum,
+        )
+    else:
+        raise UnexpectedFormat(
+            (
+                "Cachi2 could not determine the package type for the following package in "
+                f"yarn.lock: {vars(package)}"
+            ),
+            solution=(
+                "Ensure yarn.lock is well-formed and if so, report this error to the Cachi2 team"
+            ),
+        )


nitpick: Do you think that strictly from OOP's POV we'd benefit from a YarnClassicPackageFactory factory class holding this logic? Functionally, there's no difference, so you can ignore.

cachi2/core/package_managers/yarn_classic/resolver.py

eskultety

ACK, but we should IMO wait for #705 and merge the pyarn change in the meantime.

The workspace path globs in package.json are relative to the workspace root. Workspace paths need to be resolved relative to the workspace root (the request package path) rather than the request source dir to prevent workspaces in package subdirectories from being missed by the glob Signed-off-by: Taylor Madore <[email protected]>

Move responsibility for the prefetch and component resolution from fetch_yarn_sources to be more consistent with how other package managers are implemented Signed-off-by: Taylor Madore <[email protected]>

pip-compile appears to want to add propcache to requirements.txt and also update the ordering and comments of some of the other requirements Signed-off-by: Taylor Madore <[email protected]>

These correspond to the different package types that can be resolved in a Yarn 1.x yarn.lock file Signed-off-by: Taylor Madore <[email protected]>

Resolves containerbuildsystem#629 Use the pyarn library to parse yarn.lock and get the list of project dependencies. Next classify them into specific package types that can later be used to generate SBOM components with PURLs. Some of the logic used to classify the package types was inspired by or taken directly from the official yarn sources. Signed-off-by: Taylor Madore <[email protected]>

Signed-off-by: Taylor Madore <[email protected]>

slimreaper35 · 2024-10-31T19:03:59Z

cachi2/core/package_managers/yarn_classic/resolver.py

+class RegistryPackage(_BasePackage):
+    """A Yarn 1.x package from the registry."""


While working on generating SBOM I saw that there is an optional qualifier for repository_url:
https://github.com/package-url/purl-spec/blob/master/PURL-SPECIFICATION.rst#known-qualifiers-keyvalue-pairs

The default repository/registry for NPM is https://registry.npmjs.org so the second npm registry that we support is "https://registry.yarnpkg.com could/should be potentially included in the PURL.

Am I missing something ?

a-ovchinnikov reviewed Oct 21, 2024

View reviewed changes

cachi2/core/package_managers/yarn_classic/resolver.py Show resolved Hide resolved

cachi2/core/package_managers/yarn_classic/resolver.py Outdated Show resolved Hide resolved

tests/unit/package_managers/yarn_classic/test_resolver.py Outdated Show resolved Hide resolved

taylormadore force-pushed the parse-yarn-lock branch from 047a31a to 0c6cd40 Compare October 23, 2024 01:52

a-ovchinnikov reviewed Oct 23, 2024

View reviewed changes

taylormadore force-pushed the parse-yarn-lock branch from 1d5caee to 990ff46 Compare October 23, 2024 13:45

taylormadore marked this pull request as ready for review October 23, 2024 13:45

eskultety reviewed Oct 24, 2024

View reviewed changes

taylormadore force-pushed the parse-yarn-lock branch from 990ff46 to b3b350d Compare October 24, 2024 18:32

eskultety reviewed Oct 25, 2024

View reviewed changes

a-ovchinnikov approved these changes Oct 25, 2024

View reviewed changes

taylormadore force-pushed the parse-yarn-lock branch from b3b350d to d758544 Compare October 30, 2024 13:55

taylormadore added 4 commits October 30, 2024 11:56

yarn-classic: add resolve_yarn_project

154b2c4

Move responsibility for the prefetch and component resolution from fetch_yarn_sources to be more consistent with how other package managers are implemented Signed-off-by: Taylor Madore <[email protected]>

pip-compile: update requirements files

4b73cd2

pip-compile appears to want to add propcache to requirements.txt and also update the ordering and comments of some of the other requirements Signed-off-by: Taylor Madore <[email protected]>

yarn-classic: add models for parsing yarn packages

b1a635b

These correspond to the different package types that can be resolved in a Yarn 1.x yarn.lock file Signed-off-by: Taylor Madore <[email protected]>

taylormadore force-pushed the parse-yarn-lock branch from d758544 to 740a843 Compare October 30, 2024 19:43

taylormadore added 3 commits October 30, 2024 16:06

yarn-classic: get main package metadata

1bc17af

Signed-off-by: Taylor Madore <[email protected]>

yarn-classic: get workspace packages for workspaces

5a83727

Signed-off-by: Taylor Madore <[email protected]>

taylormadore force-pushed the parse-yarn-lock branch from 740a843 to 5a83727 Compare October 30, 2024 20:08

slimreaper35 reviewed Oct 31, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Yarn 1.x: Parse packages from yarn.lock #693

Yarn 1.x: Parse packages from yarn.lock #693

taylormadore commented Oct 21, 2024 •

edited

Loading

a-ovchinnikov left a comment

eskultety Oct 24, 2024

taylormadore Oct 30, 2024

eskultety Oct 24, 2024

taylormadore Oct 24, 2024

eskultety Oct 25, 2024

eskultety Oct 24, 2024

taylormadore Oct 30, 2024

eskultety left a comment

slimreaper35 Oct 31, 2024

		class RegistryPackage(_BasePackage):
		"""A Yarn 1.x package from the registry."""

Yarn 1.x: Parse packages from yarn.lock #693

Are you sure you want to change the base?

Yarn 1.x: Parse packages from yarn.lock #693

Conversation

taylormadore commented Oct 21, 2024 • edited Loading

Maintainers will complete the following section

a-ovchinnikov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eskultety left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taylormadore commented Oct 21, 2024 •

edited

Loading