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.

Sign up for GitHub

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

fix(builtin): detect yarn 2+ berry and adjust CLI args #3195

Merged
merged 1 commit into from
Jan 8, 2022
Merged

fix(builtin): detect yarn 2+ berry and adjust CLI args #3195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions docs/Built-ins.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1430,7 +1430,9 @@ check if yarn is being run by the `yarn_install` repository rule.

(*List of strings*): Arguments passed to yarn install.

See yarn CLI docs https://yarnpkg.com/en/docs/cli/install for complete list of supported arguments.
See yarn CLI docs for complete list of supported arguments.
Yarn 1: https://yarnpkg.com/en/docs/cli/install
Yarn 2+ (Berry): https://yarnpkg.com/cli/install
alexeagle marked this conversation as resolved.
Show resolved Hide resolved

Defaults to `[]`

Expand Down Expand Up @@ -1517,7 +1519,9 @@ Defaults to `False`

<h4 id="yarn_install-frozen_lockfile">frozen_lockfile</h4>

(*Boolean*): Use the `--frozen-lockfile` flag for yarn.
(*Boolean*): Use the `--frozen-lockfile` flag for yarn 1

Users of Yarn 2+ (Berry) should just pass `--immutable` to the `args` attribute.

Don't generate a `yarn.lock` lockfile and fail if an update is needed.

Expand Down Expand Up @@ -1841,9 +1845,11 @@ have bugs.
Disabling this attribute causes every run of yarn to have a unique
cache_directory.

If True, this rule will pass `--mutex network` to yarn to ensure that
If True and using Yarn 1, this rule will pass `--mutex network` to yarn to ensure that
the global cache can be shared by parallelized yarn_install rules.

The True value has no effect on Yarn 2+ (Berry).

If False, this rule will pass `--cache-folder /path/to/external/repository/__yarn_cache`
to yarn so that the local cache is contained within the external repository.

Expand Down
35 changes: 30 additions & 5 deletions internal/npm_install/npm_install.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,25 +813,43 @@ def _yarn_install_impl(repository_ctx):
is_windows_host = is_windows_os(repository_ctx)
node = repository_ctx.path(get_node_label(repository_ctx))
yarn = get_yarn_label(repository_ctx)
result = repository_ctx.execute(
[repository_ctx.path(yarn), "--version"],
timeout = repository_ctx.attr.timeout,
quiet = repository_ctx.attr.quiet,
)
if result.return_code:
fail("yarn --version failed: %s (%s)" % (result.stdout, result.stderr))
if result.stdout.startswith("1."):
yarn_version = "classic"
else:
yarn_version = "berry"
alexeagle marked this conversation as resolved.
Show resolved Hide resolved

yarn_args = []

# Set frozen lockfile as default install to install the exact version from the yarn.lock
# file. To perform an yarn install use the vendord yarn binary with:
# `bazel run @nodejs//:yarn install` or `bazel run @nodejs//:yarn install -- -D <dep-name>`
if repository_ctx.attr.frozen_lockfile:
yarn_args.append("--frozen-lockfile")
if yarn_version == "classic":
yarn_args.append("--frozen-lockfile")
else:
fail("--frozen-lockfile should not be used with yarn 2+. Just pass arguments like --immutable.")

if not repository_ctx.attr.use_global_yarn_cache:
yarn_args.extend(["--cache-folder", str(repository_ctx.path("_yarn_cache"))])
else:
elif yarn_version == "classic":
# 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"])
else:
# Can't tell from documentation if Yarn Berry has any replacement for the --mutex
# flag. We'll have to assume it's safe to run concurrently.
Comment on lines +850 to +851
Copy link

@merceyz merceyz Jan 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no replacement for it, we fixed the cache so it's atomic.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't tell from documentation if Yarn Berry has any replacement for the --mutex flag.

My observation on this:

There are a lot of knobs being exposed through .yarnrc file https://yarnpkg.com/configuration/yarnrc#enableGlobalCache so long-term it's probably best if we stop exposing each individual flags through the attributes of yarn_install rule.

Instead, we can create a yarnrc_file attribute to raise awareness to folks that they should be passing the config file explicitly to the rule. Underneath implementation, we just merge that with data and retain current behavior.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds like a good feature to me. However I don't see how to pass the config file path to yarn install - it looks like it's just automatically inferred from its location. Maybe this could be contributed separately. (maybe it already works just by passing .yarnrc.yml to the data of yarn_install and then it's auto-detected?)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, my understanding is that folks are already passing it through data and yarn auto detects it inside the sandbox.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yarn classic (v1) has flag called --use-yarnrc to specify the yarnrc location.

yarn berry searches for the rc file automatically and does not have this flag, but you could change the filename of the rc file to search for by setting the environment variable YARN_RC_FILENAME

pass
alexeagle marked this conversation as resolved.
Show resolved Hide resolved
yarn_args.extend(repository_ctx.attr.args)

# Run the package manager in the package.json folder
Expand Down Expand Up @@ -936,12 +954,17 @@ yarn_install = repository_rule(
"args": attr.string_list(
doc = """Arguments passed to yarn install.

See yarn CLI docs https://yarnpkg.com/en/docs/cli/install for complete list of supported arguments.""",
See yarn CLI docs for complete list of supported arguments.
Yarn 1: https://yarnpkg.com/en/docs/cli/install
Yarn 2+ (Berry): https://yarnpkg.com/cli/install
""",
default = [],
),
"frozen_lockfile": attr.bool(
default = True,
doc = """Use the `--frozen-lockfile` flag for yarn.
doc = """Use the `--frozen-lockfile` flag for yarn 1

Users of Yarn 2+ (Berry) should just pass `--immutable` to the `args` attribute.

Don't generate a `yarn.lock` lockfile and fail if an update is needed.

Expand All @@ -964,9 +987,11 @@ have bugs.
Disabling this attribute causes every run of yarn to have a unique
cache_directory.

If True, this rule will pass `--mutex network` to yarn to ensure that
If True and using Yarn 1, this rule will pass `--mutex network` to yarn to ensure that
the global cache can be shared by parallelized yarn_install rules.

The True value has no effect on Yarn 2+ (Berry).

If False, this rule will pass `--cache-folder /path/to/external/repository/__yarn_cache`
to yarn so that the local cache is contained within the external repository.
""",
Expand Down