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

Get SO_PEERCRED working on all Linux targets #921

Merged
merged 1 commit into from Jul 5, 2018

Conversation

@jonas-schievink
Copy link
Contributor

@jonas-schievink jonas-schievink commented Jul 4, 2018

These were disabled for ARM way back in 0db6ed1 and 09c00ed. Try to enable them for all arches and Android as well, since the removal wasn't really explained and I see no reason why this shouldn't work.

@jonas-schievink jonas-schievink force-pushed the jonas-schievink:more-arm-consts branch from bb861fe to db579b1 Jul 4, 2018
@jonas-schievink
Copy link
Contributor Author

@jonas-schievink jonas-schievink commented Jul 4, 2018

CI passes, so I've squashed and added a changelog entry.

@asomers
Copy link
Member

@asomers asomers commented Jul 4, 2018

@berkowski @Susurrus can you comment about why these socket options were disabled? Is there any problem with reenabling them?

@berkowski
Copy link
Contributor

@berkowski berkowski commented Jul 5, 2018

They were disabled at the time to get nix passing arm-based tests. There's been a lot of changes in the upstream libs since then so maybe a few missing symbols were added?

I don't have the knowledge to say definitively whether that flag is valid on all platforms. I'd trust it is if libc exports it at this point.

@asomers
Copy link
Member

@asomers asomers commented Jul 5, 2018

@berkowski do you recall if the arm-based tests were failing deterministically, or intermittently?

@berkowski
Copy link
Contributor

@berkowski berkowski commented Jul 5, 2018

@asomers, I don't remember specifics, sorry. Most of my effort at that time was just getting nix (and libc) to compile under the new architectures exposed by trust. I'd guess that I was chasing down compile-time issues rather than run-time test failings.

@asomers
Copy link
Member

@asomers asomers commented Jul 5, 2018

That sounds likely, and it's certainly working now. However, It's developed a merge conflict in the changelog. @jonas-schievink you'll have to take care of that.

@jonas-schievink jonas-schievink force-pushed the jonas-schievink:more-arm-consts branch from db579b1 to 594b924 Jul 5, 2018
@asomers
Copy link
Member

@asomers asomers commented Jul 5, 2018

bors r+

bors bot added a commit that referenced this pull request Jul 5, 2018
Merge #921
921: Get `SO_PEERCRED` working on all Linux targets r=asomers a=jonas-schievink

These were disabled for ARM way back in 0db6ed1 and 09c00ed. Try to enable them for all arches and Android as well, since the removal wasn't really explained and I see no reason why this shouldn't work.

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
@bors bors bot merged commit 594b924 into nix-rust:master Jul 5, 2018
4 checks passed
4 checks passed
bors Build succeeded
Details
buildbot/nix-rust/nix amd64_fbsd11 Build done.
Details
buildbot/nix-rust/nix i386_fbsd11 Build done.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jonas-schievink jonas-schievink deleted the jonas-schievink:more-arm-consts branch Jul 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants