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

seperat link command for Winsock #333

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

Davknapp
Copy link

Title Use seperate link command for WINSOCK_LIBRARY

Following up on issue cburstedde/libsc#206

Proposed changes: Use a seperate command instead of a generator expression. It prevents linker-errors with p4est if these expression fails to be properly generated.

@cburstedde
Copy link
Owner

Thanks again! I understand it's not originating from your code, but I'm wondering about an asymmetry with libsc: There we check for WIN32, here for HAVE_WINSOCK2_H. I would imagine this can be unified, with the goal to reduce the diff between the cmake environments of the two libraries as much as we possibly can.

@lukasdreyer
Copy link

@cburstedde Are you willing to merge this without the change? We have some users running into problems with it. I can change HAVE_WINSOCK2_H into WIN32 here, but I do not have any ideas of the technical implications.

@hannesbrandt
Copy link
Collaborator

I have been working on unifying the Winsock logic in p4est and libsc. It's still WIP, but most likely HAVE_WINSOCK2_H will stay in p4est as is and only libsc will be changed. So, from my perspective it would be fine to merge this PR and unify the Winsock logic with a separate PR in libsc later on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants