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

Remove IFF_NOTRAILERS for openbsd #893

Merged
merged 1 commit into from
May 6, 2018
Merged

Remove IFF_NOTRAILERS for openbsd #893

merged 1 commit into from
May 6, 2018

Conversation

kpcyrd
Copy link
Contributor

@kpcyrd kpcyrd commented Apr 28, 2018

nix currently doesn't compile on openbsd because IFF_NOTRAILERS isn't found. This PR fixes the openbsd build again (tested on 6.3). It seems it was changed in this commit:

openbsd/src@beb8b0d

@asomers
Copy link
Member

asomers commented Apr 28, 2018

Did you mean to enable IFF_STATICARP too? If so, can you do that as a separate commit?

@kpcyrd
Copy link
Contributor Author

kpcyrd commented Apr 28, 2018

@asomers it was basically renamed, that's why I've included it in the commit. Should I split it anyway?

@asomers
Copy link
Member

asomers commented Apr 28, 2018

Ahh, well in that case you can leave it in one commit, but you should still mention it. Also, it looks like this change is going to break compiling Nix for older versions of OpenBSD. Can you mention that in the CHANGELOG?

@kpcyrd kpcyrd force-pushed the fix-openbsd branch 2 times, most recently from a70a3b7 to fa310a0 Compare April 29, 2018 23:44
@kpcyrd
Copy link
Contributor Author

kpcyrd commented May 1, 2018

I've changed the commit message and added a note to the changelog.

@asomers
Copy link
Member

asomers commented May 1, 2018

So removing IFF_NOTRAILERS fixes Nix for Openbsd >= 6.3, and adding IFF_STATICARP breaks Nix for OpenBSD < 6.3. Would it be better not to add IFF_STATICARP to ensure that Nix will continue working on older versions of OpenBSD? That seems to me like the less disruptive option. Unfortunately Rust has no equivalent of #ifdef, so I don't see a way to do this automatically.

@asomers
Copy link
Member

asomers commented May 1, 2018

I suppose I should ask what OpenBSD's support policy is. When will 6.2 go EOL?

@kpcyrd
Copy link
Contributor Author

kpcyrd commented May 1, 2018 •
edited
Loading

OpenBSD 6.2 is going EOL when 6.4 is released, so around april 2019. Removing IFF_NOTRAILERS and adding IFF_STATICARP next year might be the sanest way. I've updated my PR.

asomers
asomers requested changes May 2, 2018
View reviewed changes
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

Could you also explain why you're removing this symbol in the commit message?

CHANGELOG.md
@@ -47,6 +47,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
([#833](https://github.com/nix-rust/nix/pull/833))
- Enabled more `ptrace::Request` definitions for uncommon Linux platforms
([#892](https://github.com/nix-rust/nix/pull/892))
- Remove `IFF_NOTRAILERS` on OpenBSD, as it has been removed in OpenBSD 6.3
Copy link
Member

Choose a reason for hiding this comment

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

Needs a link to the PR

@kpcyrd
OpenBSD: remove IFF_NOTRAILERS
c838e64 
IFF_NOTRAILERS has been removed in OpenBSD 6.3

openbsd/src@beb8b0d
@kpcyrd
Copy link
Contributor Author

kpcyrd commented May 2, 2018

I've updated the changelog and the commit message.

@asomers
Copy link
Member

asomers commented May 6, 2018

bors r+

bors bot added a commit that referenced this pull request May 6, 2018
@bors @kpcyrd
Merge #893
2741ecb 
893: Remove IFF_NOTRAILERS for openbsd r=asomers a=kpcyrd

nix currently doesn't compile on openbsd because `IFF_NOTRAILERS` isn't found. This PR fixes the openbsd build again (tested on 6.3). It seems it was changed in this commit:

openbsd/src@beb8b0d

Co-authored-by: kpcyrd <git@rxv.cc>
@bors
Copy link
Contributor

bors bot commented May 6, 2018

Timed out

@asomers
Copy link
Member

asomers commented May 6, 2018

bors retry

bors bot added a commit that referenced this pull request May 6, 2018
@bors @kpcyrd
Merge #893
8a5a762 
893: Remove IFF_NOTRAILERS for openbsd r=asomers a=kpcyrd

nix currently doesn't compile on openbsd because `IFF_NOTRAILERS` isn't found. This PR fixes the openbsd build again (tested on 6.3). It seems it was changed in this commit:

openbsd/src@beb8b0d

Co-authored-by: kpcyrd <git@rxv.cc>
@bors
Copy link
Contributor

bors bot commented May 6, 2018

Build succeeded

@bors bors bot merged commit c838e64 into nix-rust:master May 6, 2018
@kpcyrd kpcyrd deleted the fix-openbsd branch May 6, 2018 19:59
@kpcyrd
Copy link
Contributor Author

kpcyrd commented May 6, 2018

The crate is now successfully building on OpenBSD 6.3: https://openbsd.rs/#/builders/8

Any chance we could get a release soon? :)

@kpcyrd kpcyrd mentioned this pull request May 6, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asomers asomers asomers approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kpcyrd @asomers