fix: Add M1/ARM support for the test suite

[fizx]

Fixes #515

[stephenh]

Thanks for the PR @fizx ! Just a few questions on whether we could avoid another docker-compose file, but thanks for proactively tackling this!

[stephenh]

Hm, could we avoid a separate docker-compose.aarch64.yml and instead do something like ARCH=${ARCH:-x86_64} and pass in ARCH from aliases.sh?

[stephenh]

Fwiw I think we could also make this:

FROM --platform=linux/amd64 node:17-alpine3.13

Which would tell docker to just always run this as x86, even on M1s...

I think I slightly prefer that for simplicity, b/c this is meant as just a helper to execute protoc and so I don't think the perf overhead of emulating the x86 on an M1 would be a big deal/afaiu.

@boukeversteegh do you have any preference on which approach?

[boukeversteegh]

@stephenh please see my reply below. reply-by-mail doesn't seem to work as well

[stephenh]

Ah yep! I saw it, was just deferring to @fizx ; I guess specifically I'd like to avoid introducing a 2nd docker-compose.yml, so Kyle I think it's up to you if you'd like to either:

• Update the current docker-compose.yml to take a $ARCH-type arg, or
• Update the FROM to just always use --platform=linux/amd64

Fwiw here's a snippet of how we use a "sometimes-set-but-default-in-case" build arg in one of our docker-compose.ymls:

  db:
    build:
      context: .
      dockerfile: ./db.dockerfile
      args:
        TEST_PARALLELISM: ${TEST_PARALLELISM:-8}

[fizx]

Specifying architectures in the FROM line has been a source of much pain for me when porting other code to M1/ARM. I hope the updated solution is simple enough.

[stephenh]

@fizx ah sure! I just know that it's a thing (pinning an architecture in FROM); I haven't really used it personally / don't own an M1, so happy to defer since that's been a source of issues.

The ARCH build arg looks great, thanks!

[boukeversteegh]

Nice to support the new macs of course! If it can be done with a single config for all systems, I'd say that this would be better. The eventual compilation of proto files to TS has to be fast too though, but then we need to decide whether or not providing an optimized dockerized protoc is within the scope of the project or not. I'd say for now that x86 sounds good if it does the job to keep the setup small and simple, but open to hear other ideas.

…

On Sat, Feb 19, 2022, 01:04 Stephen Haberman ***@***.***> wrote: ***@***.**** commented on this pull request. Thanks for the PR @fizx <https://github.com/fizx> ! Just a few questions on whether we could avoid another docker-compose file, but thanks for proactively tackling this! ------------------------------ In docker-compose.aarch64.yml <#516 (comment)>: > @@ -0,0 +1,11 @@ +version: "3" +services: + protoc: + build: + context: . + dockerfile: "protoc.Dockerfile" + args: + - "ARCH=aarch_64" Hm, could we avoid a separate docker-compose.aarch64.yml and instead do something like ARCH=${ARCH:-x86_64} and pass in ARCH from aliases.sh? ------------------------------ In protoc.Dockerfile <#516 (comment)>: > @@ -1,10 +1,11 @@ # Docker image for protoc FROM node:17-alpine3.14 Fwiw I think we could also make this: FROM --platform=linux/amd64 node:17-alpine3.13 Which would tell docker to just always run this as x86, even on M1s... I think I slightly prefer that for simplicity, b/c this is meant as just a helper to execute protoc and so I don't think the perf overhead of emulating the x86 on an M1 would be a big deal/afaiu. @boukeversteegh <https://github.com/boukeversteegh> do you have any preference on which approach? — Reply to this email directly, view it on GitHub <#516 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAANFJTS7S55662E2VLB7HTU33M7XANCNFSM5OZBDNPQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[fizx]

On my M1, the dockerfile now builds, and yarn build:test now works. Code should be cleaner too.

Building protoc
[+] Building 2.0s (13/13) FINISHED                                                                   
 => [internal] load build definition from protoc.Dockerfile                                     0.4s
 => => transferring dockerfile: 557B                                                            0.0s
 => [internal] load .dockerignore                                                               0.6s
 => => transferring context: 2B                                                                 0.0s
 => [internal] load metadata for docker.io/library/node:17-alpine3.14                           0.0s
 => [1/8] FROM docker.io/library/node:17-alpine3.14                                             0.0s
 => https://github.com/protocolbuffers/protobuf/releases/download/v3.19.1/protoc-3.19.1-linux-  0.7s
 => CACHED [2/8] RUN apk add bash                                                               0.0s
 => CACHED [3/8] RUN apk add gcompat                                                            0.0s
 => CACHED [4/8] ADD https://github.com/protocolbuffers/protobuf/releases/download/v3.19.1/pro  0.0s
 => CACHED [5/8] RUN mkdir /usr/local/lib/protoc && unzip protoc.zip -d /usr/local/lib/protoc   0.0s
 => CACHED [6/8] RUN ln -s /usr/local/lib/protoc/bin/protoc /usr/local/bin/protoc               0.0s
 => CACHED [7/8] RUN protoc --version                                                           0.0s
 => CACHED [8/8] WORKDIR /ts-proto/integration                                                  0.0s
 => exporting to image                                                                          0.5s
 => => exporting layers                                                                         0.0s
 => => writing image sha256:50522db9a36a653bc28e715475a618ebb3df240173187bb43cf3499936d81384    0.0s
 => => naming to docker.io/library/ts-proto_protoc                                              0.0s

Use 'docker scan' to run Snyk tests against images to find vulnerabilities and learn how to fix them

[stephenh]

🎉 This PR is included in version 1.106.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