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

Do not check pkg-config when cross-compiling for android #199

Merged
merged 1 commit into from
Jul 27, 2024

Conversation

sagudev
Copy link
Contributor

@sagudev sagudev commented Jul 26, 2024

Currently when compiling for android/haiku target we check pkg-config
for zlib then fallback to using shipped zlib,
this can cause problems as pkg-config sometimes returns host's zlib.

This patch makes it to always use shipped zlib instead of pkg-config one (moves target.contains("android") check before pkg-config check).

@sagudev
Copy link
Contributor Author

sagudev commented Jul 26, 2024

CI has some network issues

Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Could you provide a motivation for this change either as commit message, as in-code comment, or a combination of both?

Thank you!

@Byron
Copy link
Member

Byron commented Jul 26, 2024

Could you spell out everything in the commit message, like I am 3 years old?
The current explanation doesn’t tell me much. Imagine someone who has no background in this.
Thanks again

Currently when compiling for android/haiku target we check pkg-config
for zlib then fallback to using shipped zlib,
this can cause problems as pkg-config sometimes returns host's zlib.

This patch makes it to always use shipped zlib instead of pkg-config one.

Signed-off-by: sagudev <[email protected]>
@sagudev
Copy link
Contributor Author

sagudev commented Jul 26, 2024

In case commit message is still unclear, you can also edit it yourself (I always Allow edits by maintainers).

@Byron
Copy link
Member

Byron commented Jul 26, 2024

I really meant the 'please explain it like I am 3' because I wouldn't know how to improve it - it's entirely unclear to me, I have no context.

@sagudev
Copy link
Contributor Author

sagudev commented Jul 26, 2024

But it is ok now? (There was a force push between #199 (comment) and #199 (comment))

Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot, I understand what's going on now and can confirm that the change makes sense to accomplish this goal (without having tested or otherwise validated it).

@Byron Byron merged commit f76b7a2 into rust-lang:main Jul 27, 2024
45 checks passed
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