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

unistd: add fchdir(2) #497

Merged
merged 2 commits into from
Jan 19, 2017
Merged

unistd: add fchdir(2) #497

merged 2 commits into from
Jan 19, 2017

Conversation

lucab
Copy link
Contributor

@lucab lucab commented Jan 17, 2017

This introduces a wrapper for fchdir(2), allowing a process to change
directory based on an open file descriptor.

The underlying function is available in libc crate since 0.2.20.

@lucab
Copy link
Contributor Author

lucab commented Jan 17, 2017

Note: this has been recently added in libc crate (rust-lang/libc#496), which has been released today.

@posborne
Copy link
Member

Excellent, thanks for the contribution @lucab. Can you add an entry to the changelog for this addition?

@posborne
Copy link
Member

Also, it would be good to understand what might be happening with this failure. We currently have some known issues with aarch64 which is why it is not gating the green check, but the failure does seem suspicious:

It would appear that the system call is not working as desired in the test environment for these targets.

@lucab lucab force-pushed the to-upstream/fchdir branch 2 times, most recently from 1b311a2 to 9ae73b3 Compare January 18, 2017 08:51
@lucab
Copy link
Contributor Author

lucab commented Jan 18, 2017

Oops, I forgot the changelog, added now.

Regarding the cross-test failure, I think I'm hitting a weird combination of an old kernel on travis + some chroot/mount-ns trick by docker + (likely) some bug on aufs/overlayfs. fchdir() works, but the original root-fd seems to be leading somewhere else in the FS, possibly the lowerdir of a stacked fs. I've reworked the test to avoid grabbing a root-fd and use a self-contained tmpdir instead, now waiting for travis result of this (but it looks like it is not picking it up).

@posborne
Copy link
Member

@lucab Looks like macos has an issue that needs to be resolved in the tests now.

This introduces a wrapper for fchdir(2), allowing a process to change
directory based on an open file descriptor.

The underlying function is available in libc crate since 0.2.20.
@lucab
Copy link
Contributor Author

lucab commented Jan 19, 2017

@posborne ack. It turned out the test was not canonicalizing paths, so it was fooled by symlinks. test_fchdir is now passing everywhere, despite some other (I guess unrelated) failures on cross-tests. PTAL.

@posborne
Copy link
Member

Well, that was fun 🥇.

@homu r+

@homu
Copy link
Contributor

homu commented Jan 19, 2017

📌 Commit 0a654e7 has been approved by posborne

@homu
Copy link
Contributor

homu commented Jan 19, 2017

⚡ Test exempted - status

@homu homu merged commit 0a654e7 into nix-rust:master Jan 19, 2017
homu added a commit that referenced this pull request Jan 19, 2017
unistd: add fchdir(2)

This introduces a wrapper for fchdir(2), allowing a process to change
directory based on an open file descriptor.

The underlying function is available in libc crate since 0.2.20.
@lucab
Copy link
Contributor Author

lucab commented Jan 19, 2017

@posborne great, thanks! Please ping here back once this gets into a released crate, so I can drop some private wrappers I have in a project which needs this feature.

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

Successfully merging this pull request may close these issues.

None yet

3 participants