Spent a while debugging why `nix-copy-closure` wasn't working anymore
and it was my shell RC printing something I added for debug.
Hopefully this can save someone else some time.
All OS and IO operations should be moved out, leaving only some misc
portable pure functions.
This is useful to avoid copious CPP when doing things like Windows and
Emscripten ports.
Newly exposed functions to break cycles:
- `restoreSignals`
- `updateWindowSize`
The new `MemorySourceAccessor` rather than being a slightly lossy flat
map is a complete in-memory model of file system objects.
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
This adds publicKeys as an optional fetcher input attribute to flakes
and builtins.fetchGit to provide a nix interface for the json-encoded
`publicKeys` attribute of the git fetcher.
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
This implements the git input attributes `verifyCommit`, `keytype`,
`publicKey` and `publicKeys` as experimental feature
`verified-fetches`. `publicKeys` should be a json string.
This representation was chosen because all attributes must be of type bool,
int or string so they can be included in flake uris (see definition of
fetchers::Attr).
When doing local builds, we get phase reporting lines in the log file,
they look like '@nix {"action":"setPhase","phase":"unpackPhase"}'.
With the ssh-ng protocol, we do have access to these messages, but since we
are only including messages of type resBuildLogLine in the logs, the phase
information does not end up in the log file.
The phase reporting could probably be improved altoghether (it looks like it
is kind of accidental that these JSON messages for phase reporting show up
but others don't, just because they are actually emitted by nixpkgs' stdenv),
but as a first step I propose to make ssh-ng behave in the same way as local builds do.
Deduplicating code moreover enforcing the pattern means:
- It is easier to write new characterization tests because less boilerplate
- It is harder to mess up new tests because there are fewer places to
make mistakes.
Co-authored-by: Jacek Galowicz <jacek@galowicz.de>
Noticed because of a warning during an rpm build:
*** WARNING: ./usr/src/debug/nix-2.18.1-1.fc40.x86_64/src/nix-copy-closure/nix-copy-closure.cc is executable but has no shebang, removing executable bit
*** WARNING: ./usr/src/debug/nix-2.18.1-1.fc40.x86_64/src/nix-channel/nix-channel.cc is executable but has no shebang, removing executable bit
I wouldn't call it *good* yet, but this will do for now.
- `RetrieveRegularNARSink` renamed to `RegularFileSink` and moved
accordingly because it actually has nothing to do with NARs in
particular.
- its `fd` field is also marked private
- `copyRecursive` introduced to dump a `SourceAccessor` into a
`ParseSink`.
- `NullParseSink` made so `ParseSink` no longer has sketchy default
methods.
This was done while updating #8918 to work with the new
`SourceAccessor`.
Adding the inputPath as a positional feature uncovered this bug.
As positional argument forms were discarded from the `expectedArgs`
list, their closures were not. When the `.completer` closure was then
called, part of the surrounding object did not exist anymore.
This didn't cause an issue before, but with the new call to
`getEvalState()` in the "inputs" completer in nix/flake.cc, a segfault
was triggered reproducibly on invalid memory access to the `this`
pointer, which was always 0.
The solution of splicing the argument forms into a new list to extend
their lifetime is a bit of a hack, but I was unable to get the "nicer"
iterator-based solution to work.
Instead of making a complete copy of the repo, fetching the
submodules, and writing the result to the store (which is all
superexpensive), we now fetch the submodules recursively using the Git
fetcher, and return a union accessor that "mounts" the accessors for
the submodules on top of the root accessor.
We now have `schemeName` and `allowedAttrs` functions for this purpose.
We look up the schema with the former; we restrict the set of input
attributes with the latter.
The brings a number of advantages, including:
- Easier to update test data if design changes (and I do think our
derivation JSON is not yet complaint with the guidelines).
- Easier to reuse test data in other implementations, inching closer to
compliance tests for Nix *the concept* rather than any one
implementation.
Before the change builder ID exhaustion printed the following message:
[0/1 built] waiting for UID to build '/nix/store/hiy9136x0iyib4ssh3w3r5m8pxjnad50-python3.11-breathe-4.35.0.drv'
After the change it should be:
[0/1 built] waiting for a free build user ID for '/nix/store/hiy9136x0iyib4ssh3w3r5m8pxjnad50-python3.11-breathe-4.35.0.drv'
Committing a lock file using markFileChanged() required the input to
be writable by the caller in the local filesystem (using the path
returned by getSourcePath()). putFile() abstracts over this.
Rather than having a misc tutorial page in the grab-bag "package management" section, this information should just be part of the S3 store docs.
---------
Co-authored-by: John Ericson <John.Ericson@Obsidian.Systems>
End goal: make `(mkDerivation x).drvPath` behave like a non-DrvDeep
context.
Problem: users won't be able to recover the DrvDeep behavior when
nixpkgs makes this change.
Solution: add this primop.
The new primop is fairly simple, and is supposed to complement other
existing ones (`builtins.storePath`, `builtins.outputOf`) so there are
simple ways to construct strings with every type of string context
element.
(It allows nothing we couldn't already do with `builtins.getContext` and `builtins.appendContext`, which is also true of those other two primops.)
This was originally in #8595, but then it was proposed to land some doc
changes separately. So now the code changes proper is just moved to
this, and the doc will be done in that.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.nore
github.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io
Leading whitespace after `nix-shell` used to produce an empty argument,
while an empty argument at the end of the line was ignored.
Fix the first issue by consuming the initial whitespace before calling
shellwords; fix the second issue by returning immediately if whitespace
is found at the end of the string instead of checking for an empty
string.
Also throw if quotes aren't terminated.
Single quotes are a basic feature of shell syntax that people expect to
work. They are also more convenient for writing literal code expressions
with less escaping.
As I complained in
https://github.com/NixOS/nix/pull/6784#issuecomment-1421777030 (a
comment on the wrong PR, sorry again!), #6693 introduced a second
completions mechanism to fix a bug. Having two completion mechanisms
isn't so nice.
As @thufschmitt also pointed out, it was a bummer to go from `FlakeRef`
to `std::string` when collecting flake refs. Now it is `FlakeRefs`
again.
The underlying issue that sought to work around was that completion of
arguments not at the end can still benefit from the information from
latter arguments.
To fix this better, we rip out that change and simply defer all
completion processing until after all the (regular, already-complete)
arguments have been passed.
In addition, I noticed the original completion logic used some global
variables. I do not like global variables, because even if they save
lines of code, they also obfuscate the architecture of the code.
I got rid of them moved them to a new `RootArgs` class, which now has
`parseCmdline` instead of `Args`. The idea is that we have many argument
parsers from subcommands and what-not, but only one root args that owns
the other per actual parsing invocation. The state that was global is
now part of the root args instead.
This did, admittedly, add a bunch of new code. And I do feel bad about
that. So I went and added a lot of API docs to try to at least make the
current state of things clear to the next person.
--
This is needed for RFC 134 (tracking issue #7868). It was very hard to
modularize `Installable` parsing when there were two completion
arguments. I wouldn't go as far as to say it is *easy* now, but at least
it is less hard (and the completions test finally passed).
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
It does not belong with the data type itself.
This also materializes the fact that `copyPath` does not do any version
negotiation just just hard-codes "16".
The non-standard interface of these serializers makes it harder to test,
but this is fixed in the next commit which then adds those tests.
Worker Protocol:
Note that the worker protocol already had a serialization for
`BuildResult`; this was added in
a4604f1928. It didn't have any versioning
support because at that time reusable seralizers were not away for the protocol
version. It could thus only be used for new messages also introduced in
that commit.
Now that we do support versioning in reusable serializers, we can expand
it to support all known versions and use it in many more places.
The exist test data becomes the version 1.29 tests: note that those
files' contents are unchanged. 1.28 and 1.27 tests are added to cover
the older code-paths.
The keyered build result test only has 1.29 because the keying was also
added in a4604f19284254ac98f19a13ff7c2216de7fe176; the older
serializations are always used unkeyed.
Serve Protocol:
Conversely, no attempt was made to factor out such a serializer for the
serve protocol, so our work there in this commit for that protocol
proceeds from scratch.
It was some ad-hoc functions to account for versions, while the already
factored-out serializer just supported the latest version.
Now, we can fold that version-specific logic into the factored out one,
and so we do.
It is dead code. It was added in
8e0946e8df as part of the repeated /
enforce-determinism feature, but that was removed in
8fdd156a65.
It is not good because it skips many fields. For testing purposes we
will soon want to add a new one that doesn't skip fields, but we want to
make sure making == sensitive to those fields won't change how Nix
works. Proving in this commit that the old version is dead code achieves
that.
hashBase is ambiguous, since it's not about the digital bases, but about
the format of hashes. Base16, Base32 and Base64 are all character maps
for binary encoding.
Rename the enum Base to HashFormat.
Rename variables of type HashFormat from [hash]Base to hashFormat,
including CmdHashBase::hashFormat and CmdToBase::hashFormat.
MemoryInputAccessor is an in-memory virtual filesystem that returns
files like <nix/fetchurl.nix>. This removes the need for special hacks
to handle those files.
This will allow us to factor out logic, which is currently scattered
inline, into several reusable instances
The tests are also updated to support versioning. Currently all Worker
and Serve protocol tests are using the minimum version, since no
version-specific serialisers have been created yet. But in subsequent
commits when that changes, we will test individual versions to ensure
complete coverage.
16591eb3cc (diff-19f999107b609d37cfb22c58e7f0bc1cf76edf1180e238dd6389e03cc279b604) (2013) added support for files to doBind
This is work towards allowing users to change the location of chrootRootDir, to, for example, a tmpfs.
inspired by trofi on matrix
> It looks like build sandbox created by nix-daemon runs on the same filesystem, as /nix/store including things like /tmp which makes all small temporary files hit the disk. Is it intentional? If it is is there an easy way to redirect chroot's root to be tmpfs?
dirsInChroot -> pathsInChroot
Two changes:
* The (probably unintentional) hack to handle paths as tarballs has
been removed. This is almost certainly not what users expect and is
inconsistent with flakeref handling everywhere else.
* The hack to support scp-style Git URLs has been moved to the Git
fetcher, so it's now supported not just by fetchTree but by flake
inputs.
Add a new experimental `impure-env` setting that is a key-value list of
environment variables to inject into FOD derivations that specify the
corresponding `impureEnvVars`.
This allows clients to make use of this feature (without having to change the
environment of the daemon itself) and might eventually deprecate the current
behaviour (pick whatever is in the environment of the daemon) as it's more
principled and might prevent information leakage.
To start, it is just a clone of the common protocol. But now that we
have the separate protocol implementations, we can add versioning
information without the versions of one protocol leaking into another.
Using the infrastructure from the previous commit, we don't have to
duplicate code for shared behavior.
Motivation: No more perverse incentives. [0] did some awkward things
because the serialisers did not store the version. I don't want anyone
making changes to be pushed towards keeping the serialization logic with
the core data types just because it's easier or the alternative is
tedious.
The actual versioning of the Worker and Serve protocol serialisers
(Common remains unversioned as the underlying mini-protocols are not
versioned) will happen in subsequent commits / PRs.
[0]: fe1f34fa60
Copy the relevant tests to ensure the new interfaces added in the last
commit are tested.
Perhaps I should try to deduplicat these tests some more. However its
not clear how to do that outside of a big ugly C++ macro.
https://github.com/google/googletest/blob/main/docs/advanced.md has some
stuff but it is cumbersome and I didn't figure it out yet.
This is done in a separate commit in order to be sure that the first
commit really didn't change any behavior; if we changed the
implementation and the tests at once, it would be harder to tell whether
or not some behavioral changes slipped in what is supposed to be a "pure
refactor".
Co-Authored-By: Valentin Gagarin <valentin.gagarin@tweag.io>
This introduces some shared infrastructure for our notion of protocols.
We can then define multiple protocols in terms of that notion.
We an also express how particular protocols depend on each other.
For example, we can define a common protocol and a worker protocol,
where the second depends on the first in terms of the data types it can
read and write.
The "serve" protocol can just use the common one for now, but will
eventually need its own machinary just like the worker protocol for
version-aware serialisers
I think it is bad for these reasons when `tests/` contains a mix of
functional and integration tests
- Concepts is harder to understand, the documentation makes a good
unit vs functional vs integration distinction, but when the
integration tests are just two subdirs within `tests/` this is not
clear.
- Source filtering in the `flake.nix` is more complex. We need to
filter out some of the dirs from `tests/`, rather than simply pick
the dirs we want and take all of them. This is a good sign the
structure of what we are trying to do is not matching the structure
of the files.
With this change we have a clean:
```shell-session
$ git show 'HEAD:tests'
tree HEAD:tests
functional/
installer/
nixos/
```
the `term` output mode leaves inline HTML around verbatim, while `nroff`
mode (used for `man` pages) does not.
the correct solution would be to pre-render all output with a more
benign tool so we have less liabilities in our own code, but this has to
do for now.
This has been the behaviour before Nix 2.4. It was dropped in a rewrite
in 759947bf72, allowing the creation of
store paths that aren't considered valid by older Nix versions or other
Nix tooling.
Nix 2.4 didn't ship in NixOS until 22.05, and stdenv.mkDerivation in
nixpkgs drops leading periods since April 2022, so it's unlikely anyone
is relying on the current lax behaviour.
Closes#9091.
Change-Id: I4a57bd9899e1b0dba56870ae5a1b680918a18ce9
This was somewhat of a false alarm. The problem was not that the
protocol implementation actually failed to round trip, but that two of
the fields were ignored entirely --- not serialized and deserialized at
all.
For reference, those fields were added in
fa68eb367e.
This reverts commit 5e3986f59c. This
un-implements RFC 92 but fixes the critical bug #9052 which many people
are hitting. This is a decent stop-gap until a minimal reproduction of
that bug is found and a proper fix can be made.
Mostly fixed#9052, but I would like to leave that issue open until we
have a regression test, so I can then properly fix the bug (unbreaking
RFC 92) later.
In #4770 I implemented proper `nix-shell(1)` support for derivations
using `__structuredAttrs = true;`. Back then we decided to introduce two
new environment variables, `NIX_ATTRS_SH_FILE` for `.attrs.sh` and
`NIX_ATTRS_JSON_FILE` for `.attrs.json`. This was to avoid having to
copy these files to `$NIX_BUILD_TOP` in a `nix-shell(1)` session which
effectively meant copying these files to the project dir without
cleaning up afterwords[1].
On last NixCon I resumed hacking on `__structuredAttrs = true;` by
default for `nixpkgs` with a few other folks and getting back to it,
I identified a few problems with the how it's used in `nixpkgs`:
* A lot of builders in `nixpkgs` don't care about the env vars and
assume that `.attrs.sh` and `.attrs.json` are in `$NIX_BUILD_TOP`.
The sole reason why this works is that `nix-shell(1)` sources
the contents of `.attrs.sh` and then sources `$stdenv/setup` if it
exists. This may not be pretty, but it mostly works. One notable
difference when using nixpkgs' stdenv as of now is however that
`$__structuredAttrs` is set to `1` on regular builds, but set to
an empty string in a shell session.
Also, `.attrs.json` cannot be used in shell sessions because
it can only be accessed by `$NIX_ATTRS_JSON_FILE` and not by
`$NIX_BUILD_TOP/.attrs.json`.
I considered changing Nix to be compatible with what nixpkgs
effectively does, but then we'd have to either move $NIX_BUILD_TOP for
shell sessions to a temporary location (and thus breaking a lot of
assumptions) or we'd reintroduce all the problems we solved back then
by using these two env vars.
This is partly because I didn't document these variables back
then (mea culpa), so I decided to drop all mentions of
`.attrs.{json,sh}` in the manual and only refer to `$NIX_ATTRS_SH_FILE`
and `$NIX_ATTRS_JSON_FILE`. The same applies to all our integration tests.
Theoretically we could deprecated using `"$NIX_BUILD_TOP"/.attrs.sh` in
the future now.
* `nix develop` and `nix print-dev-env` don't support this environment
variable at all even though they're supposed to be part of the replacement
for `nix-shell` - for the drv debugging part to be precise.
This isn't a big deal for the vast majority of derivations, i.e.
derivations relying on nixpkgs' `stdenv` wiring things together
properly. This is because `nix develop` effectively "clones" the
derivation and replaces the builder with a script that dumps all of
the environment, shell variables, functions etc, so the state of
structured attrs being "sourced" is transmitted into the dev shell and
most of the time you don't need to worry about `.attrs.sh` not
existing because the shell is correctly configured and the
if [ -e .attrs.sh ]; then source .attrs.sh; fi
is simply omitted.
However, this will break when having a derivation that reads e.g. from
`.attrs.json` like
with import <nixpkgs> {};
runCommand "foo" { __structuredAttrs = true; foo.bar = 23; } ''
cat $NIX_ATTRS_JSON_FILE # doesn't work because it points to /build/.attrs.json
''
To work around this I employed a similar approach as it exists for
`nix-shell`: the `NIX_ATTRS_{JSON,SH}_FILE` vars are replaced with
temporary locations.
The contents of `.attrs.sh` and `.attrs.json` are now written into the
JSON by `get-env.sh`, the builder that `nix develop` injects into the
derivation it's debugging. So finally the exact file contents are
present and exported by `nix develop`.
I also made `.attrs.json` a JSON string in the JSON printed by
`get-env.sh` on purpose because then it's not necessary to serialize
the object structure again. `nix develop` only needs the JSON
as string because it's only written into the temporary file.
I'm not entirely sure if it makes sense to also use a temporary
location for `nix print-dev-env` (rather than just skipping the
rewrite in there), but this would probably break certain cases where
it's relied upon `$NIX_ATTRS_SH_FILE` to exist (prime example are the
`nix print-dev-env` test-cases I wrote in this patch using
`tests/shell.nix`, these would fail because the env var exists, but it
cannot read from it).
[1] https://github.com/NixOS/nix/pull/4770#issuecomment-836799719
While `nix` has always been respectful towards requests for `NO_COLOR=1`, this change asks represents a new stage of maturity for `nix` - making it also respect quests for `NOCOLOR=1`.
This ideally makes the tool more accessible to folks like me, who are exhausted by guessing whether `NO_COLOR` or `NOCOLOR` is the right environment variable to set.
<3
