[embree3] Expose project options, plus add NEON ISAs as features
If this PR updates an existing port, please uncomment and fill out this checklist:
- [x] Changes comply with the maintainer guide
- [x] SHA512s are updated for each updated download
- [x] The "supports" clause reflects platforms that may be fixed by this new version
- [x] Any fixed CI baseline entries are removed from that file.
- [x] Any patches that are no longer applied are deleted from the port's directory.
- [x] The version database is fixed by rerunning
./vcpkg x-add-version --alland committing the result. - [x] Only one version is added to each modified port's versions file.
Looking at existing PR/issue there may be some redundancy with #30548 Something to double check before removing the draft status
Also this PR follows the suggestion to avoid listing explicitly which ISA to target which may be a good thing
All feature tested on the following triplets:
- x64-windows
- x64-windows-static
- x86-windows
Tagged vcpkg-team-review to answer this question:
Today, we would refuse to merge features that try to change ISA like this, under the 'do not implement alternatives as features' rule. However, embree3 already has other ISA features, should we make an exception in that this adds one like the others? See also, last time the maintainers weighed in on this topic: https://github.com/microsoft/vcpkg/pull/29093#issuecomment-1465969155
Alternatively, should we remove all ISA features entirely, because they are broken?
I'll vote for removal. Stuff like this can be set via VCPKG_CMAKE_COINFIGURE_OPTIONS or the port needs to inspect compiler flags to enable those.
@BillyONeal @Neumann-A great discussion ! I took the liberty to implement an embree3 port where we get rid of ISA features and handle it as best we can in the portfile.
I gave it a quick test on an internal project which target both native and emscripten platforms. For Emscripten, I had to hijack the ISA selection because it was incorrect otherwise letting Embree select the default one by itself seems to work fine.
Let me know what you think of 6d4e95e8bc5e0223ecb1501e39eddca4ceb990c0
Hey @BillyONeal @Neumann-A, did you have the time to look at my latest revision which should take into account to your feedbacks ?
Gentle reminder, @BillyONeal @Neumann-A, did you have the time to look at my latest revision which should take into account to your feedbacks ?
The title of the PR should be changed to reflect the new changes.
Indeed, I'll find something more appropriate before the end of the week.