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

Update osx workflow #296

Merged
merged 8 commits into from
Apr 5, 2024
Merged

Update osx workflow #296

merged 8 commits into from
Apr 5, 2024

Conversation

jonathangreen
Copy link
Contributor

@jonathangreen jonathangreen commented Mar 21, 2024

Update osx workflow to make sure that the brew installed version of libxml2 is in the path when compiling xmlsec, rather then the osx system supplied one, to avoid the issue noted in #283 where the tests segfault. This is done by setting PKG_CONFIG_PATH.

This gets the workflow for osx in a mostly passing state. Tests pass for Python 3.6 - 3.11. Tests for Python 3.5 are still failing because the type annotations added in #253 are not valid for Python 3.5. So that either needs to be fixed, or Python 3.5 support needs to get dropped for this to be fully passing.

@deronnax
Copy link

out of curiosity: why not use pkg-config for flags?

@jonathangreen
Copy link
Contributor Author

@deronnax it seemed like pkg-config wasn't working for me, I kept getting the osx system libxml2 instead of the one from brew. Happy to try again though, pkg-config seems like a better solution here. Any suggestions of things to try?

@jonathangreen
Copy link
Contributor Author

@deronnax hmm so this seems to be working for me now, just setting PKG_CONFIG_PATH. Not sure what was going on before. Thanks for the suggestion, I pushed a commit to drop the CFLAGS and LDFLAGS.

@jonathangreen
Copy link
Contributor Author

@deronnax a small follow up to this. For me locally on my mac, its not enough to just setting PKG_CONFIG_PATH, I still end up with segfaults because of mismatching libxml2 versions. I actually have to set CFLAGS and LDFLAGS. Thats why I initially had them set in this PR.

This must be something about my local setup though, since it seems in CI its enough to set PKG_CONFIG_PATH.

.github/workflows/macosx.yml Outdated Show resolved Hide resolved
.github/workflows/macosx.yml Outdated Show resolved Hide resolved
@jonathangreen
Copy link
Contributor Author

@nosnilmot thank you for the great suggestions. I have accepted them into the PR.

I had to make a slight change to #296 (comment) as it resulted in the build failing because GH actions does not do shell expansion on the env vars defined in the env key. So I moved your suggested change to the PKG_CONFIG_PATH into the run block, so it gets properly expanded.

@deronnax
Copy link

@jonathangreen thank you for this precious insights. You made me discover that on my mac, I am probably actually linking against the system libxml2 and not homebrew's one, which I thought I did. Do you have tips to check that?

Btw, I see you are not using --no-binary lxml with pip, so you must be getting the lxml wheel and using the libxml2 provided by lxml, no?

@jonathangreen
Copy link
Contributor Author

Btw, I see you are not using --no-binary lxml with pip, so you must be getting the lxml wheel and using the libxml2 provided by lxml, no?

Yes. In this PR I'm using the lxml wheel, and relying on the fact that the version of libxml2 being installed by brew is the same as the version being used in the most recent lxml builds. So as long as the xmlsec build uses the libxml2 from brew instead of the old version provided by osx everything works as intended.

You made me discover that on my mac, I am probably actually linking against the system libxml2 and not homebrew's one, which I thought I did. Do you have tips to check that?

Now that #299 is merged in, if you build the latest it should give you a useful exception if that is the case. A couple things to try if you are having issues getting this module to build and link against the correct version of libxml2:

  • Make sure pip isn't installing a cached previously built wheel
  • If just setting PKG_CONFIG_PATH isn't working, try also setting CFLAGS and LDFLAGS. For some reason this was necessary for me when building locally on my M1 mac.

@mxamin
Copy link
Collaborator

mxamin commented Apr 5, 2024

@jonathangreen The reason cp35 is failing is because of type hints. Type hints like Dict and List are supported from >=3.6. I removed them in a separate PR (#303) so yours should pass too after the merge.

@mxamin mxamin merged commit 42132ff into xmlsec:master Apr 5, 2024
6 of 31 checks passed
@jonathangreen jonathangreen deleted the feature/fix-osx branch April 5, 2024 15:21
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.

4 participants