Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the build step in librarian.yaml to use pnpm install with --frozen-lockfile and --ignore-scripts. The reviewer notes that the package URL and checksum still point to the older v5.3.0 release and suggests verifying if a newer version is required for this command.
| checksum: 4bc54d399b22c845fd4555f9435b7971153a2e3fafca32402658390711828f63 | ||
| build: | ||
| - pnpm install --ignore-workspace --no-frozen-lockfile --config.auto-install-peers=true | ||
| - pnpm install --frozen-lockfile --ignore-scripts |
There was a problem hiding this comment.
The pull request description indicates that pnpm install --frozen-lockfile --ignore-scripts should be used once the next gapic-generator tag is released. However, the package URL (line 25) and checksum (line 26) are still pointing to the older v5.3.0 release. Before flagging this version as invalid or outdated, please verify the version against other active repositories in the organization (e.g., google-cloud-go) to ensure consistency. If a newer version is indeed active and required for this command, please update the package version and checksum accordingly.
References
- When validating tool versions (such as protoc) in configuration files like librarian.yaml, verify the version against other active repositories in the organization (e.g., google-cloud-go) before flagging it as invalid.
|
Keeping this open as the tracker for restoring a reproducible generator build, but the target line has changed. The generator is staying standalone, not joining the root pnpm workspace (rationale in #9381). So Sequence to close this out:
Step 3 before step 2 will fail with Also note |
pnpm 7.33 reads the pinned generator tarball's lockfileVersion 6.0, so --frozen-lockfile still works. Defer the pnpm 10 bump until a new generator tag ships a 9.0 lockfile (#9407).
…en-lockfile-followup
…workspace (#9418) Stacked on #9413 — base is `chore/generator-remove-bazel`, so the diff here is consolidation only. GitHub retargets to `main` when #9413 merges. Makes `gapic-generator-typescript` a root workspace member: adds `core/generator/*` to `pnpm-workspace.yaml`, deletes the nested `pnpm-lock.yaml`, and drops the non-root `pnpm.overrides`. ### The cost: two casts in `generator.ts` The nested `pnpm.overrides.protobufjs: ^8.6.6` was doing real work. Without it the generator ends up with two protobufjs trees — its own v8, plus v7 pulled in by `proto3-json-serializer@4.0.2` — and `tsc` fails: ``` generator.ts(133,9): error TS2345: Argument of type '…protobufjs@8.8.0…Type' is not assignable to parameter of type 'protobuf.Type'. generator.ts(142,9): error TS2352: Conversion of type 'Message<{}>' to type 'Message<object>' may be a mistake… ``` So this PR adds a documented `SerializerType` alias and two `as unknown as` casts. > [!WARNING] > A root-level `pnpm.overrides` for protobufjs is **not** an alternative. 2cc19a6 tried it and broke all 18 `units` shards by forcing v8 onto google-gax; reverted in 05c2a62. ### The real fix `proto3-json-serializer` is **our own package** — `core/packages/proto3-json-serializer-nodejs`, currently `4.0.2` with `"protobufjs": "^7.5.4"`. Teaching it to accept protobufjs v8 and publishing that would delete these casts and unblock a v8 migration for google-gax too. Worth doing before or instead of this PR. ### Not included `librarian.yaml` still builds the pinned `gapic-generator-v5.3.0` tarball with `--ignore-workspace --frozen-lockfile`, which keeps working because that tag still ships a nested lockfile. It only needs to change once a tag is cut *after* this merges — see #9407. ### Verified `pnpm install --frozen-lockfile --filter @google-cloud/gapic-generator` → `compile` → `test`: **188 passing**.
1 participant
Use
pnpm install --frozen-lockfile --ignore-scriptsinlibrarian.yamlonce the nextgapic-generatortag is released after #9381 and thepackageandchecksuminlibrarian.yamlare updated to match