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

Fix compilation on DragonFlyBSD #2028

Merged
merged 3 commits into from Jan 16, 2018

Conversation

Projects
None yet
4 participants
@dkgroot
Contributor

dkgroot commented Jan 10, 2018

  • Also do a little formatting cleanup
  • Add static assert to prevent compilation when IP_ADD_MEMBERSHIP, IP_MULTICAST_LOOP are missing (not sure if this is preferred or not)

dkgroot added some commits Jan 9, 2018

Remove static compiler __VERSION check. 'import core.sys.dragonflybsd…
….netinet.in_' is fine, when compiler support is there.
@@ -77,6 +77,13 @@ version(Windows) {
}
else
import core.sys.freebsd.netinet.in_ : IP_ADD_MEMBERSHIP, IP_MULTICAST_LOOP;
} else version (DragonFlyBSD) {
static if (__VERSION__ < 2077) {

This comment has been minimized.

@Geod24

Geod24 Jan 11, 2018

Contributor

Shouldn't this be 2079 ? Since it's not merged yet, and 2078 was already released.

This comment has been minimized.

@dkgroot

dkgroot Jan 11, 2018

Contributor

You raising this question, made me think about this again. As this source is only going to be compiled/used when the changes have already landed in the compiler + druntime. It does make sense to even check the compiler version. I will remove this static if and just import core.sys.dragonflybsd.netinet.in_. There really is not need for the fall back.

@thaven

thaven suggested changes Jan 11, 2018 edited

Slightly offtopic: in fact it is a shame we need all this. druntime really should have a way to get such common constants in a more OS independent way. (e.g. by providing those somewhere as aliases to their OS specific definitions)

}
else version (DragonFlyBSD)
{
static if (__VERSION__ < 2077) {

This comment has been minimized.

@thaven

thaven Jan 11, 2018

Contributor

Somehow the comment of @Geod24 is shown as outdated, but he is right, this should be __VERSION__ < 2079 (or even later, if core.sys.dragonflybsd doesn't get merged into druntime master before 2.079)

This comment has been minimized.

@dkgroot

dkgroot Jan 11, 2018

Contributor

See
No need to check the compiler version. It's like a chicken/egg issue, without compiler support there is not way to get to this code. When compiler support lands, then core.sys.dragonflybsd.netinet.in_ will exist.

@dkgroot

This comment has been minimized.

Contributor

dkgroot commented Jan 11, 2018

@thaven I fully agree. It would make more sense and be a lot nicer if there was an generic interface that could be referred to.

dkgroot added a commit to dkgroot-dlang/eventcore that referenced this pull request Jan 11, 2018

Remove static compiler __VERSION check. "import core.sys.dragonflybsd…
….netinet.in_" will work, when compiler support is there.

[See](vibe-d/vibe.d#2028 (comment))
@s-ludwig

This comment has been minimized.

Member

s-ludwig commented Jan 16, 2018

Usually those constants actually are in druntime, but unfortunately they are just being added on a per-case basis instead of thoroughly going through each C header. I used to make druntime PR's for such missing constants, but lately I've been getting too busy and had to focus on just getting vibe.d to work.

@s-ludwig s-ludwig merged commit 88f3ec7 into vibe-d:master Jan 16, 2018

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@dkgroot

This comment has been minimized.

Contributor

dkgroot commented Jan 16, 2018

Ok understood. Thanks for merging this PR !

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