Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use go's native cross-compilation for binaries #190

Merged
merged 18 commits into from
Apr 2, 2024

Conversation

tdeebswihart
Copy link
Collaborator

@tdeebswihart tdeebswihart commented Apr 1, 2024

What was changed

  1. Cross-compilation is done outside of the docker container, and images are copying in pre-built binaries
  2. All external dependencies are vendored using submodules instead of cloning (dockerize) or curling (cli)

Why?

Building this container now takes ~1m locally instead of over an hour on my aarch64 laptop.

Checklist

I tested this locally by running make build test

Copy link
Member

@dnr dnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General questions:

  • We were previously using released versions of cli. Now we're using a submodule. Are we planning to point it only to release tags? How do we manage that?
  • Should we ensure that the server embedded in the cli is the same as the server in this image? If we do want to do that, how? go mod replace?
  • Should we skip right to the new pre-release cli instead of the old-new one?

Makefile Outdated Show resolved Hide resolved
@tdeebswihart
Copy link
Collaborator Author

General questions:

* We were previously using released versions of cli. Now we're using a submodule. Are we planning to point it only to release tags? How do we manage that?

* Should we ensure that the server embedded in the cli is the same as the server in this image? If we do want to do that, how? go mod replace?

* Should we skip right to the new pre-release cli instead of the old-new one?
  1. No plan so far, but the old method of installation is being deprecated (I asked the SDK team on Friday) so will no longer work going forwards.
  2. Longer-term yes, and probably. I'd like to take that on in a separate PR however as it's not as vital as fixing our builds
  3. I'd be happy to, but we need to discuss with our SDK folks about what the differences are since that may change the UX of our admintools pod

@dnr
Copy link
Member

dnr commented Apr 1, 2024

  1. No plan so far, but the old method of installation is being deprecated (I asked the SDK team on Friday) so will no longer work going forwards.

We can still attempt to point to only official releases.

  1. I'd be happy to, but we need to discuss with our SDK folks about what the differences are since that may change the UX of our admintools pod

The old-new cli never had an official non-preview release so this shouldn't be a breaking change. Only tctl was officially supported.

Pattern rules to the rescue
@tdeebswihart
Copy link
Collaborator Author

  1. No plan so far, but the old method of installation is being deprecated (I asked the SDK team on Friday) so will no longer work going forwards.

We can still attempt to point to only official releases.

  1. I'd be happy to, but we need to discuss with our SDK folks about what the differences are since that may change the UX of our admintools pod

The old-new cli never had an official non-preview release so this shouldn't be a breaking change. Only tctl was officially supported.

We should only point to official releases, I agree. And I'll file a ticket to swap to the pre-release new CLI as follow-up work

We aren't guaranteed to have access to {} expansion
If you need to build and push locally remove the --set bit and change
--load to --push
This cleans up how we specify its target
The arm64 dockerize binary keeps getting deleted by make
@tdeebswihart tdeebswihart marked this pull request as ready for review April 1, 2024 21:24
@tdeebswihart tdeebswihart requested a review from a team as a code owner April 1, 2024 21:24
@tdeebswihart tdeebswihart requested a review from dnr April 1, 2024 21:24
.github/workflows/docker.yml Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Co-authored-by: David Reiss <david@temporal.io>
Makefile Show resolved Hide resolved
Makefile Show resolved Hide resolved
@tdeebswihart tdeebswihart merged commit f577b71 into main Apr 2, 2024
5 checks passed
@tdeebswihart tdeebswihart deleted the tds/build-externally branch April 2, 2024 20:36
tdeebswihart added a commit that referenced this pull request Apr 4, 2024
1. Cross-compilation is done outside of the docker container, and images are copying in pre-built binaries
2. All external dependencies are vendored using submodules instead of cloning (dockerize) or curling (cli)

Building this container now takes ~1m locally instead of over an hour on my aarch64 laptop.

I tested this locally by running `make build test`
tdeebswihart added a commit that referenced this pull request Apr 4, 2024
1. Cross-compilation is done outside of the docker container, and images are copying in pre-built binaries
2. All external dependencies are vendored using submodules instead of cloning (dockerize) or curling (cli)

Building this container now takes ~1m locally instead of over an hour on my aarch64 laptop.

I tested this locally by running `make build test`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants