Skip to content

Commit

Permalink
refactor: remove support for reloading lockfile on update
Browse files Browse the repository at this point in the history
  • Loading branch information
jbedard committed May 7, 2024
1 parent 8bb680a commit b7919c1
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 35 deletions.
9 changes: 1 addition & 8 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -170,10 +167,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:
Expand Down
8 changes: 4 additions & 4 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 11 additions & 3 deletions e2e/update_pnpm_lock/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -53,25 +53,33 @@ ASPECT_RULES_JS_DISABLE_UPDATE_PNPM_LOCK=
# Have to make another change to package.json to invalidate the repository rule
_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"
# Trigger the update of the pnpm lockfile which should fail
if bazel run "$BZLMOD_FLAG" @npm//:sync; then
echo "ERROR: expected 'update_pnpm_lock' to exit with non-zero exit code on update"
exit 1
fi

# The lockfile should be updated
diff="$(git diff pnpm-lock.yaml)"
if [ -z "$diff" ]; then
echo "ERROR: expected 'git diff pnpm-lock.yaml' to not be empty"
exit 1
fi

# The action cache file should be updated
action_cache_file=".aspect/external_repository_action_cache/npm_translate_lock_LTE4Nzc1MDcwNjU="
diff="$(git diff "$action_cache_file")"
if [ -z "$diff" ]; then
echo "ERROR: expected 'git diff $action_cache_file' to not be empty"
exit 1
fi

# The lockfile has been updated and sync should now succeed
if ! bazel run "$BZLMOD_FLAG" @npm//:sync; then
echo "ERROR: expected 'update_pnpm_lock' to succeed once the lockfile is up to date"
exit 1
fi

if ! bazel test "$BZLMOD_FLAG" //...; then
echo "ERROR: expected 'bazel test $BZLMOD_FLAG //...' to pass"
exit 1
Expand Down
14 changes: 12 additions & 2 deletions e2e/update_pnpm_lock_with_import/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,24 +47,34 @@ 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"

# Trigger the update of the pnpm lockfile which should fail
if bazel run "$BZLMOD_FLAG" @npm//:sync; then
echo "ERROR: expected 'update_pnpm_lock' to exit with non-zero exit code on update"
exit 1
fi

# The lockfile should be updated
diff="$(git diff pnpm-lock.yaml)"
if [ -z "$diff" ]; then
echo "ERROR: expected 'git diff pnpm-lock.yaml' to not be empty"
exit 1
fi

# The action cache file should be updated
action_cache_file=".aspect/rules/external_repository_action_cache/npm_translate_lock_LTE4Nzc1MDcwNjU="
diff="$(git diff "$action_cache_file")"
if [ -z "$diff" ]; then
echo "ERROR: expected 'git diff $action_cache_file' to not be empty"
exit 1
fi

# The lockfile has been updated and sync should now succeed
if ! bazel run "$BZLMOD_FLAG" @npm//:sync; then
echo "ERROR: expected 'update_pnpm_lock' to succeed once the lockfile is up to date"
exit 1
fi

print_step "It should pass a bazel test run"

if ! bazel test "$BZLMOD_FLAG" //...; then
Expand Down
8 changes: 2 additions & 6 deletions npm/private/npm_translate_lock.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down
12 changes: 0 additions & 12 deletions npm/private/npm_translate_lock_state.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit b7919c1

Please sign in to comment.