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

Move conf-pkg-config dependency to ctypes-foreign #631

Merged
merged 1 commit into from
Mar 17, 2020

Conversation

aantron
Copy link
Contributor

@aantron aantron commented Mar 15, 2020

I tested this by installing Luv, which depends on ctypes, on a system without a pkg-config binary. conf-pkg-config did not get installed.

I saw that pkg-config is also used for something with Xen. I don't know how to best express that as a constraint. However, the Xen-related detection will still work if pkg-config happens to be installed, regardless of conf-pkg-config.

Fixes #630.

@yallop
Copy link
Owner

yallop commented Mar 17, 2020

I saw that pkg-config is also used for something with Xen. I don't know how to best express that as a constraint. However, the Xen-related detection will still work if pkg-config happens to be installed, regardless of conf-pkg-config.

@talex5, do you have a view on this? I think the Xen code here was your work (#231), but I'm not sure of the current status.

@talex5
Copy link
Contributor

talex5 commented Mar 17, 2020

mirage-xen depends on pkg-config itself, so it would continue to work. Also, this will all get replaced by dune at some point -- looks like #588 moves it to ctypes.foreign already. So, it think this change is OK.

@yallop
Copy link
Owner

yallop commented Mar 17, 2020

Thank you, @talex5. I'll merge this, then.

@aantron: thank you for the contribution. You're also very welcome to make a corresponding change to the ctypes/ctypes-foreign meta-data in opam-repository if you'd like to avoid the need to pin ctypes before the next release.

@yallop yallop merged commit ca4ba1f into yallop:master Mar 17, 2020
@aantron
Copy link
Contributor Author

aantron commented Mar 17, 2020

Thanks! I'll keep the possibility of making the change to existing releases in mind if anyone reports an issue with having to install pkg-config "in the wild." For now, it's just something I noticed in my own testing.

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.

conf-pkg-config is required for ctypes.foreign, not ctypes (base package)
3 participants