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

s390x arch support #2557

Closed
wants to merge 8 commits into from
Closed

Conversation

Repana-Chowdappa
Copy link

Hi team,
I am from the IBM Z (s390x) porting team. We would like to open a PR for s390x support for rust rules.

Copy link

google-cla bot commented Mar 18, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@daivinhtran
Copy link
Contributor

LGTM. Would you mind signing CLA and fixing CI failures?

@daivinhtran daivinhtran self-requested a review March 18, 2024 13:36
@UebelAndre
Copy link
Collaborator

This also looks good to me but things still fail in CI.

@gongsu832
Copy link

Besides the changes in the PR, I think you also need changes in crate_universe/private/vendor_utils.bzl, like the following:

diff --git a/crate_universe/private/vendor_utils.bzl b/crate_universe/private/vendor_utils.bzl
index 71476c99..afb438d5 100644
--- a/crate_universe/private/vendor_utils.bzl
+++ b/crate_universe/private/vendor_utils.bzl
@@ -10,6 +10,7 @@ _BUILDIFIER_SHA256S = {
     "buildifier-darwin-arm64": "4da23315f0dccabf878c8227fddbccf35545b23b3cb6225bfcf3107689cc4364",
     "buildifier-linux-amd64": "3ed7358c7c6a1ca216dc566e9054fd0b97a1482cb0b7e61092be887d42615c5d",
     "buildifier-linux-arm64": "c657c628fca72b7e0446f1a542231722a10ba4321597bd6f6249a5da6060b6ff",
+    "buildifier-linux-s390x": "c657c628fca72b7e0446f1a542231722a10ba4321597bd6f6249a5da6060b6ff",
     "buildifier-windows-amd64.exe": "45e13b2951e4c611d346dacdaf0aafaa484045a3e7300fbc5dd01a896a688177",
 }

@@ -54,6 +55,12 @@ def crates_vendor_deps_targets():
         visibility = ["//visibility:public"],
     )

+    native.config_setting(
+        name = "linux_s390x",
+        constraint_values = ["@platforms//os:linux", "@platforms//cpu:s390x"],
+        visibility = ["//visibility:public"],
+    )
+
     native.config_setting(
         name = "macos_amd64",
         constraint_values = ["@platforms//os:macos", "@platforms//cpu:x86_64"],
@@ -77,6 +84,7 @@ def crates_vendor_deps_targets():
         actual = select({
             ":linux_amd64": "@cargo_bazel.buildifier-linux-amd64//file",
             ":linux_arm64": "@cargo_bazel.buildifier-linux-arm64//file",
+            ":linux_s390x": "@cargo_bazel.buildifier-linux-s390x//file",
             ":macos_amd64": "@cargo_bazel.buildifier-darwin-amd64//file",
             ":macos_arm64": "@cargo_bazel.buildifier-darwin-arm64//file",
             ":windows": "@cargo_bazel.buildifier-windows-amd64.exe//file",

The problem though is that we don't have a downloadable s390x buildifier from buildtools repo, yet.

@Repana-Chowdappa
Copy link
Author

Repana-Chowdappa commented Apr 15, 2024

@UebelAndre - Please let me know if we need to raised another PR for below changes?

diff --git a/crate_universe/3rdparty/crates/BUILD.linux-raw-sys-0.3.8.bazel b/crate_universe/3rdparty/crates/BUILD.linux-raw-sys-0.3.8.bazel
index 4dff3b8c..29a78f56 100644
--- a/crate_universe/3rdparty/crates/BUILD.linux-raw-sys-0.3.8.bazel
+++ b/crate_universe/3rdparty/crates/BUILD.linux-raw-sys-0.3.8.bazel
@@ -51,6 +51,9 @@ rust_library(
         "@rules_rust//rust/platform:x86_64-unknown-linux-gnu": [
             "errno",  # x86_64-unknown-linux-gnu
         ],
+        "@rules_rust//rust/platform:s390x-unknown-linux-gnu": [
+            "errno",
+        ],
         "@rules_rust//rust/platform:x86_64-unknown-nixos-gnu": [
             "errno",  # x86_64-unknown-nixos-gnu
         ],
diff --git a/crate_universe/3rdparty/crates/BUILD.linux-raw-sys-0.4.10.bazel b/crate_universe/3rdparty/crates/BUILD.linux-raw-sys-0.4.10.bazel
index 295e54d3..17774ebd 100644
--- a/crate_universe/3rdparty/crates/BUILD.linux-raw-sys-0.4.10.bazel
+++ b/crate_universe/3rdparty/crates/BUILD.linux-raw-sys-0.4.10.bazel
@@ -57,6 +57,10 @@ rust_library(
             "elf",  # x86_64-unknown-linux-gnu
             "errno",  # x86_64-unknown-linux-gnu
         ],
+        "@rules_rust//rust/platform:s390x-unknown-linux-gnu": [
+            "elf",  # s390x-unknown-linux-gnu
+            "errno",  # s390x-unknown-linux-gnu
+        ],
         "@rules_rust//rust/platform:x86_64-unknown-nixos-gnu": [
             "elf",  # x86_64-unknown-nixos-gnu
             "errno",  # x86_64-unknown-nixos-gnu
diff --git a/crate_universe/private/vendor_utils.bzl b/crate_universe/private/vendor_utils.bzl
index 71476c99..f5475a84 100644
--- a/crate_universe/private/vendor_utils.bzl
+++ b/crate_universe/private/vendor_utils.bzl
@@ -9,6 +9,7 @@ _BUILDIFIER_SHA256S = {
     "buildifier-darwin-amd64": "2cb0a54683633ef6de4e0491072e22e66ac9c6389051432b76200deeeeaf93fb",
     "buildifier-darwin-arm64": "4da23315f0dccabf878c8227fddbccf35545b23b3cb6225bfcf3107689cc4364",
     "buildifier-linux-amd64": "3ed7358c7c6a1ca216dc566e9054fd0b97a1482cb0b7e61092be887d42615c5d",
+    "buildifier-linux-s390x": "3ed7358c7c6a1ca216dc566e9054fd0b97a1482cb0b7e61092be887d42615c5d",
     "buildifier-linux-arm64": "c657c628fca72b7e0446f1a542231722a10ba4321597bd6f6249a5da6060b6ff",
     "buildifier-windows-amd64.exe": "45e13b2951e4c611d346dacdaf0aafaa484045a3e7300fbc5dd01a896a688177",
 }
@@ -54,6 +55,12 @@ def crates_vendor_deps_targets():
         visibility = ["//visibility:public"],
     )
 
+    native.config_setting(
+        name = "linux_s390x",
+        constraint_values = ["@platforms//os:linux", "@platforms//cpu:s390x"],
+        visibility = ["//visibility:public"],
+    )
+
     native.config_setting(
         name = "macos_amd64",
         constraint_values = ["@platforms//os:macos", "@platforms//cpu:x86_64"],
@@ -77,6 +84,7 @@ def crates_vendor_deps_targets():
         actual = select({
             ":linux_amd64": "@cargo_bazel.buildifier-linux-amd64//file",
             ":linux_arm64": "@cargo_bazel.buildifier-linux-arm64//file",
+            ":linux_s390x": "@cargo_bazel.buildifier-linux-amd64//file",
             ":macos_amd64": "@cargo_bazel.buildifier-darwin-amd64//file",
             ":macos_arm64": "@cargo_bazel.buildifier-darwin-arm64//file",
             ":windows": "@cargo_bazel.buildifier-windows-amd64.exe//file",
diff --git a/test/unit/native_deps/native_deps_test.bzl b/test/unit/native_deps/native_deps_test.bzl
index b38748da..d4825cb5 100644
--- a/test/unit/native_deps/native_deps_test.bzl
+++ b/test/unit/native_deps/native_deps_test.bzl
@@ -161,6 +161,13 @@ def _bin_has_native_dep_and_alwayslink_test_impl(ctx):
                 "bazel-out/x64_windows-{}/bin/{}test/unit/native_deps/alwayslink.lo.lib".format(compilation_mode, workspace_prefix),
                 "-Wl,--no-whole-archive",
             ]
+    elif toolchain.target_arch == "s390x":
+        want = [
+            "-lstatic=native_dep",
+            "link-arg=-Wl,--whole-archive",
+            "link-arg=bazel-out/s390x-{}/bin/{}test/unit/native_deps/libalwayslink.lo".format(compilation_mode, workspace_prefix),
+            "link-arg=-Wl,--no-whole-archive",
+        ]
     else:
         want = [
             "-lstatic=native_dep",
@@ -207,6 +214,13 @@ def _cdylib_has_native_dep_and_alwayslink_test_impl(ctx):
                 "bazel-out/x64_windows-{}/bin/{}test/unit/native_deps/alwayslink.lo.lib".format(compilation_mode, workspace_prefix),
                 "-Wl,--no-whole-archive",
             ]
+    elif toolchain.target_arch == "s390x":
+        want = [
+            "-lstatic=native_dep{}".format(pic_suffix),
+            "link-arg=-Wl,--whole-archive",
+            "link-arg=bazel-out/s390x-{}/bin/{}test/unit/native_deps/libalwayslink{}.lo".format(compilation_mode, workspace_prefix, pic_suffix),
+            "link-arg=-Wl,--no-whole-archive",
+        ]
     else:
         want = [
             "-lstatic=native_dep{}".format(pic_suffix),

@Repana-Chowdappa Repana-Chowdappa closed this by deleting the head repository Apr 15, 2024
@Repana-Chowdappa
Copy link
Author

@UebelAndre - Added code for buildifier and CI fixes for s390x and created #2616

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

4 participants