rules_buf icon indicating copy to clipboard operation
rules_buf copied to clipboard

Allow s390x arch

Open gongsu832 opened this issue 1 year ago • 7 comments

Allow rules to be used on s390x arch.

gongsu832 avatar Dec 27 '24 20:12 gongsu832

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Dec 27 '24 20:12 CLAassistant

Since this is a dependency of protobuf, the patch allows me to build protobuf on s390x successfully which I couldn't otherwise.

gongsu832 avatar Jan 18 '25 01:01 gongsu832

@srikrsna-buf what else is needed to get this merged?

jayakasadev avatar Mar 18 '25 13:03 jayakasadev

There is no binary for s390x available, the change in this PR will not be able to download buf

srikrsna-buf avatar Mar 18 '25 13:03 srikrsna-buf

There is no binary for s390x available, the change in this PR will not be able to download buf

Sorry can you be a bit more specific? Why is having a s390x binary for buf a prereq for this change?

gongsu832 avatar Mar 18 '25 13:03 gongsu832

The purpose of the _buf_download_releases_impl function you are editing is to download a version of buf.

There is no version of buf available for s390x: https://github.com/bufbuild/buf/releases/tag/v1.50.1

This PR would change the behavior from failing with an error message Unsupported operating system or cpu architecture to silently not downloaded a version of buf at all when cpu is s390x.

  • L139 won't match any known versions of buf when cpu is s390x
  • L148 will never execute when cpu is s390x

nicksnyder avatar Mar 18 '25 15:03 nicksnyder

Thank you @nicksnyder for the explanation. Would it be acceptable to add a check to fail the build if the binaries are not found in the sha list, like the following:

diff --git a/buf/internal/toolchain.bzl b/buf/internal/toolchain.bzl
index b2bcc40..03e69be 100644
--- a/buf/internal/toolchain.bzl
+++ b/buf/internal/toolchain.bzl
@@ -130,6 +130,7 @@ def _buf_download_releases_impl(ctx):
     ctx.file("WORKSPACE", "workspace(name = \"{name}\")".format(name = ctx.name))
     ctx.file("toolchain.bzl", _TOOLCHAIN_FILE)
     sha_list = ctx.read("sha256.txt").splitlines()
+    sha_found = False
     for sha_line in sha_list:
         if sha_line.strip(" ").endswith(".tar.gz"):
             continue
@@ -139,6 +140,7 @@ def _buf_download_releases_impl(ctx):
         if lower_case_bin.find(os) == -1 or lower_case_bin.find(cpu) == -1:
             continue

+        sha_found = True
         output = lower_case_bin[:lower_case_bin.find(os) - 1]
         if os == "windows":
             output += ".exe"
@@ -153,6 +155,9 @@ def _buf_download_releases_impl(ctx):
             output = output,
         )

+    if not sha_found:
+        fail("buf binaries not found in the release")
+
     if os == "darwin":
         os = "osx"

Ahh but that would make the build fail on s390x now. Hmmm...looks like we have to get bufbuild to release s390x binaries first.

gongsu832 avatar Mar 18 '25 17:03 gongsu832