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

Save cwd fd instead of path #6386

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cole-h
Copy link
Member

@cole-h cole-h commented Apr 8, 2022

This reverts commit 56009b2.


Submitted as a draft at the behest of @Ericson2314.

Note that this does not work, for some reason. See some of the discussion in #6348 for what went wrong (specifically, it appears to strip /home/vin from /home/vin/workspace/vcs/nix for some unknown reason).

@edolstra
Copy link
Member

edolstra commented Apr 8, 2022

I thought the FD method didn't work? I didn't follow the entire discussion though.

@cole-h
Copy link
Member Author

cole-h commented Apr 8, 2022

Sorry I didn't make this part clear -- it didn't, but @Ericson2314 requested I resubmit this part as a draft so that someone in the future can try to take this further / investigate the exact issue behind it.

@Ericson2314 Ericson2314 requested review from Ericson2314 and removed request for Ericson2314 April 8, 2022 20:49
@Ericson2314 Ericson2314 self-assigned this Apr 8, 2022
@Ericson2314
Copy link
Member

Yeah because of things like #2759 I am very keen on doing as much as possible with file descriptors. So I figured I could assign this draft PR to myself to hopefully debug someday.

@stale stale bot added the stale label Oct 29, 2022
@Ericson2314
Copy link
Member

@cole-h Would you know how to make a test here demonstrating the failure?

@stale stale bot removed the stale label Feb 19, 2023
@cole-h
Copy link
Member Author

cole-h commented Feb 19, 2023

Currently? No. But I'll give it a shot tomorrow and see if I can't make one. I'll probably start with just a simple reproducer using the magic of flakes and work my way to a more comprehensive test.

@Ericson2314
Copy link
Member

That would be wonderful, thanks in advance, @cole-h!

@cole-h
Copy link
Member Author

cole-h commented Feb 20, 2023

(Rebased)

OK, so here's a simple reproducer:

  1. nix build github:cole-h/nix/7aebe9181068d7313a4b10f56df00adad2b0e924
  2. sudo result/bin/nix shell nixpkgs#coreutils -c pwd
  3. See the error: pwd: couldn't find directory entry in ‘../../../..’ with matching i-node

However, if you do sudo result/bin/nix shell nixpkgs#coreutils -c sh -c pwd, it works as expected. I don't know what's going on there, but hopefully this is enough info for someone to investigate further if they want to.

I had a hard time trying to write a NixOS test for it, so this is the best I can do.

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.

3 participants