Dockerised binding regeneration #257

keynmol · 2024-06-25T10:40:18Z

Fixes GRAPH-709

Several of us had problems with local environment trying to regenerate bindings.
We're switching to the dockerised build as a stopgap solution.

Test plan

new script will be used on CI

Dockerfile.bindings

.github/workflows/protobuf.yml

Development.md

Dockerfile.bindings

.dockerignore

keynmol · 2024-06-25T14:45:18Z

all done

This is the minimal version I could find that fits all criteria: 1. Determinstic output: tree-sitter/tree-sitter#2755 2. Published for ARM64 3. Tests for reprolang pass

keynmol · 2024-06-26T13:46:29Z

.tool-versions

@@ -1,5 +1,6 @@
 golang 1.19.10
-nodejs 16.7.0
+nodejs 16.20.2


Nodejs version in 16.x lineage that supports arm64 (for local development)

keynmol · 2024-06-26T13:47:53Z

cmd/scip/tests/reprolang/package.json

@@ -12,6 +12,6 @@
    "nan": "^2.15.0"
  },
  "devDependencies": {
-    "tree-sitter-cli": "^0.20.4"
+    "tree-sitter-cli": "0.21.0"


Minimal version that is deterministic (tree-sitter/tree-sitter#2755), and available from arm64 (for local work), and available on NPM: https://www.npmjs.com/package/tree-sitter-cli?activeTab=versions (0.20.9 is missing)

It's for tests only, so doesn't matter

keynmol · 2024-06-26T13:48:58Z

dev/proto-generate.sh

@@ -19,7 +19,7 @@ fi

 echo "--- Haskell ---"
 command -v cabal > /dev/null 2>&1 || { echo >&2 "Haskell's 'cabal' command is not installed. Please install it first via https://www.haskell.org/ghcup/"; exit 1; }
-cabal install proto-lens-protoc-0.7.1.1 ghc-source-gen-0.4.3.0 --overwrite-policy=always --ghc-options='-j2 +RTS -A32m' --installdir="$PWD/.bin"
+cabal install proto-lens-protoc-0.8.0.1 ghc-source-gen-0.4.5.0 --overwrite-policy=always --ghc-options='-j2 +RTS -A32m' --installdir="$PWD/.bin"


Bumps required because latest GHC doesn't support original versions...

keynmol · 2024-06-26T13:49:09Z

package.json

+    "build": "tsc --build --force bindings/typescript",
+    "prettier": "prettier --write --list-different '**/*.{ts,js(on)?,md,yml}'",
+    "prettier-check": "prettier --check '**/*.{ts,js(on)?,md,yml}'"


No need to reference node_modules directly

keynmol · 2024-06-26T14:08:43Z

.github/workflows/build-env-docker.yml

+          push: true
+          tags: ${{ steps.meta.outputs.tags }}
+          labels: ${{ steps.meta.outputs.labels }}
+          platforms: linux/amd64,linux/arm64


While this is slow (VERY slow), this workflow runs only on main and nothing depends on it - so it finishes when it finishes.

kritzcreek

Using the container I can build all the bindings locally so I'm happy with this. Had one question about the image name

kritzcreek · 2024-06-26T15:28:31Z

.github/workflows/protobuf-reprolang.yml

+          username: ${{ github.actor }}
+          password: ${{ secrets.GITHUB_TOKEN }}
+
+      - run: docker pull ghcr.io/sourcegraph/scip:latest || echo "no suitable cache"


Why is it scip-bindings-env everywhere else, but just scip here?

Ah, last place where I forgot to update it.
Frankly I don't think this helps at all, but I will still update the image once I'm back at the computer

varungandhi-src · 2024-06-27T05:57:41Z

.github/workflows/build-env-docker.yml

+# Configures this workflow to run every time a change is pushed to the branch called `release`.
+on:
+  push:
+    branches: ['main']


Should we limit running this only to specific commits? I'm worried we might get rate-limited if we try making a few changes in a single day given the size of the image. However, the docs don't mention any limits for total pull/push. https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-container-registry

🤔

I think with Github's own registry rate limiting is not really an issue, but I will at least limit the concurrency of this job to avoid producing intermediate docker images for commits that aren't top of branch.

varungandhi-src · 2024-06-27T06:01:44Z

.github/workflows/protobuf-reprolang.yml

+          # We're changing the owner of the checkout folder to a particular user id,
+          # matching the user id of `asdf` user we create inside the docker container.
+          sudo chown -R 1001:1001 . && ./dev/generate-all-in-docker.sh


Is this needed because otherwise we might get permission errors inside the container? I don't understand why we need to create a separate asdf user inside the container.

The reason for this is that I failed to make asdf work with a root user - there were some errors coming out from asdf itself which I didn't have time to debug.

It's an eyesore to have to do this, but to avoid it we'd need to make asdf work with root – which might not be worth the time, given the entire docker container is just a minimal hack to get back to reproducibility.

kritzcreek · 2024-06-27T07:52:20Z

Hmm looks like the image publishing step failed on my PR after merging:

https://github.com/sourcegraph/scip/actions/runs/9691568032/job/26743288072

should we rip that out for now?

keynmol · 2024-06-27T09:04:40Z

I will fix that workflow, I have a working one I can just copypaste.

varungandhi-src reviewed Jun 25, 2024

View reviewed changes

Dockerfile.bindings Outdated Show resolved Hide resolved

keynmol added 2 commits June 25, 2024 13:23

Dockerised build for binding regeneration

20b4dce

Preload haskell dependencies as they're by far the slowest

2ffd6a8

keynmol force-pushed the docker-for-bindings-generator branch from 217fd22 to 2ffd6a8 Compare June 25, 2024 12:37

keynmol added 2 commits June 25, 2024 14:10

Pin image to a concrete SHA

feb7137

README

8130c6c

keynmol marked this pull request as ready for review June 25, 2024 13:16

keynmol changed the title ~~WIP docker container for bindings generation~~ Dockerised binding regeneration Jun 25, 2024

keynmol added 2 commits June 25, 2024 14:17

Use dockerised proto command

6785b05

Run prettier

39946f8

keynmol commented Jun 25, 2024

View reviewed changes

.github/workflows/protobuf.yml Outdated Show resolved Hide resolved

varungandhi-src reviewed Jun 25, 2024

View reviewed changes

Development.md Outdated Show resolved Hide resolved

varungandhi-src reviewed Jun 25, 2024

View reviewed changes

Dockerfile.bindings Outdated Show resolved Hide resolved

varungandhi-src reviewed Jun 25, 2024

View reviewed changes

.dockerignore Outdated Show resolved Hide resolved

keynmol added 4 commits June 25, 2024 14:37

Include reprolang parser in generation

c792e11

Move dockerfile to ./dev, update .dockerignore

f45a87d

Unify workflows that test generated code

55f0f8e

Not a tty

9f23ec2

keynmol added 11 commits June 26, 2024 10:12

Chown/chmod the target folder

49b58d4

Publish image on main for remote cache

611cc99

formatting and test publishing

82f944b

WIP

f52e6ec

WIP

ff59c0e

Hack around node_modules issue by moving them to a non-mounted location

45cdad9

Can't write to .yarnrc either...

70dd331

pls make it stahp

a96164a

WIP

3b25b36

WIP

2e7f823

Cleanup

2e2a60c

keynmol added 2 commits June 26, 2024 14:15

Ensure we generate TS code for ABI v13

66c03fa

Upgrade TS to 0.21.0

795aeda

This is the minimal version I could find that fits all criteria: 1. Determinstic output: tree-sitter/tree-sitter#2755 2. Published for ARM64 3. Tests for reprolang pass

keynmol commented Jun 26, 2024

View reviewed changes

keynmol requested review from varungandhi-src and kritzcreek June 26, 2024 13:53

keynmol commented Jun 26, 2024

View reviewed changes

Rename docker image to clarify what it is

e114ec5

kritzcreek approved these changes Jun 26, 2024

View reviewed changes

Change image name

7fd96dd

keynmol merged commit 76a272e into main Jun 27, 2024
6 checks passed

keynmol deleted the docker-for-bindings-generator branch June 27, 2024 05:28

varungandhi-src reviewed Jun 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dockerised binding regeneration #257

Dockerised binding regeneration #257

keynmol commented Jun 25, 2024 •

edited

Loading

keynmol commented Jun 25, 2024 •

edited

Loading

keynmol Jun 26, 2024

keynmol Jun 26, 2024 •

edited

Loading

keynmol Jun 26, 2024

keynmol Jun 26, 2024

keynmol Jun 26, 2024

keynmol Jun 26, 2024

kritzcreek left a comment

kritzcreek Jun 26, 2024

keynmol Jun 26, 2024

varungandhi-src Jun 27, 2024

keynmol Jun 27, 2024

varungandhi-src Jun 27, 2024

keynmol Jun 27, 2024

kritzcreek commented Jun 27, 2024

keynmol commented Jun 27, 2024

Dockerised binding regeneration #257

Dockerised binding regeneration #257

Conversation

keynmol commented Jun 25, 2024 • edited Loading

Test plan

keynmol commented Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

keynmol Jun 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kritzcreek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kritzcreek commented Jun 27, 2024

keynmol commented Jun 27, 2024

keynmol commented Jun 25, 2024 •

edited

Loading

keynmol commented Jun 25, 2024 •

edited

Loading

keynmol Jun 26, 2024 •

edited

Loading