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

OpenBSD neither provides nor requires libdl and librt. #4017

Merged
merged 1 commit into from Jul 18, 2016
Merged

OpenBSD neither provides nor requires libdl and librt. #4017

merged 1 commit into from Jul 18, 2016

Conversation

bentley
Copy link
Contributor

@bentley bentley commented Jul 17, 2016

Fixes linking errors on OpenBSD.


This change is Reviewable

@Fallcrest Fallcrest mentioned this pull request Jul 17, 2016
@Parlane
Copy link
Member

Parlane commented Jul 17, 2016

@dolphin-emu-bot rebuild

@endrift
Copy link
Contributor

endrift commented Jul 17, 2016

Should maybe make these match all BSD flavors, not just special casing OpenBSD and FreeBSD, since I highly doubt e.g. NetBSD or PC-BSD will be any different.

@Parlane
Copy link
Member

Parlane commented Jul 17, 2016

@endrift yea that's the only reason I didn't merge. Could the matches just be on "BSD" ?

@bentley
Copy link
Contributor Author

bentley commented Jul 17, 2016

Matching on “BSD” is not a good strategy in general, as there’s a lot of divergence. Notice how FreeBSD requires librt but not libdl. #4018 shows some other examples of divergence between OpenBSD and FreeBSD: the sysctl interface differs, and OpenBSD doesn't support newlocale().

A better way would be to default to POSIX and only special‐case POSIX‐violating systems. So the librt hunk would stay (because POSIX requires librt, and OpenBSD/OS X violate that), and the libdl hunks would be changed to match only Linux (because POSIX does not require libdl).

@endrift
Copy link
Contributor

endrift commented Jul 17, 2016

Yeah, that's more or less the way I was thinking, although I can't speak for other people here since they care more about Linux than any other *NIX system.

@bentley
Copy link
Contributor Author

bentley commented Jul 18, 2016

Okay, here’s a new version per the previous comment.

@Parlane
Copy link
Member

Parlane commented Jul 18, 2016

@dolphin-emu-bot rebuild

@endrift
Copy link
Contributor

endrift commented Jul 18, 2016

Reviewed 1 of 3 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@Parlane Parlane merged commit 4b9173c into dolphin-emu:master Jul 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants