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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

yarn_versions helper only includes v1 #3071

Closed
Karibash opened this issue Nov 10, 2021 · 12 comments 路 Fixed by #3195
Closed

yarn_versions helper only includes v1 #3071

Karibash opened this issue Nov 10, 2021 · 12 comments 路 Fixed by #3195
Labels
Can Close? We will close this in 30 days if there is no further activity

Comments

@Karibash
Copy link

Karibash commented Nov 10, 2021

馃殌 Yarn Berry Support

Relevant Rules

yarn_install

Description

When trying to use Yarn berry with the yarn_install rule, an error occurs because it implicitly sets the --mutex network, which is not supported by Yarn Berry.
The following implementation shows that the mutex option can be disabled by setting the use_global_yarn_cache to false, but the --cache-folder is implicitly set next.

if not repository_ctx.attr.use_global_yarn_cache:
yarn_args.extend(["--cache-folder", str(repository_ctx.path("_yarn_cache"))])
else:
# Multiple yarn rules cannot run simultaneously using a shared cache.
# See https://github.com/yarnpkg/yarn/issues/683
# The --mutex option ensures only one yarn runs at a time, see
# https://yarnpkg.com/en/docs/cli#toc-concurrency-and-mutex
# The shared cache is not necessarily hermetic, but we need to cache downloaded
# artifacts somewhere, so we rely on yarn to be correct.
yarn_args.extend(["--mutex", "network"])

In Yarn Berry, setting the --cache-folder in the CLI is deprecated, so an error will occur in this case as well.
https://github.com/yarnpkg/berry/blob/83311e1d4dec7fb6c0ead21f37efa9b687be58b9/packages/plugin-essentials/sources/commands/install.ts#L211-L219

The following is an example of WORKSPACE.bazel for using Yarn Berry.

http_archive(
    name = "vendored_yarn_3_1_0",
    build_file_content = """exports_files(["vendored_yarn_3_1_0/berry--yarnpkg-cli-3.1.0/packages/yarnpkg-cli/bin/yarn.js"])""",
    urls = ["https://github.com/yarnpkg/berry/archive/refs/tags/@yarnpkg/cli/3.1.0.tar.gz"],
)

load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories")
node_repositories(
    vendored_yarn = "@vendored_yarn_3_1_0//:berry--yarnpkg-cli-3.1.0/packages/yarnpkg-cli",
)

load("@build_bazel_rules_nodejs//:index.bzl", "yarn_install")
yarn_install(
    name = "npm",
    args = ["--immutable"],
    frozen_lockfile = False,
    use_global_yarn_cache = False,
    data = [
      "@//:.yarnrc.yml",
      "@vendored_yarn_3_1_0//:berry--yarnpkg-cli-3.1.0/packages/yarnpkg-cli/bin/yarn.js",
    ],
    package_json = "//:package.json",
    yarn_lock = "//:yarn.lock",
)

Describe the solution you'd like

The ideas suggested in the issue comments below are helpful.

  1. add a flag to remove the automatic addition of --mutex network
  2. add a bool attribute yarn2_compatible that does not use --mutex network and replaces --frozen-lockfile with --immutable
  3. detect that yarn2 is used and behave accordingly

#1599 (comment)

Personally, I think the 1st idea is better, as it seems to be easy to implement without destructive changes.
An example is shown below.

yarn_install(
    name = "npm",
    args = ["--immutable"],
    frozen_lockfile = False,
-   use_global_yarn_cache = False,
+   use_mutex = False,
    data = [
      "@//:.yarnrc.yml",
      "@vendored_yarn_3_1_0//:berry--yarnpkg-cli-3.1.0/packages/yarnpkg-cli/bin/yarn.js",
    ],
    package_json = "//:package.json",
    yarn_lock = "//:yarn.lock",
)
@lummax
Copy link

lummax commented Jan 14, 2022

https://github.com/bazelbuild/rules_nodejs/blob/d2f9c4137543c3ed8477ca3aa73b2aa0fcbd7fe9/nodejs/private/yarn_versions.bzl while that comment still refers to an open issue, the list of supported yarn versions could be extended now.

But that could require modifications of the download functions, as yarn berry is available as https://github.com/yarnpkg/berry/raw/%40yarnpkg/cli/{version}/packages/yarnpkg-cli/bin/yarn.js as a single *.js file.

@alexeagle
Copy link
Collaborator

I noticed from the example above that https://github.com/yarnpkg/berry/archive/refs/tags/@yarnpkg/cli/3.1.0.tar.gz is over 150MB file so I don't think that's quite the right workaround either.

@alexeagle
Copy link
Collaborator

Re-opening this issue to get the yarn_versions builtin to support more versions.

@alexeagle alexeagle reopened this Jan 14, 2022
@alexeagle alexeagle changed the title Yarn Berry Support yarn_versions helper only includes v1 Jan 14, 2022
alexeagle added a commit that referenced this issue Jan 17, 2022
@vincent-lecrubier-skydio

@alexeagle I see you made a PR for this and closed it. Would you mind explaining the current status on this please?

@sgammon
Copy link
Contributor

sgammon commented May 5, 2022

friendly ping?

alexeagle added a commit that referenced this issue May 5, 2022
@alexeagle
Copy link
Collaborator

I checked out that pr again, and commented there. It doesn't actually work, and I don't have a client who needs it. Contributions welcome of course 馃榿

@github-actions
Copy link

github-actions bot commented Nov 5, 2022

This issue has been automatically marked as stale because it has not had any activity for 6 months. It will be closed if no further activity occurs in 30 days. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

@github-actions github-actions bot added the Can Close? We will close this in 30 days if there is no further activity label Nov 5, 2022
@alexeagle
Copy link
Collaborator

Just to wrap this up, rules_nodejs is no longer maintained and rules_js standardized on pnpm because it's lockfile allows us to port the node_modules layout to starlark, which allows lazy install of only needed packages for the requested build and test targets

@sgammon
Copy link
Contributor

sgammon commented Nov 8, 2022

dumb question but that means pnpm is the way to go and yarn berry won't be supported, right? @alexeagle

@sgammon
Copy link
Contributor

sgammon commented Nov 8, 2022

that does sound like a major gain

@github-actions github-actions bot removed the Can Close? We will close this in 30 days if there is no further activity label Nov 9, 2022
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had any activity for 6 months. It will be closed if no further activity occurs in 30 days. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

@github-actions github-actions bot added the Can Close? We will close this in 30 days if there is no further activity label May 12, 2023
@github-actions
Copy link

This issue was automatically closed because it went 30 days without any activity since it was labeled "Can Close?"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Can Close? We will close this in 30 days if there is no further activity
Projects
None yet
5 participants