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

Nix repl - fails to read /proc files #3026

Closed
SRGOM opened this issue Aug 6, 2019 · 3 comments · Fixed by #3546
Closed

Nix repl - fails to read /proc files #3026

SRGOM opened this issue Aug 6, 2019 · 3 comments · Fixed by #3546

Comments

@SRGOM
Copy link
Member

SRGOM commented Aug 6, 2019 •
edited
Loading

Please see very small shell session for understanding:

$nix repl                                                                                                                                                             Welcome to Nix version 2.1.3. Type :? for help.

nix-repl> builtins.readFile /etc/os-release
"NAME=NixOS\nID=nixos\nVERSION=\"18.09.2466.884b0a4417e (Jellyfish)\"\nVERSION_CODENAME=jellyfish\nVERSION_ID=\"18.09.2466.884b0a4417e\"\nPRETTY_NAME=\"NixOS 18.09.2466.884b0a4417e (Jellyfish)\"\nHOME_URL=\"https://nixos.org/\"\nSUPPORT_URL=\"https://nixos.org/nixos/support.html\"\nBUG_REPORT_URL=\"https://github.com/NixOS/nixpkgs/issues\"\n"

nix-repl> builtins.readFile /proc/cpuinfo
""
@SRGOM
Copy link
Member Author

SRGOM commented Sep 29, 2019

Also- /sys/class/dmi/id/product_version (and other files in the same dir)

@cleverca22
Copy link
Contributor

i think thats because it will first stat() the file to get its size, and proc claims all files are 0 bytes in size

@matthewbauer matthewbauer mentioned this issue Apr 27, 2020
Closed
@guibou
Copy link
Contributor

guibou commented Apr 27, 2020

Sorry, I've opened #3539 as duplicate.

I observed that /proc files are actually easily readable from inside a runCommand if you really want to do that.

guibou added a commit to guibou/nix that referenced this issue Apr 29, 2020
@guibou
builtins.readFile: do not truncate content
ef314a9 
This closes NixOS#3026 by allowing `builtins.readFile` to read a file with a
wrongly reported file size, for example, files in `/proc` may report a
file size of 0. Reading file in `/proc` is not a good enough motivation,
however I do think it just makes nix more robust by allowing more file
to be read.  Especially, I do considerer the previous behavior to be
dangerous because nix was previously reading truncated files. Examples
of file system which incorrectly report file size may be network file
system or dynamic file system (for performance reason, a dynamic file
system such as FUSE may generate the content of the file on demand).

```
nix-repl> builtins.readFile "/proc/version"
""
```

With this commit:

```
nix-repl> builtins.readFile "/proc/version"
"Linux version 5.6.7 (nixbld@localhost) (gcc version 9.3.0 (GCC)) NixOS#1-NixOS SMP Thu Apr 23 08:38:27 UTC 2020\n"
```

Here is a summary of the behavior changes:

- If the reported size is smaller, previous implementation
was silently returning a truncated file content. The new implementation
is returning the correct file content.

- If a file had a bigger reported file size, previous implementation was
failing with an exception, but the new implementation is returning the
correct file content. This change of behavior is coherent with this pull
request.

Open questions

- The behavior is unchanged for correctly reported file size, however
performances may vary because it uses the more complex sink interface.
Considering that sink is used a lot, I don't think this impacts the
performance a lot.
- `builtins.readFile` on an infinite file, such as `/dev/random` may
fill the memory.
guibou added a commit to guibou/nix that referenced this issue Apr 29, 2020
@guibou
builtins.readFile: do not truncate content
5a34a47 
This closes NixOS#3026 by allowing `builtins.readFile` to read a file with a
wrongly reported file size, for example, files in `/proc` may report a
file size of 0. Reading file in `/proc` is not a good enough motivation,
however I do think it just makes nix more robust by allowing more file
to be read.  Especially, I do considerer the previous behavior to be
dangerous because nix was previously reading truncated files. Examples
of file system which incorrectly report file size may be network file
system or dynamic file system (for performance reason, a dynamic file
system such as FUSE may generate the content of the file on demand).

```
nix-repl> builtins.readFile "/proc/version"
""
```

With this commit:

```
nix-repl> builtins.readFile "/proc/version"
"Linux version 5.6.7 (nixbld@localhost) (gcc version 9.3.0 (GCC)) NixOS#1-NixOS SMP Thu Apr 23 08:38:27 UTC 2020\n"
```

Here is a summary of the behavior changes:

- If the reported size is smaller, previous implementation
was silently returning a truncated file content. The new implementation
is returning the correct file content.

- If a file had a bigger reported file size, previous implementation was
failing with an exception, but the new implementation is returning the
correct file content. This change of behavior is coherent with this pull
request.

Open questions

- The behavior is unchanged for correctly reported file size, however
performances may vary because it uses the more complex sink interface.
Considering that sink is used a lot, I don't think this impacts the
performance a lot.
- `builtins.readFile` on an infinite file, such as `/dev/random` may
fill the memory.
- it does not support adding file to store, such as `${/proc/version}`.
@guibou guibou mentioned this issue Apr 29, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

builtins.readFile: do not truncate content guibou/nix
3 participants
@cleverca22 @SRGOM @guibou