-
Notifications
You must be signed in to change notification settings - Fork 7.3k
[xdigest] new port #49708
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
base: master
Are you sure you want to change the base?
[xdigest] new port #49708
Conversation
ba40c18 to
2a29077
Compare
|
oopsie.. |
b5836c0 to
f8b1807
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please complete the new port checklist:
- Changes comply with the maintainer guide.
- The name of the port matches an existing name for this component on https://repology.org/ if possible, and/or is strongly associated with that component on search engines.
- Optional dependencies are resolved in exactly one way. For example, if the component is built with CMake, all
find_packagecalls are REQUIRED, are satisfied byvcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx. - The versioning scheme in
vcpkg.jsonmatches what upstream says. - The license declaration in
vcpkg.jsonmatches what upstream says. - The installed as the "copyright" file matches what upstream says.
- The source code of the component installed comes from an authoritative source.
- The generated "usage text" is accurate. See adding-usage for context.
- The version database is fixed by rerunning
./vcpkg x-add-version --alland committing the result. - Only one version is in the new port's versions file.
- Only one version is added to each modified port's versions file.
| "asm" | ||
| ], | ||
| "features": { | ||
| "asm": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like this shouldn't be a feature; if anything it would be something someone may want to force off, and that's not what features are for. (In particular, in vcpkg's model it is always assumed that turning on a feature is 'safe')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, according to vcpkg's model, this should not be in the port at all? And it should decide whether to build assemblies or not on its own? If feature in vcpkg suppose to enable some specific optional modules let's say, that makes sense.
Also, in this case the feature should actually be safe to use as it's the core funtionality of the library.
|
Can you describe how this library avoids taking symbols defacto owned by OpenSSL? Things like https://github.com/rinrab/xdigest/blob/0a0c673c5d982733bb7198bb89d1c65b3ab254d6/xdigest/include/internal/common.h#L48 do not fill one with confidence that this can be installed without introducing multiple definition errors if OpenSSL is also installed. |
This library was developed by me in recent days and I wish it was added to the vcpkg repository. For the rest title and portfile are self explanatory.
Project's homepage can be found here on github: https://github.com/rinrab/xdigest
find_packagecalls are REQUIRED, are satisfied byvcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx.vcpkg.jsonmatches what upstream says.vcpkg.jsonmatches what upstream says../vcpkg x-add-version --alland committing the result.