Skip to content

Commit

Permalink
Rewrite makefile to be reliable (#4034)
Browse files Browse the repository at this point in the history
Phase 2 of makefile cleanups!

As a tl;dr, what this PR does is:
- makes the high-level build steps explicit so they're easier to follow (right near the top)
- switches to Make-only book-keeping files, so steps are reliably recalculated in all situations
- moves tool-bins to a .bin folder so they are not deleted by default, and versions them more strictly (you can now switch between versions for "free", great for e.g. trying new protocs)
- adds a _reliable_ way to avoid codegen in buildkite / wherever it's needed or desired

As a bonus, this should now be safe to always run in parallel, which runs about 2x as fast for me:
```
> time make bins
...
make bins  104.18s user 38.68s system 310% cpu 45.941 total

> time make bins -j8
...
make bins -j8  108.43s user 53.00s system 717% cpu 22.489 total
```
I'll change Buildkite to do this in a bit, when it has been battle-tested a bit more.

Next (and final) phase is to make test-running a bit more normal, but that needs a bit more work.

---

As a brief summary about why book-keeping files are important, be aware of these two details:
1. Make _only_ looks at file modification times to determine what to do.  If A depends on B and B is newer than A (or A does not exist), it'll re-build A.
2. Git (and other tools) _do not_ record modification times, to work better with Make.  This is why `touch go.mod` doesn't lead to anything to commit - git doesn't record _when_ you modified it, only what the modifications were.

Because of this, on e.g. a brand new `git clone`, _all_ files are "the same" age... with some randomness about what ones are newer than others, within e.g. milliseconds.  So as far as Make is concerned, a bunch of _random_ files have been modified.  If you had makefile rules like `output.go: input.go \n do codegen`, some of those output files may be newer than their inputs, so they may or may not be re-made.

That's pretty clearly wrong.  The same kind of thing happens when you change branches - depending on what changed between your before/after commits, those files are now "new".

The way out of this madness is to rely on files that e.g. Git does _not_ modify: these book-keeping files.
- When cloning (or clean), they do not exist, so they must all be re-made.
- When changing branches, any changed files are _always_ newer than these book-keeping files, so they will _always_ be re-made (when appropriate).

So that's what this PR does.
  • Loading branch information
Groxx committed Mar 9, 2021
1 parent 0c46f3d commit 1b3436c
Show file tree
Hide file tree
Showing 4 changed files with 348 additions and 218 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
.tmp/
.vscode/
.build/
.bin/
/vendor
/_vendor-*
/cadence
Expand Down
2 changes: 2 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ RUN go mod download

COPY . .

# bypass codegen, use committed files. must be run separately, before building things.
RUN make .faux-codegen
RUN CGO_ENABLED=0 make copyright cadence-cassandra-tool cadence-sql-tool cadence cadence-server


Expand Down
Loading

0 comments on commit 1b3436c

Please sign in to comment.