Skip to content

Conversation

gemmaro
Copy link
Contributor

@gemmaro gemmaro commented Jul 11, 2025

Hello,

This adds detection for optional zlib support that may be available indirectly through libxml2, to extconf.rb.

Background:
While this gem doesn't directly depend on zlib, it can leverage zlib functionality when available through libxml2. Currently, the HAVE_ZLIB_H macro is never defined, which means zlib support is always disabled even when zlib is actually available via libxml2. This change allows the extension to conditionally use zlib features.

Thank you.

This defines HAVE_ZLIB_H macro when zlib is available through libxml2
dependency.
@cfis
Copy link
Member

cfis commented Jul 12, 2025

Thanks!

@cfis cfis merged commit e50f31a into xml4r:master Jul 12, 2025
@gemmaro gemmaro deleted the ext/zlib-header branch July 12, 2025 05:19
@cfis
Copy link
Member

cfis commented Jul 13, 2025

I should have paid more attention, looks like this commit broke windows. Can you fix?

See e50f31a

@gemmaro
Copy link
Contributor Author

gemmaro commented Jul 13, 2025

Thanks for bringing this to my attention. I suspected the Windows CI might have been broken before this change, so I did some testing to verify.

I created a test branch and pull request to isolate the issue:

The CI fails the same way, confirming this is an existing Windows CI issue unrelated to the zlib detection changes. This is probably an issue that emerged over time due to ecosystem changes - it likely worked fine in the past.

I have access to a Windows environment and am trying to investigate and fix this, but I'm struggling to set up the compilation environment for this gem. Since it might take some time to resolve, it would probably be better to create a separate issue for this.

@cfis
Copy link
Member

cfis commented Jul 13, 2025

Actually it looks like this - aed5b20

I have a windows build environment setup already, so can take a look.

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