From def5af07714731c83e4fe0f55c1e5dcedf7c4011 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Sun, 5 May 2024 23:01:20 -0700 Subject: [PATCH] refactor: remove support for reloading lockfile on update --- .github/workflows/ci.yaml | 9 +-------- docs/README.md | 8 ++++---- e2e/update_pnpm_lock/test.sh | 4 ++-- e2e/update_pnpm_lock_with_import/test.sh | 4 ++-- npm/private/npm_translate_lock.bzl | 8 ++------ npm/private/npm_translate_lock_state.bzl | 12 ------------ 6 files changed, 11 insertions(+), 34 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 0a2957a99..cfc9f05cf 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -151,10 +151,7 @@ jobs: # Don't run workspace smoke test under bzlmod - bzlmod: 1 folder: e2e/workspace - # Don't run update_pnpm_lock under bzlmod - - bzlmod: 1 - folder: e2e/update_pnpm_lock - # Don't run update_pnpm_lock under bzlmod + # Don't run npm_translate_lock_auth under bzlmod - bzlmod: 1 folder: e2e/npm_translate_lock_auth # rules_docker is not compatible with Bazel 7 @@ -172,10 +169,6 @@ jobs: folder: e2e/npm_link_package - bzlmod: 1 folder: e2e/rules_foo - - bazel-version: - major: 7 - bzlmod: 1 - folder: e2e/update_pnpm_lock_with_import # gyp_no_install_script is broken in an usual way on 6.5.0 # that is not worth investigating as we're dropping Bazel 6 support soon - bazel-version: diff --git a/docs/README.md b/docs/README.md index f704e9f89..934b6214d 100644 --- a/docs/README.md +++ b/docs/README.md @@ -68,10 +68,10 @@ using the identical version of pnpm that Bazel is configured with: $ bazel run -- @pnpm//:pnpm --dir $PWD install --lockfile-only ``` -Instead of checking in a `pnpm-lock.yaml` file, you could use a `package-lock.json` or `yarn.lock` -file with the `npm_package_lock`/`yarn_lock` attributes of `npm_translate_lock`. -If you do, rules_js will run `pnpm import` to generate a `pnpm-lock.yaml` file on-the-fly. -This is only recommended during migrations; see the notes about these attributes in the [migration guide](https://docs.aspect.build/guides/rules_js_migration). +During migration from npm or yarn you can still use a `package-lock.json` or `yarn.lock` as the source of truth by +setting the `npm_package_lock`/`yarn_lock` attributes of `npm_translate_lock`. When the `pnpm-lock.yaml` is out of date +rules_js will run `pnpm import` to generate the `pnpm-lock.yaml` file. +See the notes about these attributes in the [migration guide](https://docs.aspect.build/guides/rules_js_migration). Next, you'll typically use `npm_translate_lock` to translate the lock file to Starlark, which Bazel extensions understand. The `WORKSPACE` snippet you pasted above already contains this code. diff --git a/e2e/update_pnpm_lock/test.sh b/e2e/update_pnpm_lock/test.sh index 6889792fd..49c988027 100755 --- a/e2e/update_pnpm_lock/test.sh +++ b/e2e/update_pnpm_lock/test.sh @@ -54,8 +54,8 @@ ASPECT_RULES_JS_DISABLE_UPDATE_PNPM_LOCK= _sedi 's#"@types/node": "16"#"@types/node": "14"#' package.json # Trigger the update of the pnpm lockfile -if ! bazel run "$BZLMOD_FLAG" @npm//:sync; then - echo "ERROR: expected 'bazel run $BZLMOD_FLAG @npm//:sync' to pass" +if bazel run "$BZLMOD_FLAG" @npm//:sync; then + echo "ERROR: expected 'update_pnpm_lock' to exit with non-zero exit code" exit 1 fi diff --git a/e2e/update_pnpm_lock_with_import/test.sh b/e2e/update_pnpm_lock_with_import/test.sh index f199d0663..481895b0e 100755 --- a/e2e/update_pnpm_lock_with_import/test.sh +++ b/e2e/update_pnpm_lock_with_import/test.sh @@ -47,8 +47,8 @@ fi ASPECT_RULES_JS_FROZEN_PNPM_LOCK= print_step "It should update the lockfile after a running the invalide target with ASPECT_RULES_JS_FROZEN_PNPM_LOCK unset" -if ! bazel run "$BZLMOD_FLAG" @npm//:sync; then - echo "ERROR: expected 'bazel run $BZLMOD_FLAG @npm//:sync' to pass" +if bazel run "$BZLMOD_FLAG" @npm//:sync; then + echo "ERROR: expected 'update_pnpm_lock' to fail on update" exit 1 fi diff --git a/npm/private/npm_translate_lock.bzl b/npm/private/npm_translate_lock.bzl index 647c406f3..3405b86d8 100644 --- a/npm/private/npm_translate_lock.bzl +++ b/npm/private/npm_translate_lock.bzl @@ -102,17 +102,13 @@ def _npm_translate_lock_impl(rctx): if state.action_cache_miss(): _fail_if_frozen_pnpm_lock(rctx, state) if _update_pnpm_lock(rctx, state): - if rctx.attr.bzlmod: - msg = """ + msg = """ INFO: {} file updated. Please run your build again. See https://github.com/aspect-build/rules_js/issues/1445 """.format(state.label_store.relative_path("pnpm_lock")) - fail(msg) - else: - # If the pnpm lock file was changed then reload it before translation - state.reload_lockfile() + fail(msg) helpers.verify_node_modules_ignored(rctx, state.importers(), state.root_package()) diff --git a/npm/private/npm_translate_lock_state.bzl b/npm/private/npm_translate_lock_state.bzl index c6d90faeb..8bc923c7e 100644 --- a/npm/private/npm_translate_lock_state.bzl +++ b/npm/private/npm_translate_lock_state.bzl @@ -81,17 +81,6 @@ WARNING: `update_pnpm_lock` attribute in `npm_translate_lock(name = "{rctx_name} _copy_update_input_files(priv, rctx, label_store) _copy_unspecified_input_files(priv, rctx, label_store) -def _reload_lockfile(priv, rctx, label_store): - _load_lockfile(priv, rctx, label_store) - - if _should_update_pnpm_lock(priv): - _init_importer_labels(priv, label_store) - - _init_root_package(priv, rctx, label_store) - - if _should_update_pnpm_lock(priv): - _copy_unspecified_input_files(priv, rctx, label_store) - ################################################################################ def _validate_attrs(attr, is_windows): if is_windows and not attr.pnpm_lock: @@ -626,7 +615,6 @@ def _new(rctx): set_input_hash = lambda label, value: _set_input_hash(priv, label, value), action_cache_miss = lambda: _action_cache_miss(priv, rctx, label_store), write_action_cache = lambda: _write_action_cache(priv, rctx, label_store), - reload_lockfile = lambda: _reload_lockfile(priv, rctx, label_store), ) npm_translate_lock_state = struct(