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.

Sign up for GitHub

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

Jump to bottom

Add s390x support to load puller and loader binary #1661

Merged
merged 1 commit into from
Nov 4, 2020
Merged

Add s390x support to load puller and loader binary #1661

merged 1 commit into from
Nov 4, 2020

Conversation

srajmane
Copy link
Contributor

No description provided.

@google-cla google-cla bot added the cla: yes label Oct 23, 2020
@srajmane
Copy link
Contributor Author

@alex1545 @smukherj1 Could you please have a look ?

smukherj1
smukherj1 reviewed Oct 29, 2020
View reviewed changes
container/load.bzl Outdated
if repository_ctx.os.name.lower().startswith("mac os"):
loader = repository_ctx.attr._loader_darwin
arch = repository_ctx.execute(["uname", "-m"]).stdout.strip()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately this will most likely break on Windows. Also given we don't have a Windows CI, it'll be hard to validate either way.

I think I want to stay away from having to maintain platform specific ways of detecting the architecture. Instead could you add a new attribute that defaults to amd64 but could be explicitly specified to be s390x?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smukherj1 Please review updated load.bzl and pull.bzl files.

@smukherj1
Copy link
Collaborator

/gcbrun

@smukherj1
Copy link
Collaborator

Actually buildkite CI is failing. Could you fix the formatting errors reported by buildifier?

@k8s-ci-robot k8s-ci-robot removed the lgtm label Nov 4, 2020
@srajmane
Copy link
Contributor Author

srajmane commented Nov 4, 2020

Actually buildkite CI is failing. Could you fix the formatting errors reported by buildifier?

@smukherj1 Fixed formatting errors.

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: smukherj1, srajmane

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@smukherj1
Copy link
Collaborator

/gcbrun

@rbe-toolchains-pr-bot rbe-toolchains-pr-bot merged commit b4617fa into bazelbuild:master Nov 4, 2020
@tcnghia tcnghia mentioned this pull request Nov 5, 2020
Merged
tcnghia pushed a commit to tcnghia/rules_docker that referenced this pull request Nov 6, 2020
@srajmane @tcnghia
Added s390x support to load puller and loader binary (bazelbuild#1661)
0dc5350
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smukherj1 smukherj1 smukherj1 approved these changes

@alex1545 alex1545 Awaiting requested review from alex1545

@DaveGay DaveGay Awaiting requested review from DaveGay

@eytankidron eytankidron Awaiting requested review from eytankidron

Assignees

@smukherj1 smukherj1

Labels
approved cla: yes lgtm size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@srajmane @smukherj1 @k8s-ci-robot @rbe-toolchains-pr-bot