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

Add s390x arch support #2616

Merged
merged 10 commits into from
Sep 6, 2024
Merged

Add s390x arch support #2616

merged 10 commits into from
Sep 6, 2024

Conversation

Repana-Chowdappa
Copy link
Contributor

This is continuing of #2557 , added buildifier code changes for s390x arch also.

@Repana-Chowdappa
Copy link
Contributor Author

@UebelAndre - now we have buildifier binaries available for s390x arch from buildtools 7.3.1 https://github.com/bazelbuild/buildtools/releases/download/v7.3.1/buildifier-linux-s390x . Can we please update the same on

_BUILDIFIER_VERSION = "7.1.1"
?

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

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

Looks like you just need to run buildifier one more time and the change is good to go!

@Repana-Chowdappa
Copy link
Contributor Author

Repana-Chowdappa commented Aug 29, 2024

@UebelAndre - I don't have access to retry or rerun buildifier on CI, please provide me the access to the same.

@UebelAndre
Copy link
Collaborator

@UebelAndre - I don't have access to retry or rerun buildifier on CI, please provide me the access to the same.

You would just run buildifier on whatever machine you can and push the results to your branch.

@Repana-Chowdappa
Copy link
Contributor Author

@UebelAndre - Can we upgrade the buildifier to 7.3.1? where we have s390x binary is available.

@UebelAndre
Copy link
Collaborator

@UebelAndre - Can we upgrade the buildifier to 7.3.1? where we have s390x binary is available.

Yeah, go ahead and bump the version 😄

@Repana-Chowdappa
Copy link
Contributor Author

@UebelAndre - I have fixed the buildifier CI failures, Can you please merge this PR, we need this for a customer deployment.

@Repana-Chowdappa
Copy link
Contributor Author

@UebelAndre - All CI checks passed. Can you please help us on merging this PR.

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me!

@UebelAndre UebelAndre added this pull request to the merge queue Sep 6, 2024
Merged via the queue into bazelbuild:main with commit 3d1856b Sep 6, 2024
4 checks passed
@@ -48,6 +48,9 @@ rust_library(
"@rules_rust//rust/platform:i686-unknown-linux-gnu": [
"errno", # i686-unknown-linux-gnu
],
"@rules_rust//rust/platform:s390x-unknown-linux-gnu": [
Copy link
Collaborator

@UebelAndre UebelAndre Sep 6, 2024

Choose a reason for hiding this comment

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

@Repana-Chowdappa I noticed on #2841 that this change goes away when I regenerate dependencies. How was this produced?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@UebelAndre - any specific reason to remove this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

These files are generated and if there is a change made to them they should be made in a way that regenerates into the file. The removal feels more like a miss where something was not hooked up correctly and I was wondering how these came to be in the first place.

@Repana-Chowdappa
Copy link
Contributor Author

@UebelAndre - when will be the next release for rules_rust? so that we will have rust available for s390x platform.

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