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

Add support for vcpkg "features" #7

Closed
crlf0710 opened this issue Aug 15, 2018 · 4 comments
Closed

Add support for vcpkg "features" #7

crlf0710 opened this issue Aug 15, 2018 · 4 comments

Comments

@crlf0710
Copy link

For example, loading "harfbuzz[icu]".

@mcgoo
Copy link
Owner

mcgoo commented Aug 15, 2018

Thanks for the report. Could you clarify what you mean by this?

As it is, the intention is that vcpkg::find_package("harfbuzz").unwrap(); will find harfbuzz and whatever dependencies it is installed with in your vcpkg tree. If the user has installed it with feature icu enabled, icu should get linked also. Assuming vcpkg-rs is working properly, you would just document that harfbuzz with icu is required.

Right now, if the user's vcpkg installation does not have harfbuzz installed, vcpkg-rs emits no metadata and returns an error. I suppose it would be possible to specify to vcpkg-rs a required set of features and return an error if they are not all present. Is that what you are looking for?

@ras0219-msft
Copy link

ras0219-msft commented Aug 15, 2018

To hijack the thread a bit: It might also be interesting to support an "install if missing" mode, which may be more along the lines of what @crlf0710 is looking for.

@crlf0710
Copy link
Author

@mcgoo

I suppose it would be possible to specify to vcpkg-rs a required set of features and return an error if they are not all present.

Yes, this is what is on my mind. And besides this, currently vcpkg-rs doesn't parse the status text correctly when there are features. For example, when i do this:

vcpkg_cli probe harfbuzz

I get:

didn't know how to deal with port :-
{"Architecture": "x64-windows", "Depends": "icu", "Description": "icu support for harfbuzz", "Feature": "icu", "Multi-Arch": "same", "Package": "harfbuzz", "Status": "install ok installed"}
didn't know how to deal with port :-
{"Architecture": "x64-windows", "Description": "Builtin (UCDN) Unicode callbacks support", "Feature": "ucdn", "Multi-Arch": "same", "Package": "harfbuzz", "Status": "install ok installed"}

This is annoying and apparently something is missing from the parsing part.

@mcgoo
Copy link
Owner

mcgoo commented Aug 21, 2018

I just published vcpkg 0.2.6 which correctly handles linking libraries required by optional features. Now vcpkg::find_package("harfbuzz").unwrap(); will correctly link the libs for icu if the icu feature is enabled for harfbuzz.

I'm going to close this issue, feel free to reopen it if this doesn't work for you.

I'd accept a PR relating to additional validation of features so that the build script could display a diagnostic if you require harfbuzz[icu] but only harfbuzz is installed. You would have to detect the condition from build.rs and print a message. (Typically, packages that are using vcpkg have another way of finding the libraries to link to and the use of vcpkg is secondary - for these libraries, if vcpkg does not find the library, it just does nothing, assuming that the other mechanism is going to work out. The failure is then an error in the link phase if the library is not found some other way. There is nowhere to display a message unless you fail the build from build.rs, so there might not be anywhere to print a message that will be all that useful.)

@ras0219-msft The install-if-missing thing is a great idea - definitely something that I am interested in. It would require an opt-in using an environment variable I suppose - it seems rude or at least surprising to update someone's vcpkg installation without intervention.

@mcgoo mcgoo closed this as completed Aug 21, 2018
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

No branches or pull requests

3 participants