build release binaries for all platforms #24
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- This allows us to build them all on the same machine if we want to.
- This just makes it more complex and isn't needed in real life.
- Because some final builds are not clean, and thus not release builds.
- Better represents purpose.
- Using the makefile to produce multi-platform binaries was getting too hard to read. Hopefully this makes it clearer.
samsalisbury
changed the title
samsalisbury
commented
Jan 12, 2023
| if err := fs.MkdirEmpty(c.Paths.TargetDir()); err != nil { | ||
| return err | ||
| } | ||
| return fs.Mkdirs(c.Paths.ZipDir(), c.Paths.MetaDir) |
There was a problem hiding this comment.
The change on line 149 above ensures that the target directory is empty at the start of the build process. This is pretty important for reproducibility. It wasn't noticed before, since we were using uniquely-named temporary directories for most builds. However, now we are building directly into dist/.../..., the issue came to light.
samsalisbury
requested review from
a team,
sarahethompson and
jeanneryan
and removed request for
a team
samsalisbury
commented
Jan 12, 2023
| # 2) INTERMEDIATE_BUILD - Some build metadata. | ||
| # 3) RELEASE_BUILD - All build metadata. | ||
| # 3) BOOTSTRAPPED_BUILD - All build metadata. |
There was a problem hiding this comment.
I renamed all these for hopefully the last time... I think we're consistent with naming everywhere now.
samsalisbury
commented
Jan 12, 2023
| @@ -1,5 +1,7 @@ | |||
| SHELL := /usr/bin/env bash -euo pipefail -c | |||
|
|
|||
| MAKEFLAGS := --jobs=10 | |||
There was a problem hiding this comment.
This speeds things up by default, on later versions of GNU Make.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Justification
Makes it quick and easy to build actions-go-build CLI binaries for all target platforms, via new make targets
make buildandmake release.This is a precursor to being able to use prebuilt binaries in the action, rather than having to compile the action CLI itself every time it is run, this should speed it up quite a lot.
Summary
The core is to add targets for building for all platforms to the Makefile. See
make buildandmake release. A lot of the build logic is moved out of the Makefile and into a scriptdev/buildwhich can be invoked by itself to run builds, e.g../dev/build all_from_scratchto build all binaries from scratch.Quality
This PR includes only one tiny change to behaviour: it ensures the build target directory is empty before running the build. This is essential for reproducibility, but hasn't caused issues until now (the new make targets surfaced the issue). That code path is already covered by countless tests, so I didn't add a new one.