aspect-build / aspect-build/rules_js

[Bug]: Setting `ASPECT_RULES_JS_DISABLE_UPDATE_PNPM_LOCK` disables `update_pnpm_lock` until repo generated by `npm_translate_lock` is deleted

Open
#1,925 0 comments 0 reactions 0 assignees View on GitHub
bug need: investigation
Dominant language
Starlark
Stars
378
Forks
183
Avg merge
1d 11h
Merged PRs (30d)
29

Description

### What happened?

I recently configured my repo to set `update_pnpm_lock = True` in my `npm_translate_lock()` call in `MODULE.bazel` following all of the instructions described here: https://docs.aspect.build/rulesets/aspect_rules_js/docs/pnpm/#update_pnpm_lock

I am setting the `ASPECT_RULES_JS_DISABLE_UPDATE_PNPM_LOCK` environment variable to `1` in my CI environment generally, so that only a specific CI job/step reports if the pnpm lock file is outdated as described at the above link:

> If the `ASPECT_RULES_JS_DISABLE_UPDATE_PNPM_LOCK` environment variable is set, `update_pnpm_lock` is disabled even if set to True. This can be useful for some CI uses cases where multiple jobs run Bazel by you only want one of the jobs checking that the pnpm lock file is up-to-date.

However, I noticed that once Bazel has been run with the `ASPECT_RULES_JS_DISABLE_UPDATE_PNPM_LOCK` environment variable set, further invocations of Bazel will no longer respect `update_pnpm_lock = True` (i.e. if your `pnpm-lock.yaml` file is outdated then Bazel won't run `pnpm install ...` to update it nor fail if you set `ASPECT_RULES_JS_FROZEN_PNPM_LOCK=1`), even if I completely unset the `ASPECT_RULES_JS_DISABLE_UPDATE_PNPM_LOCK` environment variable (not just empty it but completely remove it from the environment, e.g. using `env --unset=ASPECT_RULES_JS_DISABLE_UPDATE_PNPM_LOCK bazel build ...`).

Interestingly the opposite direction works as desired, so if `ASPECT_RULES_JS_DISABLE_UPDATE_PNPM_LOCK=1` has never been set then Bazel respects `update_pnpm_lock = True`, but then once you do set `ASPECT_RULES_JS_DISABLE_UPDATE_PNPM_LOCK=1` then `update_pnpm_lock` will be disabled.

### Version

Development (host) and target OS/architectures:

```
❯ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=22.04
DISTRIB_CODENAME=jammy
DISTRIB_DESCRIPTION="Ubuntu 22.04.4 LTS"

❯ uname -m
x86_64
```

Output of `bazel --version`:

```
❯ bazel --version
bazel 7.2.1
```

Version of the Aspect rules, or other relevant rules from your
`WORKSPACE` or `MODULE.bazel` file:

Via `MODULE.bazel`, `1.42.3` from BCR

Language(s) and/or frameworks involved:
TypeScript, but maybe N/A to this issue

### How to reproduce

_No response_

### Any other information?

I'm currently using the following workaround in the specific CI job step that I want to check if the pnpm lock file is outdated, by deleting the repo generated by `npm_translate_lock` and then shutting down the Bazel server so that the next `bazel` invocation will recreate it but this time with `update_pnpm_lock = True` being respected as long as I also unset `ASPECT_RULES_JS_DISABLE_UPDATE_PNPM_LOCK`:

```
rm -rf "$(bazel info output_base)/external/aspect_rules_js~~npm~npm"
bazel shutdown # seems to be required, otherwise the next bazel invocation fails
env --unset=ASPECT_RULES_JS_DISABLE_UPDATE_PNPM_LOCK ASPECT_RULES_JS_FROZEN_PNPM_LOCK=1 bazel run '@@aspect_rules_js~~npm~npm//:sync'
```

I'm wondering if `rules_js` may need to use this feature of Bazel to declare more formally that it's sensitive to the `ASPECT_RULES_JS_DISABLE_UPDATE_PNPM_LOCK` environment variable: https://github.com/bazelbuild/bazel/issues/19511

I'm not sure `rules_js` is currently doing that either via the `environ` attribute of the `repository_rule`:
https://github.com/aspect-build/rules_js/blob/f0c67a25438454faf896ac6490ed9e3ff493ef1c/npm/private/npm_translate_lock.bzl#L146-L149

Nor using the `repository_ctx`'s `get_env()` method (instead `rctx.os.environ` appears to be used directly): https://github.com/aspect-build/rules_js/blob/f0c67a25438454faf896ac6490ed9e3ff493ef1c/npm/private/npm_translate_lock_state.bzl#L569-L571

Contributor guide

Open the contributing guide

Research direction

Start with npm/private/npm_translate_lock.bzl at the repository_rule definition and npm/private/npm_translate_lock_state.bzl around the rctx.os.environ usage. Reproduce the transition from ASPECT_RULES_JS_DISABLE_UPDATE_PNPM_LOCK=1 to an unset variable using Bazel 7.2.1, then verify that update_pnpm_lock and ASPECT_RULES_JS_FROZEN_PNPM_LOCK are respected without deleting the generated repository or shutting down Bazel.

Written by the indexing model from the issue text.

Assessment

Domain
build-system
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.