Make nix/unpack-channel.nix a builtin builder by edolstra · Pull Request #2748 · NixOS/nix

edolstra · 2019-03-27T23:17:42Z

This rewrites the builder of <nix/unpack-channel.nix> in C++ and Rust. Previously the builder was a shell script. Since this was the only builder that was still a shell script, this allows us to get rid of the dependency on bash, coreutils, tar, xz, bzip2 etc., reducing Nix's closure size.

This is mostly an experiment in using Rust in Nix. It uses the tar crate to do the parsing of tar files.

matthewbauer · 2019-03-28T01:21:15Z

src/libstore/builtins/unpack-channel.cc

+    auto source = sinkToSource([&](Sink & sink) {
+        auto decompressor =
+            hasSuffix(src, ".bz2") ? makeDecompressionSink("bzip2", sink) :
+            hasSuffix(src, ".xz") ? makeDecompressionSink("xz", sink) :


Yeah, we currently lack gzip support in compression.cc. Probably not a big deal since our channels don't use it, but it would be nice to add.

matthewbauer · 2019-03-28T01:25:03Z

configure.ac

-  [path of cat, mkdir, etc.]),
-  coreutils=$withval, coreutils=$(dirname $cat))
-AC_SUBST(coreutils)
+AC_SUBST(coreutils, [$(dirname $(type -p cat))])


For cross compilation purposes, it is nice to let this be a flag. These should just be runtime deps AFAICT. See NixOS/nixpkgs#58104

Well, with this PR, it's now only used during the tests, which I assume cannot be run in a cross-compiled build anyway?

That's true right now

configure.ac

nix-rust/src/lib.rs

lovesegfault · 2019-07-03T04:15:06Z

This is blocking on getting Nix working well on a number of Linux distros. As an employee of a Nix-based company, it'd be really nice if this landed so I can go back to doing work on my machine, which I haven't been able to for a while now.

Janiczek · 2019-07-09T18:29:26Z

