Skip to content

[build] Refactor zlib configuration #2625

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

Merged
merged 1 commit into from
Aug 28, 2024
Merged

[build] Refactor zlib configuration #2625

merged 1 commit into from
Aug 28, 2024

Conversation

fhanau
Copy link
Contributor

@fhanau fhanau commented Aug 28, 2024

  • Drop visibility attribute, only zlib library itself needs to be visible
  • Drop explicit includes directive, this is generally bad style in bazel
  • Drop support for unsupported architectures, add target_compatible_with for arch-specific targets

Will make another PR that switches to using local_defines in the afternoon.

@fhanau fhanau requested a review from npaun August 28, 2024 17:45
@fhanau fhanau requested review from a team as code owners August 28, 2024 17:45
@fhanau fhanau requested a review from erikcorry August 28, 2024 17:45
Copy link
Member

@anonrig anonrig left a comment

Choose a reason for hiding this comment

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

Would this reduce build time?

@npaun
Copy link
Member

npaun commented Aug 28, 2024

Seems like something got bungled with visibility?

@fhanau
Copy link
Contributor Author

fhanau commented Aug 28, 2024

It's including system zlib headers instead of the headers here when dropping the includes = ["."] which causes linker issues – unexpected, but easy to work around.

@fhanau fhanau force-pushed the felix/gn-zlib-cleanup branch from 16f722e to 49c53fc Compare August 28, 2024 20:13
- Drop visibility attribute, only zlib library itself needs to be visible
- Drop support for unsupported architectures, add target_compatible_with for
  arch-specific targets
- Fold up x86_defines
@fhanau fhanau force-pushed the felix/gn-zlib-cleanup branch from 49c53fc to eb520b1 Compare August 28, 2024 20:42
@fhanau fhanau merged commit 26d8f3b into main Aug 28, 2024
13 checks passed
@fhanau fhanau deleted the felix/gn-zlib-cleanup branch August 28, 2024 21:19
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.

3 participants