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 and make zip -d step more resilient #10798

Closed
wants to merge 2 commits into from

Conversation

jonpspri
Copy link
Contributor

Continuation of PR #10643 for application to third_party directory

Build on s390x fails because the process to remove unnecessary libraries from netty was not configured for x86. As a result, zip -d failed because it had nothing to remove. This PR does two things:

Adds conditions for s390x to the src and third_party BUILD files, and
Adds a condition to the genrule so that 'zip -d' does not execute if there's nothing to do.

See also Issue #9263; Pull Requests #9346 and #9945

Copy link
Member

@philwo philwo left a comment

Choose a reason for hiding this comment

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

Looks good, except for one comment.

Comment on lines 472 to 473
"//src/conditions:freebsd": "*.so *.jnilib *.dll",
"//src/conditions:openbsd": "*.so *.jnilib *.dll",
Copy link
Member

Choose a reason for hiding this comment

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

Why are you removing these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Um... because despite all my efforts not to I managed to overlook something when I ran the push? I’ll un-remove them now...

Copy link
Member

Choose a reason for hiding this comment

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

No worries, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@philwo Patched up.

@bazel-io bazel-io closed this in ab62a6e Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants