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

makefile: add support for arm64 platfrom #1251

Merged
merged 1 commit into from
Sep 10, 2019
Merged

Conversation

liusdu
Copy link

@liusdu liusdu commented Aug 9, 2019

it works well on my env

# file consul-template
consul-template: ELF 64-bit LSB executable, ARM aarch64, version 1 (SYSV), statically linked, Go BuildID=yBBsvpv1PEzSZNVawlQb/nVRSLofiQGfncQaZ4euB/7hXFeZH6_F2NAJC0gap1/f3JuZNrjcmTkEhzmJfzq, stripped
[root@localhost ~]# ./consul-template -v
consul-template v0.21.0 (0d999b3)

Signed-off-by: Liu Hua sdu.liu@huawei.com

@hashicorp-cla
Copy link

hashicorp-cla commented Aug 9, 2019

CLA assistant check
All committers have signed the CLA.

Signed-off-by: Liu Hua <sdu.liu@huawei.com>
@eikenb
Copy link
Contributor

eikenb commented Aug 20, 2019

Hey @liusdu, thanks for taking the time to submit the PR. I'd be happy to merge this but can't without the CLA. Can you sign it?

@liusdu
Copy link
Author

liusdu commented Sep 2, 2019

Hi @eikenb I cannot find what is wrong with it. Actually I signed the cla.
I think you can open a new pr and resend this code to fix this problem. I am ok about this~

@eikenb
Copy link
Contributor

eikenb commented Sep 3, 2019

@liusdu, that is very strange. I'm not sure what would have kept it from registering that you signed the CLA. It is a github thing that they don't provide much insight into.

I guess opening a new PR is the quickest/simplest path forward. Sorry for the inconvenience and thanks.

@eikenb
Copy link
Contributor

eikenb commented Sep 3, 2019

Hey @liusdu, a team-mate pointed out that while this PR was made by you the commit itself was made by @aarch64 and github won't consider the CLA valid until the person who did the commit has signed it. So if you could get @aarch64 to sign the CLA that should fix it. Thanks.

@liusdu
Copy link
Author

liusdu commented Sep 6, 2019

@evanphx Done! thank you very much!

@eikenb
Copy link
Contributor

eikenb commented Sep 10, 2019

FYI. I'm going to be changing all the ARM builds to enable CGO due to upstream Go bug: golang/go#32912. It hasn't been enabled before now, but wasn't a problem as I don't think anyone used them (else I suspect someone would have filed an issue). But I assume that you submitted this as you would want to use them. So working binaries are probably needed.

@eikenb eikenb merged commit ee91205 into hashicorp:master Sep 10, 2019
@eikenb eikenb added this to the 0.22.0 - Community PR focused milestone Sep 10, 2019
@eikenb
Copy link
Contributor

eikenb commented Sep 10, 2019

Didn't notice until after I merged it but the arm64 exclude entries (eg. freebsd/arm64) match both the arm and arm64 entries, preventing both.

After some playing around I decided to just keep the more restrictive arm builds (only build arm for Linux) that you have and just removed the '*/arm' entries. I think this is pretty safe as I don't think anyone is using the any of the arm builds as they should all be broken due to that golang bug.

If this ends up being an issue it is easy enough to rework the builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants