Keep generated go files in the git repo

[matzf]

• Add generated go/proto files to git

• Add make target make gogen to generate go/proto files
  Generate *.capnp.go and structs.gen.go files in bazel and then copy to source directory in a make target.

  For the case of structs.gen.go which is generated with a simple shell script, this setup is somewhat overcomplicated. Generating the *.capnp.go files does use bazel infrastructure to get the capnp tooling.
  To keep things symmetric, both cases are handled the same.

• Add CI check generated go/proto files
  Detect uncommitted changes to generated go/proto files by cleaning and
  regenerating the files.

Fixes #2822

---

This change is 

[matzf]

I had to setup a workaround for a bug in misspell that created false positives in one of the generated capnp.go files. I've created a PR to fix it (client9/misspell#157), but unfortunately it appears that the misspell maintainer is no longer around.

[sustrik]

Reviewed 21 of 23 files at r1, 4 of 4 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matzf)

---

scion.sh, line 338 at r2 (raw file):

    if [ -n "$out" ]; then echo "$out"; ret=1; fi
    lint_step "misspell"
    $TMPDIR/misspell -source go -error $LOCAL_DIRS || ret=1

Why is this needed? Isn't it inferred based on the file extension?

[matzf]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @sustrik)

---

scion.sh, line 338 at r2 (raw file):

Previously, sustrik (Martin Sustrik) wrote…

Why is this needed? Isn't it inferred based on the file extension?

This is the workaround I hinted at in the previous comment; the problem is that misspell has a false positive in one of the generated files. It just happens that this false positive is not found when running in go mode, because in this mode only comments are checked. Somewhat surprisingly, the source type is not inferred in the default auto mode -- it just always seems to use plain.

An alternative workaround could be to explicitly exclude the generated files (like linelen does, just above). Would you prefer that?

[sustrik]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matzf)

---

scion.sh, line 338 at r2 (raw file):

Previously, matzf (Matthias Frei) wrote…

This is the workaround I hinted at in the previous comment; the problem is that misspell has a false positive in one of the generated files. It just happens that this false positive is not found when running in go mode, because in this mode only comments are checked. Somewhat surprisingly, the source type is not inferred in the default auto mode -- it just always seems to use plain.

An alternative workaround could be to explicitly exclude the generated files (like linelen does, just above). Would you prefer that?

Yes, that sounds cleaner.

[matzf]

Reviewable status: 24 of 25 files reviewed, 1 unresolved discussion (waiting on @sustrik)

---

scion.sh, line 338 at r2 (raw file):

Previously, sustrik (Martin Sustrik) wrote…

Yes, that sounds cleaner.

Done.

[sustrik]

Reviewed 1 of 1 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved

[kormat]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @matzf)

a discussion (no related file):
I'm wondering about including gogen as part of the bazel target. It's very light-weight when nothing has changed, and it means it'll instantly show up changes in the user's working tree, without needing a CI run to catch it.

---

[kormat]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @matzf)

---

Makefile, line 18 at r3 (raw file):

gogen_clean:
	rm go/proto/*.gen.go go/proto/*.capnp.go || true

Use rm -f instead of || true. That prevents errors being output when the files don't exist.

[kormat]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @matzf)

---

Makefile, line 15 at r3 (raw file):

gogen: gogen_clean
	bazel build //go/proto:structs //go/proto:capnp
	cp --no-preserve=mode bazel-genfiles/go/proto/gogen/*.gen.go bazel-genfiles/go/proto/gogen/*.capnp.go go/proto/

What is it you want to skip in go/proto/gogen/*?

[matzf]

Reviewable status: 24 of 25 files reviewed, 3 unresolved discussions (waiting on @kormat, @matzf, and @sustrik)

a discussion (no related file):

Previously, kormat (Stephen Shirley) wrote…

I'm wondering about including gogen as part of the bazel target. It's very light-weight when nothing has changed, and it means it'll instantly show up changes in the user's working tree, without needing a CI run to catch it.

Makes sense, done. I've added it to all, not bazel though, seems cleaner.

---

[kormat]

Reviewable status: 24 of 25 files reviewed, 4 unresolved discussions (waiting on @kormat, @matzf, and @sustrik)

---

Makefile, line 11 at r4 (raw file):

all: tags clibs dispatcher bazel

clean: gogenlinks_clean

You removed the clean target - please don't do that :)

[matzf]

Reviewable status: 24 of 25 files reviewed, 4 unresolved discussions (waiting on @kormat and @sustrik)

---

Makefile, line 15 at r3 (raw file):

Previously, kormat (Stephen Shirley) wrote…

What is it you want to skip in go/proto/gogen/*?

Done. Nothing, it seems.

---

Makefile, line 18 at r3 (raw file):

Previously, kormat (Stephen Shirley) wrote…

Use rm -f instead of || true. That prevents errors being output when the files don't exist.

Done. I wish there was a flag that does that without also forcing removal of read only files...

---

Makefile, line 11 at r4 (raw file):

Previously, kormat (Stephen Shirley) wrote…

You removed the clean target - please don't do that :)

Oops, done.

[kormat]

Reviewed 20 of 23 files at r1, 3 of 4 files at r2, 1 of 1 files at r3, 19 of 19 files at r5.
Reviewable status: complete! all files reviewed, all discussions resolved