Skip to content

Stop reading pnpm_lock from label_store, take 2#2395

Merged
jbedard merged 1 commit intoaspect-build:mainfrom
dzbarsky:zbarsky/label-store2
Oct 15, 2025
Merged

Stop reading pnpm_lock from label_store, take 2#2395
jbedard merged 1 commit intoaspect-build:mainfrom
dzbarsky:zbarsky/label-store2

Conversation

@dzbarsky
Copy link
Copy Markdown
Collaborator

@dzbarsky dzbarsky commented Oct 15, 2025

Changes are visible to end-users: yes/no

  • Searched for relevant documentation and updated as needed: yes/no
  • Breaking change (forces users to change their own code or config): yes/no
  • Suggested release notes appear below: yes/no

Test plan

  • Covered by existing test cases
  • New test cases added
  • Manual testing; please provide instructions so we can reproduce:

chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Oct 15, 2025
View reviewed changes
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

rules_js/npm/private/npm_translate_lock.bzl

Lines 163 to 167 in 8269efe

generate_repository_files(
rctx,
state.label_store.label("pnpm_lock"),
importers,
packages,

P0 Badge Update generate_repository_files call after removing pnpm_lock argument

The signature of generate_repository_files no longer accepts pnpm_lock_label, yet the call still passes it as the second positional parameter. As a result the label is bound to the importers parameter and the actual importer/package dictionaries shift one position, so helpers.get_npm_imports receives a Label instead of the expected mapping and will fail during repository rule execution. The call should drop the label argument and pass importers and packages in the updated order.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Comment thread npm/private/npm_translate_lock_state.bzl
@aspect-workflows
Copy link
Copy Markdown

aspect-workflows Bot commented Oct 15, 2025 •
edited
Loading

Test

All tests were cache hits

278 tests (100.0%) were fully cached saving 37s.

Test

e2e/bzlmod

All tests were cache hits

5 tests (100.0%) were fully cached saving 618ms.

Test

e2e/git_dep_metadata

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Test

e2e/gyp_no_install_script

All tests were cache hits

2 tests (100.0%) were fully cached saving 173ms.

Test

e2e/js_image_oci

All tests were cache hits

1 test (100.0%) was fully cached saving 7s.

Test

e2e/npm_link_package

All tests were cache hits

3 tests (100.0%) were fully cached saving 407ms.

Test

e2e/npm_link_package-esm

All tests were cache hits

3 tests (100.0%) were fully cached saving 484ms.

Test

e2e/npm_link_package-rerooted

All tests were cache hits

3 tests (100.0%) were fully cached saving 536ms.

Test

e2e/npm_translate_lock

All tests were cache hits

3 tests (100.0%) were fully cached saving 919ms.

Test

e2e/npm_translate_lock_disable_hooks

All tests were cache hits

3 tests (100.0%) were fully cached saving 247ms.

Test

e2e/npm_translate_lock_empty

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Test

e2e/npm_translate_lock_exclude_package_contents

All tests were cache hits

1 test (100.0%) was fully cached saving 34ms.

Test

e2e/npm_translate_lock_multi

All tests were cache hits

2 tests (100.0%) were fully cached saving 164ms.

Test

e2e/npm_translate_lock_partial_clone

All tests were cache hits

1 test (100.0%) was fully cached saving 61ms.

Test

e2e/npm_translate_lock_replace_packages

All tests were cache hits

3 tests (100.0%) were fully cached saving 295ms.

Test

e2e/npm_translate_lock_subdir_patch

All tests were cache hits

1 test (100.0%) was fully cached saving 95ms.

Test

e2e/npm_translate_package_lock

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Test

e2e/npm_translate_yarn_lock

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Test

e2e/package_json_module

All tests were cache hits

1 test (100.0%) was fully cached saving 348ms.

Test

e2e/patch_from_repo

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Test

e2e/pnpm_lockfiles

All tests were cache hits

67 tests (100.0%) were fully cached saving 9s.

Test

e2e/pnpm_repo_install

All tests were cache hits

1 test (100.0%) was fully cached saving 2s.

Test

e2e/pnpm_workspace

All tests were cache hits

15 tests (100.0%) were fully cached saving 3s.

Test

e2e/pnpm_workspace_deps

All tests were cache hits

3 tests (100.0%) were fully cached saving 901ms.

Test

e2e/pnpm_workspace_rerooted

All tests were cache hits

15 tests (100.0%) were fully cached saving 5s.

Test

e2e/repo_mapping

All tests were cache hits

3 tests (100.0%) were fully cached saving 454ms.

Test

e2e/rules_foo

All tests were cache hits

2 tests (100.0%) were fully cached saving 258ms.

Test

e2e/runfiles

All tests were cache hits

1 test (100.0%) was fully cached saving 118ms.

Test

e2e/stamped_package_json

All tests were cache hits

1 test (100.0%) was fully cached saving 44ms.

Test

e2e/vendored_node

All tests were cache hits

1 test (100.0%) was fully cached saving 78ms.

Test

e2e/vendored_tarfile

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

Test

e2e/verify_patches

All tests were cache hits

2 tests (100.0%) were fully cached saving 109ms.

Test

e2e/worker

All tests were cache hits

1 test (100.0%) was fully cached saving 35ms.

Test

e2e/workspace

All tests were cache hits

1 test (100.0%) was fully cached saving 35ms.

Buildifier      Format

@dzbarsky dzbarsky force-pushed the zbarsky/label-store2 branch 5 times, most recently from f45d0c9 to 6ba30b2 Compare October 15, 2025 17:17
@dzbarsky dzbarsky force-pushed the zbarsky/label-store2 branch from 6ba30b2 to 38618a9 Compare October 15, 2025 18:25
@jbedard jbedard merged commit f0ec5f7 into aspect-build:main Oct 15, 2025
99 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

@jbedard jbedard jbedard approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dzbarsky @jbedard