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

enable windows/arm64 go build with bazel #3072

Merged
merged 7 commits into from
Mar 23, 2022

Conversation

niyas-sait
Copy link
Contributor

What type of PR is this?

Feature

What does this PR do? Why is it needed?

Enable windows/arm64 go build with Bazel

Other notes for review

Go for windows/arm64 is available from version 17. This patch adds the missing pieces to build go programs with bazel.

@niyas-sait
Copy link
Contributor Author

@linzhp Can you help with review please ?

Comment on lines 285 to 289
goos, goarch = "windows", "amd64"
if _get_env_var(ctx, "PROCESSOR_ARCHITECTURE") == "ARM64":
goarch = "arm64"
elif _get_env_var(ctx, "PROCESSOR_ARCHITEW6432") == "ARM64":
goarch = "arm64"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you try ctx.os.arch?

Suggested change
goos, goarch = "windows", "amd64"
if _get_env_var(ctx, "PROCESSOR_ARCHITECTURE") == "ARM64":
goarch = "arm64"
elif _get_env_var(ctx, "PROCESSOR_ARCHITEW6432") == "ARM64":
goarch = "arm64"
goos = "windows"
if ctx.os.arch == "<some archecture>":
goarch = "arm64"
else:
goarch = "amd64"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Much better! Seems to work

@niyas-sait
Copy link
Contributor Author

@linzhp Unfortunately, one test is failing on CI now with the latest change. Any guess?

Error: 'repository_os' value has no field or method 'arch'
  | (18:42:06) ERROR: C:/b/bk-windows-k0sj/bazel/rules-go-golang/WORKSPACE:9:23: fetching _go_download_sdk rule //external:go_sdk: Traceback (most recent call last):
  | File "C:/b/sz2aij3q/external/io_bazel_rules_go/go/private/sdk.bzl", line 47, column 45, in _go_download_sdk_impl
  | goos, goarch = _detect_host_platform(ctx)
  | File "C:/b/sz2aij3q/external/io_bazel_rules_go/go/private/sdk.bzl", line 280, column 18, in _detect_host_platform
  | if ctx.os.arch == "aarch64":
  | Error: 'repository_os' value has no field or method 'arch'
 

@linzhp
Copy link
Contributor

linzhp commented Mar 22, 2022

Yeah, repository_os.arch is only available in main branch of Bazel, not yet available in 5.0 release. We can either wait for 5.1 release, or ctx.os.environ.get("PROCESSOR_ARCHITECTURE")

@niyas-sait
Copy link
Contributor Author

Yeah, repository_os.arch is only available in main branch of Bazel, not yet available in 5.0 release. We can either wait for 5.1 release, or ctx.os.environ.get("PROCESSOR_ARCHITECTURE")

Thanks. Shall I revert latest commit and use previous approach of using environment variables ?

@linzhp
Copy link
Contributor

linzhp commented Mar 22, 2022

You can decide which way you want to go

@niyas-sait
Copy link
Contributor Author

You can decide which way you want to go

Ok I will use env. variable for the time being.

go/private/sdk.bzl Outdated Show resolved Hide resolved
@linzhp linzhp merged commit d606ba2 into bazel-contrib:master Mar 23, 2022
@niyas-sait
Copy link
Contributor Author

@linzhp Any idea when the next release is expected? I have a related PR for bazelisk that can be applied only after rules_go release

@linzhp
Copy link
Contributor

linzhp commented Apr 5, 2022

I am hoping to get #3083 in so next release will be for Go 1.18 support.

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.

2 participants