@lovesegfault (I'm unaffiliated with Nix, but...) If your company is that dependent on Nix, maybe it could fund some dedicated time from the Nix developers on this issue?

lovesegfault · 2019-07-09T20:36:15Z

@Janiczek We definitely should, I'll see what I can do.

illegalprime · 2019-09-15T11:11:17Z

I think this fixes NixOS/nixpkgs#58104 as I understand it. Thanks for this!

This was the last function using a shell script, so this allows us to get rid of tar, coreutils, bash etc.

This is no longer needed.

We can now convert Rust Errors to C++ exceptions. At the Rust->C++ FFI boundary, Result<T, Error> will cause Error to be converted to and thrown as a C++ exception.

In particular, these are emitted by 'git archive' (in fetchGit).

Also, fetchGit now runs in O(1) memory since we pipe the output of 'git archive' directly into unpackTarball() (rather than first reading it all into memory).

https://hydra.nixos.org/build/107466992

https://hydra.nixos.org/build/107467517 Seems that on i686-linux, gcc and rustc disagree on how to return 1-word structs: gcc has the caller pass a pointer to the result, while rustc has the callee return the result in a register. Work around this by using a bare pointer.

This is a hack to fix the build on macOS, which was failing because libnixrust.a contains compiler builtins that clash with libclang_rt.osx.a. There's probably a better solution... https://hydra.nixos.org/build/107473280

puckipedia

I know that the code's been merged for more than a week now, but I still felt the need to post some opinions; I didn't actually assume this code would be merged any time, as i did not regard it as production-ready, with many of the FIXMEs strewn about.

puckipedia · 2019-12-07T16:18:25Z

nix-rust/src/lib.rs

+        unsafe {
+            let size = std::mem::size_of::<T>();
+            let ptr = libc::malloc(size);
+            *(ptr as *mut T) = t; // FIXME: probably UB


This is indeed UB, as you're dereferencing an arbitrary value (which could be illegal for this T); this could cause double-frees, arbitrary UAFs, segfaults, etc.

puckipedia · 2019-12-07T16:22:55Z

nix-rust/src/error.rs

+
+extern "C" {
+    #[allow(improper_ctypes)] // YOLO
+    fn make_error(s: &str) -> CppException;


Fat pointers are, afaik, not FFI-safe at all, and may just do the entire wrong thing.

puckipedia · 2019-12-07T16:26:29Z

src/libutil/rust-ffi.hh

+namespace rust {
+
+// Depending on the internal representation of Rust slices is slightly
+// evil...


It is. There's zero guarantee that the internal representation of a slice will look like this.

puckipedia · 2019-12-07T16:27:52Z

src/libutil/rust-ffi.hh

+    }
+};
+
+/* C++ representation of Rust's Result<T, CppException>. */


Note that there's no guarantee that the enum actually is represented like this; Rust does a lot of niche filling and depending on the type of T the tag might just disappear.

puckipedia · 2019-12-07T16:30:57Z

nix-rust/src/lib.rs

+    source: foreign::Source,
+    dest_dir: &str,
+) -> CBox<Result<(), error::CppException>> {
+    CBox::new(tarfile::unpack_tarfile(source, dest_dir).map_err(|err| err.into()))


Returning a CBox<Result<(), error::CppException>> seems kind of useless, why not just return a pointer to a CppException directly? This also saves allocations in the best case, and is actually FFI-safe.

kaii-zen · 2019-12-09T14:40:06Z

Is this merged to the flakes branch? I can't seem to be able to fetch flakes that aren't already in the store. Getting the suspicious-looking:

unpacking 'https://api.github.com/repos/NixOS/hydra/tarball/47797576838974c8209536b67bb45e953a50900f'...
error: numeric field did not have utf-8 text: �ps�� when getting cksum for �
(use '--show-trace' to show detailed location information)
error: evaluation of the deployment specification failed

Ericson2314 · 2019-12-09T20:39:24Z

CC @edef1c

matthewbauer reviewed Mar 28, 2019

View reviewed changes

configure.ac Outdated Show resolved Hide resolved

Ericson2314 reviewed Apr 10, 2019

View reviewed changes

nix-rust/src/lib.rs Outdated Show resolved Hide resolved

edolstra mentioned this pull request May 8, 2019

'nix-channel --update' stopped working on nix-2.2.1: error: while setting up the build environment: getting attributes of path '': No such file or directory #2673

Closed

edolstra force-pushed the rust branch from 58c0e2d to 752437c Compare July 4, 2019 23:24

edolstra changed the title ~~[WIP] Make nix/unpack-channel.nix a builtin builder~~ Make nix/unpack-channel.nix a builtin builder Jul 5, 2019

edolstra force-pushed the rust branch from ac5bb12 to 441dc95 Compare September 5, 2019 08:57

domenkozar mentioned this pull request Sep 29, 2019

nix: allow static compilation NixOS/nixpkgs#70024

Merged

3 tasks

edolstra mentioned this pull request Oct 22, 2019

Port Nix to Meson #3160

Closed

edolstra mentioned this pull request Nov 12, 2019

fetchTarball and implicit dependencies on tar and gzip? #1234

Closed

edolstra added 15 commits November 26, 2019 22:07

Add some Rust code

11da5b2

Enable Rust code to call C++ Source objects

e60f6bd

Make <nix/unpack-channel.nix> a builtin builder

045708d

This was the last function using a shell script, so this allows us to get rid of tar, coreutils, bash etc.

Remove most of <nix/config.nix>

6a9c815

This is no longer needed.

Clean up the configure script

87b7b25

Update Rust dependencies

0dbb249

Include cargo dependencies in the Nix tarball

d722e21

Reduce the size of the vendor directory by removing some winapi cruft

afb0218

Only pass '-Z offline' to cargo if we have a vendor directory

343ebcc

Rust cleanup

8110b4e

More Rust FFI adventures

f738cd4

We can now convert Rust Errors to C++ exceptions. At the Rust->C++ FFI boundary, Result<T, Error> will cause Error to be converted to and thrown as a C++ exception.

Move code around

b7fba16

Shut up some rust warnings

d14b1c2

Move code around

d33dd6e

Ignore tar header entries

f2bd847

In particular, these are emitted by 'git archive' (in fetchGit).

edolstra added 3 commits November 26, 2019 22:07

Drop remaining uses of external "tar"

8918bae

Also, fetchGit now runs in O(1) memory since we pipe the output of 'git archive' directly into unpackTarball() (rather than first reading it all into memory).

-Z offline -> --offline

88f8063

Update Cargo.lock

e6c1d1b

edolstra force-pushed the rust branch from 5351f1b to e6c1d1b Compare November 26, 2019 21:52

edolstra added 5 commits November 27, 2019 00:17

Fix macOS build

dbc4f9d

https://hydra.nixos.org/build/107466992

When OPTIMIZE=0, build rust code in debug mode

f553a8b

make clean: Delete nix-rust/target

895ce1b

edolstra merged commit f102d79 into NixOS:master Nov 29, 2019

edolstra deleted the rust branch November 29, 2019 18:33

edolstra mentioned this pull request Dec 5, 2019

Add gzip decompression support #3256

Closed

puckipedia reviewed Dec 7, 2019

View reviewed changes

yorickvP mentioned this pull request Dec 9, 2019

Decompress tar files using libarchive #3259

Merged

yorickvP mentioned this pull request Dec 10, 2019

Automatically generate cache for stackage projects input-output-hk/haskell.nix#358

Closed

doronbehar mentioned this pull request May 16, 2020

nix: replace build utils with host utils in config.nix NixOS/nixpkgs#58104

Closed

10 tasks

ldesgoui mentioned this pull request Aug 1, 2021

Write Nix 2.4 release notes #4521

Closed

Uh oh!

Comments

Conversation

edolstra commented Mar 27, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lovesegfault commented Jul 3, 2019

Uh oh!

Janiczek commented Jul 9, 2019

Uh oh!

lovesegfault commented Jul 9, 2019

Uh oh!

illegalprime commented Sep 15, 2019

Uh oh!

puckipedia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaii-zen commented Dec 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ericson2314 commented Dec 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

kaii-zen commented Dec 9, 2019 •

edited

Loading