Skip to content
This repository has been archived by the owner on Oct 2, 2023. It is now read-only.

Add s390x support to load puller and loader binary #1661

Merged

Conversation

srajmane
Copy link
Contributor

No description provided.

@srajmane
Copy link
Contributor Author

@alex1545 @smukherj1 Could you please have a look ?

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?

@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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants