this is only used to close non-stdio files in derivation sandboxes. we
may as well encode that in its name, drop the unnecessary integer set,
and use close_range to deal with the actual closing of files. not only
is this clearer, it also makes sandbox setup on linux fast by 1ms each
(cherry-picked and adapted from
c7d97802e4)
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
Co-authored-by: Cole Helbling <cole.e.helbling@outlook.com>
Co-authored-by: John Ericson <git@JohnEricson.me>
Since withFramedSink() is now used a lot more than in the past (for
every addToStore() variant), we were creating a lot of threads, e.g.
nix flake show --no-eval-cache --all-systems github:NixOS/nix/afdd12be5e19c0001ff3297dea544301108d298
would create 46418 threads. While threads on Linux are cheap, this is
still substantial overhead.
So instead, just poll from FramedSink before every write whether there
are pending messages from the daemon. This could slightly increase the
latency on log messages from the daemon, but not on exceptions (which
were only synchronously checked from FramedSink anyway).
This speeds up the command above from 19.2s to 17.5s on my machine (a
9% speedup).
Incorrectly high expectations lead to frustration for users who
stick around to experience how useless it is for e.g. a devShell
https://functional.cafe/@arianvp/112976284363120036:
> Flakes doesn't have eval caching. It has command line argument
> caching. It literally just stores the cli argument you passed
> in a sqlite database and yes that's as useless as it sounds
> When I discovered flakes had no expression level caching whatsoever
> I kind of felt lied to and betrayed.
Setting it to /bin/sh will make it more predictable when users have
their favorite shell in SHELL, which might not behave as expected.
For instance, a bad rc file could send something to stdout before
our LocalCommand gets to write "started".
This may help https://github.com/NixOS/nix/issues/11010
* add cross-references to `nix-path` overriding
while this information is already present in the settings, it's more
likely to be first accessed through the "lookup path" page, which
currently requires following two links to get to the practically
important bits.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
This is because with the split packages of the Meson build, we simply
have no idea what directory the binaries will be installed in when we
build the library.
In the process of doing so, consolidate and make more sophisticated the
logic to cope with a few corner cases (e.g. `NIX_BIN_DIR` exists, but no
binaries are inside it).
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
In d60c3f7f7c, this was changed to close a
hole in the sandbox. Unfortunately, this was too restrictive such that it
made local port binding fail, thus making derivations that needed
`__darwinAllowLocalNetworking` gain nearly nothing, and thus largely
fail (as the primary use for it is to enable port binding).
This unfortunately does mean that a sandboxed build process can, in
coordination with an actor outside the sandbox, escape the sandbox by
binding a port and connecting to it externally to send data. I do not
see a way around this with my experimentation and understanding of the
(quite undocumented) macOS sandbox profile API. Notably it seems not
possible to use the sandbox to do any of:
- Restrict the remote IP of inbound network requests
- Restrict the address being bound to
As such, the `(local ip "*:*")` here appears to be functionally no
different than `(local ip "localhost:*")` (however it *should* be
different than removing the filter entirely, as that would make it also
apply to non-IP networking). Doing `(allow network-inbound (require-all
(local ip "localhost:*") (remote ip "localhost:*")))` causes listening
to fail.
Note that `network-inbound` implies `network-bind`.
This ended up motivating a good deal of other infra improvements in
order to get Windows right:
- `OsString` to complement `std::filesystem::path`
- env var code for working with the underlying `OsString`s
- Rename `PATHNG_LITERAL` to `OS_STR`
- `NativePathTrait` renamed to `OsPathTrait`, given a character template
parameter until #9205 is complete.
Split `tests.cc` matching split of `util.{cc,hh}` last year.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
The test split matches PR #8920, so the utility files and tests files
are once again to 1-1. The string changes continues what was started in
PR #11093.
In the FFI world we have many tools that are not gcc/clang and therefore
not always support the latest C standard. This fixes support with cffi
i.e. used in https://github.com/tweag/python-nix
It was failing with:
error: AWS error fetching 'nix-cache-info': The specified bucket does not exist
because `S3BinaryCacheStoreImpl` had a `bucketName` field that
shadowed the inherited `bucketName from `S3BinaryCacheStoreConfig`.
We didn't even realize you *could* use this syntax with -E and -f, much
less that the attribute path could be *empty*.
Change-Id: Id1a6715609f3a76a5ce477bd43a7832effbbe07b
* docs: unify documentation on search paths
- put all the information on search path semantics into `builtins.findFile`
- put all the information on determining the value of `builtins.nixPath` into the
`nix-path` setting
maybe `builtins.nixPath` is a better place for this, but those bits
can still be moved around now that it's all next to each other.
- link to the syntax page for lookup paths from all places that are
concerned with it
- add or clarify examples
- add a test verifying a claim from documentation
This also bans various sneaking of negative numbers from the language
into unsuspecting builtins as was exposed while auditing the
consequences of changing the Nix language integer type to a newtype.
It's unlikely that this change comprehensively ensures correctness when
passing integers out of the Nix language and we should probably add a
checked-narrowing function or something similar, but that's out of scope
for the immediate change.
During the development of this I found a few fun facts about the
language:
- You could overflow integers by converting from unsigned JSON values.
- You could overflow unsigned integers by converting negative numbers
into them when going into Nix config, into fetchTree, and into flake
inputs.
The flake inputs and Nix config cannot actually be tested properly
since they both ban thunks, however, we put in checks anyway because
it's possible these could somehow be used to do such shenanigans some
other way.
Note that Lix has banned Nix language integer overflows since the very
first public beta, but threw a SIGILL about them because we run with
-fsanitize=signed-overflow -fsanitize-undefined-trap-on-error in
production builds. Since the Nix language uses signed integers, overflow
was simply undefined behaviour, and since we defined that to trap, it
did.
Trapping on it was a bad UX, but we didn't even entirely notice
that we had done this at all until it was reported as a bug a couple of
months later (which is, to be fair, that flag working as intended), and
it's got enough production time that, aside from code that is IMHO buggy
(and which is, in any case, not in nixpkgs) such as
https://git.lix.systems/lix-project/lix/issues/445, we don't think
anyone doing anything reasonable actually depends on wrapping overflow.
Even for weird use cases such as doing funny bit crimes, it doesn't make
sense IMO to have wrapping behaviour, since two's complement arithmetic
overflow behaviour is so *aggressively* not what you want for *any* kind
of mathematics/algorithms. The Nix language exists for package
management, a domain where bit crimes are already only dubiously in
scope to begin with, and it makes a lot more sense for that domain for
the integers to never lose precision, either by throwing errors if they
would, or by being arbitrary-precision.
Fixes: https://github.com/NixOS/nix/issues/10968
Original-CL: https://gerrit.lix.systems/c/lix/+/1596
Change-Id: I51f253840c4af2ea5422b8a420aa5fafbf8fae75
The actual motive here is the avoidance of integer overflow if we were
to make these use checked NixInts and retain the subtraction.
However, the actual *intent* of this code is a three-way comparison,
which can be done with operator<=>, so we should just do *that* instead.
Change-Id: I7f9a7da1f3176424b528af6d1b4f1591e4ab26bf
- move <sys/resource.h> from a __linux__ block to a !_WIN32 block: this
matches what the actual code does, using getrlimit() & setrlimit() in
!_WIN32 blocks
- drop <sys/mount.h>, which is not portable, and it is not used
This is not allowed in C++20, and GCC 14 warns about it:
../src/libutil/ref.hh:26:20: warning: template-id not allowed for constructor in C++20 [-Wtemplate-id-cdtor]
26 | explicit ref<T>(const std::shared_ptr<T> & p)
| ^
../src/libutil/ref.hh:26:20: note: remove the '< >'
../src/libutil/ref.hh:33:21: warning: template-id not allowed for constructor in C++20 [-Wtemplate-id-cdtor]
33 | explicit ref<T>(T * p)
| ^
../src/libutil/ref.hh:33:21: note: remove the '< >'
This change updates the seccomp profile to return ENOTSUP for getxattr
functions family. This reflects the behavior of filesystems that don’t
support extended attributes (or have an option to disable them), e.g.
ext2.
The current behavior is confusing for some programs because we can read
extended attributes, but only get to know that they are not supported
when setting them. In addition to that, ACLs on Linux are implemented
via extended attributes internally and if we don’t return ENOTSUP, acl
library converts file mode to ACL.
https://git.savannah.nongnu.org/cgit/acl.git/tree/libacl/acl_get_file.c?id=d9bb1759d4dad2f28a6dcc8c1742ff75d16dd10d#n69
The internal "completionCallback" and "listPossibleCallback" helpers
are used only when building with editline; hence, do not build then
when using readline, matching their usage in
"ReadlineLikeInteracter::init()".
By syncing with Nixpkgs, we reuse the same derivation, which is
generally a good idea, and has the benefit that it is transitively
a channel blocker.
Changes:
- https://github.com/NixOS/nixpkgs/pull/163313 (SuperSandro2000)
> nix: disable big-parallel for aws-sdk-cpp
> aws-sdk-cpp only takes ~1m52s on a 4 core machine under 50% load
> which does not justify the requirement on big parallel.
> Tested with `nix-build -A nixVersions.nix_2_6.aws-sdk-cpp`.
> I can finally build nix without requiring a big-parallel machine.
- https://github.com/NixOS/nixpkgs/pull/227506 (Artturin)
> nix: use [ ] instead null to empty requiredSystemFeatures
> fixes 'error: value is null while a list was expected' with 'nixpkgs.hostPlatform.gcc.arch = "x86_64";'
This was done originally because std::smatch does not accept `const char
*` as iterators. However, this was because we should have been using
std::cmatch instead.
(cherry picked from commit 12a5838d11)
Found by looking for interesting asan reports from the test suite.
What happened here is that name got overwritten, but it was what
actually held the backing memory for the thing it got overwritten by,
which was a by-reference value coming out of std::regex.
Due to absurd reasons I cannot seem to use a string_view iterator here,
so I just copy the string with a longer lifetime instead. idk lol
==3796364==ERROR: AddressSanitizer: heap-use-after-free on address 0x503000014c61 at pc 0x74843523bf1d bp 0x7ffc68351330 sp 0x7ffc68350af0
READ of size 3 at 0x503000014c61 thread T0
0 0x74843523bf1c in __asan_memcpy (/nix/store/mzhqknx2mc94jdz4n320hn1lml86398y-clang-wrapper-17.0.6/resource-root/lib/linux/libclang_rt.asan-x86_64.so+0x159f1c)
1 0x6403cf6cbff4 in std::char_traits<char>::copy(char*, char const*, unsigned long) /nix/store/14c6s4xzhy14i2b05s00rjns2j93gzz4-gcc-13.2.0/include/c++/13.2.0/bits/char_traits.h:445:33
<...>
7 0x6403cf6cbff4 in std::__cxx11::sub_match<__gnu_cxx::__normal_iterator<char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>::str() const /nix/store/14c6s4xzhy14i2b05s00rjns2j93gzz4-gcc-13.2.0/include/c++/13.2.0/bits/regex.h:966:6
8 0x6403cf6cbff4 in std::__cxx11::sub_match<__gnu_cxx::__normal_iterator<char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>>::operator std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>() const /nix/store/14c6s4xzhy14i2b05s00rjns2j93gzz4-gcc-13.2.0/include/c++/13.2.0/bits/regex.h:955:16
9 0x6403cf6cbff4 in nix::getClosureInfo[abi:cxx11](nix::ref<nix::Store>, nix::StorePath const&) /home/jade/lix/lix2/build/src/nix/diff-closures.cc:37:26
10 0x6403cf6cd70c in nix::printClosureDiff(nix::ref<nix::Store>, nix::StorePath const&, nix::StorePath const&, std::basic_string_view<char, std::char_traits<char>>) /home/jade/lix/lix2/build/src/nix/diff-closures.cc:54:25
11 0x6403cf873331 in CmdProfileDiffClosures::run(nix::ref<nix::Store>) /home/jade/lix/lix2/build/src/nix/profile.cc:479:17
<...>
0x503000014c61 is located 17 bytes inside of 21-byte region [0x503000014c50,0x503000014c65)
freed by thread T0 here:
0 0x748435250470 in operator delete(void*) (/nix/store/mzhqknx2mc94jdz4n320hn1lml86398y-clang-wrapper-17.0.6/resource-root/lib/linux/libclang_rt.asan-x86_64.so+0x16e470)
<...>
6 0x6403cf6cbda2 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::~basic_string() /nix/store/14c6s4xzhy14i2b05s00rjns2j93gzz4-gcc-13.2.0/include/c++/13.2.0/bits/basic_string.h:792:9
7 0x6403cf6cbda2 in nix::getClosureInfo[abi:cxx11](nix::ref<nix::Store>, nix::StorePath const&) /home/jade/lix/lix2/build/src/nix/diff-closures.cc:36:13
8 0x6403cf6cd70c in nix::printClosureDiff(nix::ref<nix::Store>, nix::StorePath const&, nix::StorePath const&, std::basic_string_view<char, std::char_traits<char>>) /home/jade/lix/lix2/build/src/nix/diff-closures.cc:54:25
<...>
previously allocated by thread T0 here:
0 0x74843524fa38 in operator new(unsigned long) (/nix/store/mzhqknx2mc94jdz4n320hn1lml86398y-clang-wrapper-17.0.6/resource-root/lib/linux/libclang_rt.asan-x86_64.so+0x16da38)
<...>
9 0x6403cf6cb68c in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>::basic_string<std::basic_string_view<char, std::char_traits<char>>, void>(std::basic_string_view<char, std::char_traits<char>> const&, std::allocator<char> const&) /nix/store/14c6s4xzhy14i2b05s00rjns2j93gzz4-gcc-13.2.0/include/c++/13.2.0/bits/basic_string.h:784:4
10 0x6403cf6cb68c in nix::getClosureInfo[abi:cxx11](nix::ref<nix::Store>, nix::StorePath const&) /home/jade/lix/lix2/build/src/nix/diff-closures.cc:33:21
11 0x6403cf6cd70c in nix::printClosureDiff(nix::ref<nix::Store>, nix::StorePath const&, nix::StorePath const&, std::basic_string_view<char, std::char_traits<char>>) /home/jade/lix/lix2/build/src/nix/diff-closures.cc:54:25
12 0x6403cf873331 in CmdProfileDiffClosures::run(nix::ref<nix::Store>) /home/jade/lix/lix2/build/src/nix/profile.cc:479:17
<...>
(cherry-picked from b9b1bbd22f)
* manual: Contributing -> Development, Hacking -> Building
what's currently called "hacking" are really instructions for setting up
a development environment and compiling from source. we have
a contribution guide in the repo (which rightly focuses on GitHub
workflows), and the material in the manual is more about working
on the code itself.
since we'd otherwise have three headings that amount to "Building Nix",
this change also moves the "classic Nix" instructions to the top.
we may want to reorganise this in the future, and bring
contributor-oriented information closer to the code, but for now let's
stick to more accurate names to ease navigation.
* fix NIX_PATH overriding
- test restricted evaluation
- test precedence for setting the search path
Co-authored-by: Robert Hensing <robert@roberthensing.nl>
Co-authored-by: John Ericson <git@JohnEricson.me>
We are piping curl downloads into `unpackTarfileToSink()`, but the
latter is typically slower than the former if you're on a fast
connection. So the download could appear unnecessarily slow. (There is
even a risk that if the Git import is *really* slow for whatever
reason, the TCP connection could time out.)
So let's make the download buffer bigger by default - 64 MiB is big
enough for the Nixpkgs tarball. Perhaps in the future, we could have
an unlimited buffer that spills data to disk beyond a certain
threshold, but that's probably overkill.
Currently, the worker protocol has a version number that we increment
whenever we change something in the protocol. However, this can cause
a collision between Nix PRs / forks that make protocol changes
(e.g. PR #9857 increments the version, which could collide with
another PR). So instead, the client and daemon now exchange a set of
protocol features (such as `auth-forwarding`). They will use the
intersection of the sets of features, i.e. the features they both
support.
Note that protocol features are completely distinct from
`ExperimentalFeature`s.
This is in accordance with ARM's naming convention.
"Low" is confusing, because it could refer to either the cold end
of the stack as an abstract data type, or a low address.
These are different places, because the stack grows down through
the address space.
... as well as match buildReadlineNoMarkdown.
Unfortunately it doesn't support long inputs or multiline inputs
for now.
This needs to make better use of the interacter interface.
This avoids the double warning
warning: 'ping-store' is a deprecated alias for 'store ping'
warning: 'nix store ping' is a deprecated alias for 'nix store info'
* Only build perl subproject on Linux
* Fix various Windows regressions
* Don't put the emulator hook in test builds
We run the tests in a separate derivation. Only need it for the dev shell.
* Fix native dev shells
* Fix cross dev shells we don't know how to emulate
Co-authored-by: PoweredByPie <poweredbypie@users.noreply.github.com>
Co-authored-by: Joachim Schiele <js@lastlog.de>
Co-authored-by: John Ericson <John.Ericson@Obsidian.Systems>
In _very_ rare cases (I had about 7 cases out of 32200 files!),
the order of how inherit-from bindings are printed when using
`nix-instantiate --parse` gets messed up.
The cause of this seems to be because the std::map the bindings are
placed in is keyed on a _pointer_, which then uses an
[implementation-defined strict total order](https://en.cppreference.com/w/cpp/language/operator_comparison#Pointer_total_order).
The fix here is to key the bindings on their displacement instead,
which maintains the same order as they appear in the file.
Unfortunately I wasn't able to make a reproducible test for this in the
source, there's something about the local environment that makes it
unreproducible for me.
However I was able to make a reproducible test in a Nix build on a Nix
version from a very recent master:
nix build github:infinisil/non-det-nix-parsing-repro
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Following what is outlined in #10766 refactor the uds-remote-store such
that the member variables (state) don't live in the store itself but in
the config object.
Additionally, the config object includes a new necessary constructor
that takes a scheme & authority.
Tests are commented out because of linking errors with the current config system.
When there is a new config system we can reenable them.
Co-authored-by: John Ericson <John.Ericson@Obsidian.Systems>
This generally gives a better experience with bindings generators,
possibly other tooling.
A possible risk is that some generators may not represent unknown
codes correctly.
Rust bindgen by default generates suitable code:
* a type alias nix_err = c_int
* individual constants for the known enum values
It does _not_ generate a closed type that can only hold the values
that were known at code generation time.
If this proves to be a problem, we could instead split the type:
`typedef int nix_err;` for return values
`enum nix_known_err` for code generation.
This would complicate the interface, so let's not do it unless it
is shown to be needed.
This was accidentally introduced
in f71b4da0b3. We didn't notice this
because the version got interpreted by the daemon as the obsolete "CPU
affinity will follow" field, and being non-zero, it would then read
another integer for the ignored CPU affinity.
The default value for the setting was evaluated by
calling a method on the object _being currently constructed_,
so we were using it before all fields were initialized.
This has been fixed by making the called method static,
and not using the previously used fields at all.
But functionality hasn't changed!
The fields were usually always zero (by chance?) anyway,
meaning the conditional path was always taken.
Thus the current logic has been kept, the code simplified,
and UB removed.
This was found with the helper of UBSan.
Splitting it out immediately answers questions like [this],
without increasing the number of compilation units.
I did consider using boost::hash_combine instead, but it doesn't seem
to be quite as capable, accepting only two arguments.
[this]: https://github.com/NixOS/nix/pull/11113#discussion_r1679991573
Progress towards #10766
I thought that #10768 achieved, but when I went to use this stuff (in
Hydra), turns out it did not. (Those `using FooConfig;` lines were not
working --- they are so finicky!) This PR gets the job done, and adds
some trivial unit tests to make sure I did what I intended.
I had to add add a header to expose `SSHStoreConfig`, after which the
preexisting `ssh-store-config.*` were very confusingly named files, so I
renamed them to `common-ssh-store-config.hh` to match the type defined
therein.
They are not actually part of the store layer, but instead part of the
Nix executable infra (libraries don't need plugins, executables do).
This is part of a larger project of moving all of our legacy settings
infra to libmain, and having the underlying libraries just have plain
configuration structs detached from any settings infra / UI layer.
Progress on #5638
This makes it possible to certain discern failures from empty
snippets, which I think is an ok review comment.
Maybe it should do so for swapped column indexes too, but I'm not
sure.
I don't think it matters in the grand scheme. We don't even have
a real use case for `nullopt` now anyway.
Since we don't have a use case, I'm not applying this logic to
higher level functions yet.
Unfortunately these don't render correctly, because they go into the
markdown renderer, instead of the terminal.
```
nix-repl> :doc lib.version
Attribute '[35;1mversion[0m'
… defined at [35;1m/home/user/h/nixpkgs/lib/default.nix:73:40[0m
```
We could switch that to go direct to the terminal, but then we should
do the same for the primops, to get a consistent look.
Reverting for now.
This reverts commit 3413e0338cbee1c7734d5cb614b5325e51815cde.
fetchurl can be given a name and url aside from just the url.
Giving a name can be useful if the url has invalid characters such as
tilde for the store.
... at call sites that are may be in the hot path.
I do not know how clever the compiler gets at these sites.
My primary concern is to not regress performance and I am confident
that this achieves it the easy way.
When the separator is empty, no difference is observable.
Note that concatStringsSep has centralized definitions. This adds the
required definitions. Alternatively, `strings-inline.hh` could be
included at call sites.
Considering that `value` was probably parsed with tokenizeString
prior, it's unlikely to contain empty strings, and we have no
reason to remove them either.
Empty attributes are probably not well supported, but the least we
could do is leave a hint.
Attribute path rendering and parsing should be done according to
Nix expression syntax in my opinion.
(System) features are unlikely to be empty strings, but when they
come in through structuredAttrs, they probably can.
I don't think this means we should drop them, but most likely they
will be dropped after this because next time they'll be parsed with
tokenizeString.
TODO: We should forbid empty features.
I don't think it's completely impossible, but I can't construct
one easily as derivationStrict seems to (re)tokenize the outputs
attribute, dropping the empty output.
It's not a scenario we have to account for here.
Bug not reported in 6 years, but here you go.
Also it is safe to switch to normal concatStringsSep behavior
because tokenizeString does not produce empty items.
The empty attribute name should not be dropped from attribute paths.
Rendering attribute paths with concatStringsSep is lossy and wrong,
but this is just a first improvement while dealing with the
dropEmptyInitThenConcatStringsSep problem.
Known behavior changes:
- `MemorySourceAccessor`'s comparison operators no longer forget to
compare the `SourceAccessor` base class.
Progress on #10832
What remains for that issue is hopefully much easier!
- Fix eval cache not being persisted in `nix develop` (since #10570)
- Don't attempt to commit cache transaction if there is no active transaction, which will spew errors in edge cases
- Drive-by: trivial typo fix
