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

Prebuilt binaries for s390x #1230

Merged
merged 10 commits into from Jan 24, 2020
Merged

Conversation

@shahidhs-ibm
Copy link
Contributor

@shahidhs-ibm shahidhs-ibm commented Jan 7, 2020

As per discussion in issue #296, creating this PR to add s390x cross compilation support. From this we also would like to provide tier3 support for the s390x architecture.

@linux-foundation-easycla
Copy link

@linux-foundation-easycla linux-foundation-easycla bot commented Jan 7, 2020

CLA Check
One or more committers are not authorized under a signed CLA as indicated below. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

@thelinuxfoundation
Copy link

@thelinuxfoundation thelinuxfoundation commented Jan 7, 2020

Thank you for your pull request. Before we can look at your contribution, we need to ensure all contributors are covered by a Contributor License Agreement.

After the following items are addressed, please respond with a new comment here, and the automated system will re-verify.

Regards,
CLA GitHub bot

$tool_dir/build_artifact_node_arm.sh
docker run -e JOBS=8 -e ARTIFACTS_OUT=/var/grpc/artifacts -v $base_dir:/var/grpc alpine_node_artifact /var/grpc/tools/run_tests/artifacts/build_artifact_node.sh --with-alpine
Copy link
Member

@murgatroid99 murgatroid99 Jan 7, 2020

This line appears to be redundant with line 80.

@@ -69,8 +69,13 @@ if [ "$DO_NATIVE" = "true" ] ; then
fi

if [ "$DO_CROSS" = "true" ] ; then
docker build -t alpine_node_artifact $base_dir/tools/docker/alpine_artifact
Copy link
Member

@murgatroid99 murgatroid99 Jan 7, 2020

Why move this line up here?

Copy link
Contributor Author

@shahidhs-ibm shahidhs-ibm left a comment

Thanks @murgatroid99 for the code review. Incorporated the review comments. Please check.

@murgatroid99
Copy link
Member

@murgatroid99 murgatroid99 commented Jan 8, 2020

From the build failure log it looks like build_artifact_node_s390x.sh is not executable. Please fix that.

@shahidhs-ibm
Copy link
Contributor Author

@shahidhs-ibm shahidhs-ibm commented Jan 9, 2020

@murgatroid99 Updated the build_artifact_node_s390x.sh file mode to make it executable. The build should pass now.
I am facing issue while signing CNCF CLA. When I select 'IBM' as company to sign the corporate CLA, the page don't move forward and gets stuck into the loading animation. Any idea how to resolve this?

@srini100
Copy link
Contributor

@srini100 srini100 commented Jan 9, 2020

@shahidhs-ibm, does it get stuck even if you select a different company?

@shahidhs-ibm
Copy link
Contributor Author

@shahidhs-ibm shahidhs-ibm commented Jan 13, 2020

@srini100 Yes, it gets stuck when I select different company. I tried clearing my browser cache and changing different browsers but the issue persists.

@WillsonHG
Copy link

@WillsonHG WillsonHG commented Jan 13, 2020

@shahidhs-ibm Can you please try the EasyCLA authorization again? We think we have resolved the issue when selecting your company. Thanks.

@shahidhs-ibm
Copy link
Contributor Author

@shahidhs-ibm shahidhs-ibm commented Jan 16, 2020

@WillsonHG Thank you. The issue in selecting company name is resolved and now my CLA check with EasyCLA is passing. However 'cla/linuxfoundation' check is still failing. I am checking with my company's CLA manager if he/she needs to correct/update something at their end.
Please let me know if you have any idea on how to fix this.

@WillsonHG
Copy link

@WillsonHG WillsonHG commented Jan 16, 2020

@shahidhs-ibm Glad that was resolved.

I'm not sure why the older cla/linuxfoundation is still running on this repo. That is a question for the repo maintainer. But if you want to also pass that check, your CLA manager will need to add your email to the whitelist at identity.linuxfoundation.org

The other thing that may be an issue is the user "root". Neither EasyCLA nor the cla/linuxfoundation is recognizing that user or its commit. Would you be able to author that commit instead?

@shahidhs-ibm
Copy link
Contributor Author

@shahidhs-ibm shahidhs-ibm commented Jan 17, 2020

@WillsonHG Thank you for detailed response. Do you think that 'root' user commit is causing CLA issue? That commit was a mistake and I reverted it with new commit, however that entry remained and I can't author it. Shall I create a new PR? Any suggestion?

@murgatroid99
Copy link
Member

@murgatroid99 murgatroid99 commented Jan 17, 2020

We have a green check mark on the EasyCLA check. I think the CLA is fine.

@shahidhs-ibm
Copy link
Contributor Author

@shahidhs-ibm shahidhs-ibm commented Jan 20, 2020

@murgatroid99 @WillsonHG Are we good on CLA front then? If yes then could you please review and merge? Please let me know if you have any comments / suggestions.

@murgatroid99
Copy link
Member

@murgatroid99 murgatroid99 commented Jan 21, 2020

I think this looks OK, but I want to make sure we're on the same page about maintenance. We, the gRPC team, cannot effectively maintain this build. As long as the build continues to work we will publish the resulting binaries, but if it breaks, you will need to fix that, and I do not want to block releases for long on those breakages. Will you be able to do that?

@shahidhs-ibm
Copy link
Contributor Author

@shahidhs-ibm shahidhs-ibm commented Jan 24, 2020

@murgatroid99 We are OK to provide our commitment on the maintenance of s390x binaries. As per comment in issue, will it help if we provide s390x VMs?

Copy link
Member

@murgatroid99 murgatroid99 left a comment

I don't know if we can make use of those s390x VMs. We can discuss that later. For now, this change is acceptable. Thank you for your contribution.

@murgatroid99 murgatroid99 merged commit 4421ff6 into grpc:master Jan 24, 2020
17 of 20 checks passed
@shahidhs-ibm
Copy link
Contributor Author

@shahidhs-ibm shahidhs-ibm commented Jan 29, 2020

@murgatroid99 Thank you for merging the PR. Can we expect Pre-built binaries for s390x be available in next grpc release? Is there any way by which we can download s390x binaries for testing?

@murgatroid99
Copy link
Member

@murgatroid99 murgatroid99 commented Jan 29, 2020

We have recently been having some build problems that we are actively working on fixing. Once we get the build working, the next release will have whatever binaries come out of the build script, which should now include the s390x binaries.

@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants