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.

Sign up for GitHub

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

Jump to bottom

[SLikeNet] Update and fix building with x64-mingw-static #18358

Merged
merged 4 commits into from Jul 23, 2021
Merged

[SLikeNet] Update and fix building with x64-mingw-static #18358

merged 4 commits into from Jul 23, 2021

Conversation

Pospelove
Copy link
Contributor

This PR makes slikenet port buildable with the x64-mingw-static triplet. However there are some problems with merging it as I can see:

  1. The patch introduces tons of changes. Obviously, this violates the maintainer guide.
    The reason for patching code this way is pretty simple. As far as I know, MinGW doesn't have some essential functions support like inet_pton/inet_ntop. Building SLikeNet requires them.
  2. TwoWayAuthentication.cpp breaks compilation. Investigating. For now, I remove this file via portfile.cmake.

I would really appreciate your feedback, especially options on how to workaround missing functions. My current idea is that we should add something like vcpkg-platform-helper as a port that would implement functions missing on some platforms.

@Pospelove Pospelove changed the title [SLikeNet] Fix building with x64-mingw-static [SLikeNet] Update and fix building with x64-mingw-static Jun 10, 2021
@NancyLi1013 NancyLi1013 added category:port-update The issue is with a library, which is requesting update new revision category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. labels Jun 10, 2021
@NancyLi1013
Copy link
Contributor

Please ping me if this PR is read for review @Pospelove.

@Pospelove Pospelove marked this pull request as ready for review June 12, 2021 03:37
@Pospelove
Copy link
Contributor Author

This PR makes slikenet port buildable with the x64-mingw-static triplet. However there are some problems with merging it as I can see:

  1. The patch introduces tons of changes. Obviously, this violates the maintainer guide.
    The reason for patching code this way is pretty simple. As far as I know, MinGW doesn't have some essential functions support like inet_pton/inet_ntop. Building SLikeNet requires them.
  2. TwoWayAuthentication.cpp breaks compilation. Investigating. For now, I remove this file via portfile.cmake.

I would really appreciate your feedback, especially options on how to workaround missing functions. My current idea is that we should add something like vcpkg-platform-helper as a port that would implement functions missing on some platforms.

  1. I have read the guide again and now I think that there is no violation. I do not implement anything new, just fix compatibility. Patched license file since the third-party implementation is being used.
  2. Figured out that it's just an optional plugin. So, disabling it via compile definition when building under MinGW.

@Pospelove
Copy link
Contributor Author

@NancyLi1013 I think it's ready

NancyLi1013
NancyLi1013 suggested changes Jun 15, 2021
View reviewed changes
Copy link
Contributor

@NancyLi1013 NancyLi1013 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems you rewrite portfile.cmake instead of only updating the changes based on the original file. Can you update this?

ports/slikenet/portfile.cmake Outdated Show resolved Hide resolved
ports/slikenet/portfile.cmake Outdated Show resolved Hide resolved
@Pospelove Pospelove mentioned this pull request Jun 16, 2021
Open
@Pospelove @NancyLi1013
SLikeNet now builds under MinGW
35dbfe0 
Use vcpkg.json instead of CONTROL


Fix


Run x-add-version


Fix deprecation warning


Update ports/slikenet/portfile.cmake

Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
NancyLi1013
NancyLi1013 suggested changes Jun 16, 2021
View reviewed changes
ports/slikenet/vcpkg.json Outdated Show resolved Hide resolved
@Pospelove @NancyLi1013
Update ports/slikenet/vcpkg.json
76021fd 
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
@NancyLi1013
Copy link
Contributor

Please vcpkg x-add-version slikenet --overwrite-version to solve the failures on x86-windows.

@NancyLi1013
Copy link
Contributor

LGTM, thanks for your updates and fixes. @Pospelove

@NancyLi1013 NancyLi1013 added the info:reviewed Pull Request changes follow basic guidelines label Jun 17, 2021
@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vicroms vicroms merged commit 0517776 into microsoft:master Jul 23, 2021
@Pospelove Pospelove deleted the slikenet-mingw branch July 23, 2021 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NancyLi1013 NancyLi1013 NancyLi1013 approved these changes

Assignees

@NancyLi1013 NancyLi1013

Labels
category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Pospelove @NancyLi1013 @vicroms @BillyONeal