-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[eventbus] Add new port #41563
base: master
Are you sure you want to change the base?
[eventbus] Add new port #41563
Conversation
/azp run |
Rerunning due to network mistake |
Azure Pipelines successfully started running 1 pipeline(s). |
@JonLiu1993 Now that all the test have passed, is the port ready for review? and/or approval? |
Tested usage successfully by
|
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.
The portfile fetches the wrong revision for the claimed version.
# before this revision the build would fail on windows | ||
vcpkg_from_github( | ||
OUT_SOURCE_PATH SOURCE_PATH | ||
REPO gelldur/EventBus | ||
REF 4689564c4c775456bfa0dfd976b4f48aca5f4d2a | ||
SHA512 0f1f3c21d1c5a18da87e331f252cb464143ba2038a26e6edb6b11c9544c02dd1919fe728e803b382e8f6b89550582d7905170437c184f784cfa3f28c784a7e59 |
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.
# before this revision the build would fail on windows | |
vcpkg_from_github( | |
OUT_SOURCE_PATH SOURCE_PATH | |
REPO gelldur/EventBus | |
REF 4689564c4c775456bfa0dfd976b4f48aca5f4d2a | |
SHA512 0f1f3c21d1c5a18da87e331f252cb464143ba2038a26e6edb6b11c9544c02dd1919fe728e803b382e8f6b89550582d7905170437c184f784cfa3f28c784a7e59 | |
vcpkg_from_github( | |
OUT_SOURCE_PATH SOURCE_PATH | |
REPO gelldur/EventBus | |
REF v${VERSION} | |
SHA512 0 # please insert SHA512 |
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.
@dg0yt the latest version would fail on windows there were some fixes added to address that in the commit referenced REF, and according to the maintainers guide, if a patch is in the upstream I should favor using it than patching the code myself, What should I do in this case?
To summarize, using v${VERSION} will fail
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.
4689564c4c775456bfa0dfd976b4f48aca5f4d2a is older than, and included, in 3.1.2:
gelldur/EventBus@4689564
So this is just totally wrong. If the build fails with 3.1.2, check the actual error.
Note: I will be converting your PR to draft status. When you respond, please revert to "ready for review". That way, I can be aware that you've responded since you can't modify the tags. |
apply suggested changes Co-authored-by: Kai Pastor <[email protected]>
Pinging @motazmuhammad for response. Is work still being done for this PR? |
Fixes##41549
find_package
calls are REQUIRED, are satisfied byvcpkg.json
's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx.vcpkg.json
matches what upstream says.vcpkg.json
matches what upstream says../vcpkg x-add-version --all
and committing the result.