-
-
Notifications
You must be signed in to change notification settings - Fork 151
refactor: remove pnpm-workspace.yaml from label_store #2500
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.
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
base: 3.x
Are you sure you want to change the base?
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Add `{pnpm_workspace}` to the 'data' attribute of `npm_translate_lock(name = "{rctx_name}")` to suppress this warning. | ||
| """.format( | ||
| pnpm_lock = pnpm_lock_label, | ||
| pnpm_workspace = label_store.label(pnpm_workspace_key), | ||
| pnpm_workspace = pnpm_workspace_label, | ||
| rctx_name = priv["rctx_name"], | ||
| )) | ||
| if not utils.exists(rctx, label_store.path(pnpm_workspace_key)): | ||
| pnpm_workspace_path = rctx.path(pnpm_workspace_label) | ||
| if not pnpm_workspace_path.exists: | ||
| msg = "ERROR: expected `{path}` to exist since the `{pnpm_lock}` file contains workspace packages".format( | ||
| path = label_store.path(pnpm_workspace_key), | ||
| path = pnpm_workspace_path, | ||
| pnpm_lock = pnpm_lock_label, | ||
| ) | ||
| fail(msg) | ||
| _copy_input_file(priv, rctx, attr, label_store, pnpm_workspace_key) | ||
| if _should_update_pnpm_lock(priv): | ||
| # NB: rctx.read will convert binary files to text but that is acceptable for | ||
| # the purposes of calculating a hash of the file | ||
| _set_input_hash( | ||
| priv, | ||
| PNPM_WORKSPACE_FILENAME, | ||
| utils.hash(rctx.read(pnpm_workspace_path)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copy pnpm-workspace.yaml into external repo before running pnpm
When workspaces are present and pnpm-workspace.yaml is not listed in data, _copy_unspecified_input_files now only computes an input hash (lines 272‑278) and never copies the file into the external repository. The pnpm install --lockfile-only/import step later runs in the repository directory, so without copying the workspace file the command executes without any workspace definition and will fail or update the lockfile incorrectly. Prior to this refactor _copy_input_file copied the file; restoring that copy keeps auto‑updates functional for workspace projects.
Useful? React with 👍 / 👎.



Changes are visible to end-users: no
Test plan