Merge in latest master #2
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reduces the number of store queries it performs. Also prints a warning if any of the selectors did not match any installed derivations. UX Caveats: - Will print a warning that nothing matched if a previous selector already removed the path - Will not do anything if no selectors were provided (no change from before). Fixes NixOS#3531
Suppose I have a path /nix/store/[hash]-[name]/a/a/a/a/a/[...]/a, long enough that everything after "/nix/store/" is longer than 4096 (MAX_PATH) bytes. Nix will happily allow such a path to be inserted into the store, because it doesn't look at all the nested structure. It just cares about the /nix/store/[hash]-[name] part. But, when the path is deleted, we encounter a problem. Nix will move the path to /nix/store/trash, but then when it's trying to recursively delete the trash directory, it will at some point try to unlink /nix/store/trash/[hash]-[name]/a/a/a/a/a/[...]/a. This will fail, because the path is too long. After this has failed, any store deletion operation will never work again, because Nix needs to delete the trash directory before recreating it to move new things to it. (I assume this is because otherwise a path being deleted could already exist in the trash, and then moving it would fail.) This means that if I can trick somebody into just fetching a tarball containing a path of the right length, they won't be able to delete store paths or garbage collect ever again, until the offending path is manually removed from /nix/store/trash. (And even fixing this manually is quite difficult if you don't understand the issue, because the absolute path that Nix says it failed to remove is also too long for rm(1).) This patch fixes the issue by making Nix's recursive delete operation use unlinkat(2). This function takes a relative path and a directory file descriptor. We ensure that the relative path is always just the name of the directory entry, and therefore its length will never exceed 255 bytes. This means that it will never even come close to AX_PATH, and Nix will therefore be able to handle removing arbitrarily deep directory hierachies. Since the directory file descriptor is used for recursion after being used in readDirectory, I made a variant of readDirectory that takes an already open directory stream, to avoid the directory being opened multiple times. As we have seen from this issue, the less we have to interact with paths, the better, and so it's good to reuse file descriptors where possible. I left _deletePath as succeeding even if the parent directory doesn't exist, even though that feels wrong to me, because without that early return, the linux-sandbox test failed. Reported-by: Alyssa Ross <hi@alyssa.is> Thanks-to: Puck Meerburg <puck@puckipedia.com> Tested-by: Puck Meerburg <puck@puckipedia.com> Reviewed-by: Puck Meerburg <puck@puckipedia.com>
Fix long paths permanently breaking GC
Set GCROOT to store path to prevent garbage collection
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)) #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}`.
The commit 3cc1125 adds a `grantpt` call on the builder pseudo terminal fd. This call is actually only required for MacOS, but it however requires a RW access to /dev/pts which is only RO bindmounted in the Bazel Linux sandbox. So, Nix can not be actually run in the Bazel Linux sandbox for unneeded reasons.
Only call grantpt on MacOS systems
Now it is always `drain` (see previous commit).
When used with `readFile`, we have a pretty good heuristic of the file size, so `reserve` this in the `string`. This will save some allocation / copy when the string is growing.
Without dereferencing this pointer, you'd get an error like this: ``` error: unsupported argument 'abc' to 'fetchTarball', at 0x13627e8 ```
Fix displaying error-position in `builtins.fetch{Tree,Tarball}`
Tested against NixOS/nixpkgs#72074. Fixes NixOS#3540.
This avoids inheriting the caller's shellHook, which can happen when running a dev-shell inside a dev-shell.
Add an option to print the logs in a machine-readable format
Add tests for pool.hh
This function was used in only one place, where it could easily be replaced by readDerivation() since it's not performance-critical. (This function appears to have been modelled after queryDerivationOutputs(), which exists only to make the garbage collector faster.)
This replaces the copy&paste with a helper function in hash.hh.
Allow empty hash in derivations
Use `std::string_view` in a few more places
This means that 'throw Error({ ... ErrorInfo ... })' now works.
These are now shown in the progress bar. Closes NixOS#3577.
E.g. instead of error: --- BuildError ----------------------------------------------- nix builder for '/nix/store/03nk0a3n8h2948k4lqfgnnmym7knkcma-foo.drv' failed with exit code 1 error: --- Error ---------------------------------------------------- nix build of '/nix/store/03nk0a3n8h2948k4lqfgnnmym7knkcma-foo.drv' failed we now get error: --- Error ---------------------------------------------------- nix builder for '/nix/store/03nk0a3n8h2948k4lqfgnnmym7knkcma-foo.drv' failed with exit code 1
|
As the latest commit is really old, I would prefer to do a rebase. How are your experiments going over at https://github.com/obsidiansystems/nix ? |
|
For what it's worth, in the end I used a nginx reverse proxy to make the files available from ipfs. This does not require a patched nix daemon: {
"tarballs.example.com" = {
forceSSL = true;
enableACME = true;
root = "/srv/ipfs/tarballs";
locations."/" = {
extraConfig = ''
autoindex on;
'';
};
};
"cache.example.com" = {
forceSSL = true;
enableACME = true;
root = "/srv/ipfs/binary_cache";
};
"channels.example.com" = {
forceSSL = true;
enableACME = true;
root = "/srv/ipfs/channels";
locations."/" = {
extraConfig = ''
autoindex on;
'';
};
};
}use together with https://github.com/NixIPFS/infrastructure/blob/master/ipfs_mirror/deploy.nix |
|
Sure we can turn this into a rebase if you like, though these merges commit is already included in our history FWIW, Our experiments are going good! It's true that the version here doesn't offer much over just using the HTTP gateway to some IPFS node, but we are about to leverage IPLD to represent the references graph, narfiles, etc., natively, which should up the value proposition for directly integrating in Nix by making pinning and other things much nicer. |
@Ericson2314 I just gave you access to this repository. Feel free to push your branch(es). I pushed to https://github.com/NixIPFS/nix/tree/ipfs_binary_cache_legacy to preserve the single commit at the top for future reference. |
|
For the short term it's not just me but a bunch of us on the team that need access, so we'll continue using the company fork. But after we complete our initial proposal and our work transitions to a proper community project, I'd love to use this repo/organization for anything that isn't merged upstream, so thanks in advance. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It builds, but I need to figure out how to test this.