fix: fix various build failures in the generator; update gax versions; update baselines#8237
fix: fix various build failures in the generator; update gax versions; update baselines#8237
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a custom Bazel repository rule to manage pnpm lockfiles, downgrades the google-gax dependency across multiple baselines and templates, and updates baseline API dumps with new proto field descriptions and deprecations. Additionally, it improves ESM support in the templater and refactors baseline update scripts to use parallel execution. Feedback identifies a significant anti-pattern in the new Bazel rule, which modifies the source tree during the loading phase, and points out inconsistencies between package.json and package-lock.json regarding dependency changes for prettier and @types/long.
| result = repository_ctx.execute( | ||
| ["node", pnpm_cjs_path, "--dir", workspace_root, "install", "--lockfile-only"], | ||
| working_directory = workspace_root, | ||
| ) |
There was a problem hiding this comment.
Modifying the source tree from a repository rule is a significant anti-pattern in Bazel. Repository rules are executed during the loading phase and should be hermetic, only affecting their own external repository directory. Running pnpm install --lockfile-only on the workspace root (workspace_root) can lead to non-deterministic builds, potential infinite loops if Bazel detects the change and restarts the loading phase, and failures in environments where the source tree is read-only (e.g., many CI systems).
Consider moving this lockfile refresh to a separate maintenance script or a CI check instead of running it as part of the Bazel workspace loading process.
There was a problem hiding this comment.
Funny, you wrote this code :P
Split off from: #8130 (#8018)
This fixes several issues keeping us from moving forward with generator work:
No tests are removed.