pjfanning opened a new issue, #8802:
URL: https://github.com/apache/texera/issues/8802

   ### What happened?
   
   The frontend's protobuf TypeScript bindings are committed to the repo and 
ship in the source release. They are not generated from the `.proto` files at 
build time.
   
   ```
   frontend/src/app/common/type/proto/org/apache/texera/amber/core/workflow.ts
   
frontend/src/app/common/type/proto/org/apache/texera/amber/core/virtualidentity.ts
   frontend/src/app/common/type/proto/scalapb/scalapb.ts
   frontend/src/app/common/type/proto/google/protobuf/descriptor.ts   (6,824 
lines)
   ```
   
   They are generated by hand with `bin/frontend-proto-gen.sh` (protoc + 
`ts-proto`). No build step or CI job runs that script or checks that the 
committed output matches the `.proto` files. The Python bindings, by contrast, 
are generated during the build by `bin/python-proto-gen.sh` (see `build.yml`, 
which installs the protoc version pinned in `bin/protoc-version.txt`) and are 
not committed.
   
   Problems with this:
   
   1. **The source release ships generated code whose source isn't in the 
release.** `google/protobuf/descriptor.ts` is generated from Google's 
`google/protobuf/descriptor.proto` (BSD-3-Clause), which reaches the generator 
through the import in `scalapb.proto`. That `.proto` comes from protoc's own 
include path and is not in the source tarball, so the release ships about 6.8k 
lines of generated code, including the comments copied from Google's `.proto`, 
without the source it came from. ASF source releases should contain the source, 
with generated artefacts produced by the build.
   2. **They drift.** Nothing keeps the committed `.ts` files in sync with 
`common/workflow-core/src/main/protobuf/**`. They have already gone stale once 
and had to be fixed (#4334). A `.proto` change that misses a manual 
regeneration builds without error and leaves the frontend and backend 
disagreeing about the wire types.
   3. **They need special handling for licence headers.** All four are listed 
in `.licenserc.yaml` `paths-ignore`, so those exemptions have to be maintained 
for files that shouldn't be in the source tree at all.
   
   **Suggested fix:** generate the frontend bindings during the build, the same 
way the Python ones are.
   - Run `bin/frontend-proto-gen.sh` (or an equivalent `yarn` script, e.g. a 
`prebuild` / `prestart` hook) as part of the frontend build, using the pinned 
`bin/protoc-version.txt` protoc and the `ts-proto` version already in 
`devDependencies`.
   - Delete `frontend/src/app/common/type/proto/` from git, add it to 
`.gitignore`, and remove its entries from `.licenserc.yaml`.
   - Make sure the frontend CI jobs and `bin/texera-web-application.dockerfile` 
install protoc before building.
   
   If generating at build time isn't wanted, the minimum fix is a CI check that 
regenerates the files and fails on `git diff`. That stops the drift, but point 
1 would remain.
   
   Related: the scalapb options file 
`common/workflow-core/src/main/protobuf/scalapb/scalapb.proto` is vendored from 
ScalaPB (Apache-2.0) but carries an ASF header and is missing from `LICENSE`. 
That's a separate fix, along the lines of #8775.
   
   ### How to reproduce?
   
   ```bash
   
B=https://dist.apache.org/repos/dist/dev/incubator/texera/1.2.1-incubating-RC1
   curl -sO $B/apache-texera-1.2.1-incubating-src.tar.gz
   tar xzf apache-texera-1.2.1-incubating-src.tar.gz
   cd apache-texera-1.2.1-incubating-src
   find frontend/src/app/common/type/proto -name '*.ts'
   head -5 frontend/src/app/common/type/proto/google/protobuf/descriptor.ts   # 
"source: google/protobuf/descriptor.proto"
   find . -name descriptor.proto                                              # 
not shipped
   ```
   
   ### Version/Branch
   
   1.2.1-incubating-RC1 (same layout on release/v1.3 and main)
   
   ### Commit Hash
   
   3f760c837086efa569b5820dacd60f292d6d4c3a
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to