Commit graph

8753 commits

Author SHA1 Message Date
John Ericson aa74c7b0bc Gate experimental features in DerivationOutput::fromJSON
This is an entry point for outside data, so we need to check enabled
experimental features here.
2023-04-17 17:36:12 -04:00
Robert Hensing 64ee02890c
Merge pull request #8230 from obsidiansystems/daemon-trust-override
Experimentally allow forcing `nix-daemon` trust; use this to test
2023-04-17 19:43:41 +02:00
John Ericson d41e1bed5e Experimentally allow forcing nix-daemon trust; use this to test
We finally test the status quo of remote build trust in a number of
ways. We create a new experimental feature on `nix-daemon` to do so.

PR #3921, which improves the situation with trustless remote building,
will build upon these changes. This code / tests was pull out of there
to make this, so everything is easier to review, and in particular we
test before and after so the new behavior in that PR is readily apparent
from the testsuite diff alone.
2023-04-17 13:06:21 -04:00
John Ericson 2c8475600d Fix some issues with experimental config settings
Issues:

1. Features gated on disabled experimental settings should warn and be
   ignored, not silently succeed.

2. Experimental settings in the same config "batch" (file or env var)
   as the enabling of the experimental feature should work.

3. For (2), the order should not matter.

These are analogous to the issues @roberth caught with my changes for
arg handling, but they are instead for config handling.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
2023-04-17 12:41:04 -04:00
Robert Hensing 3f9589f17e
Merge pull request #6312 from obsidiansystems/keyed-build-result
Shuffle `BuildResult` data definition, make state machine clearer, introduce `SingleDrvOutputs`
2023-04-17 18:08:05 +02:00
Robert Hensing e641de085b
Merge pull request #3746 from obsidiansystems/path-info
Introduce `StoreReferences` and `ContentAddressWithReferences`
2023-04-17 15:49:48 +02:00
John Ericson 537e8719f2
Explain various .self = false,
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
2023-04-17 09:15:11 -04:00
John Ericson 72ffa7fedb
Merge pull request #7732 from hercules-ci/make-initLibStore-viable-alternative
Make `initLibStore` a viable alternative
2023-04-17 08:04:41 -04:00
Robert Hensing 9af9c260fc
Merge pull request #8193 from hercules-ci/dry-strings
Deduplicate string literal rendering, fix 4909
2023-04-17 11:19:40 +02:00
John Ericson 9800c1e807 Mark experimental configuration settings programmatically
Fix #8162

The test is changed to compare `nlohmann::json` values, not strings of dumped
JSON, which allows us to format things more nicely.
2023-04-16 10:58:04 -04:00
Robert Hensing b6125772d7 libexpr: Move identifier-like printing to print.cc 2023-04-16 14:07:35 +02:00
Robert Hensing 28a5cdde02 libexpr/value/print.* -> libexpr/print.*
Generalizes the file to sensibly allow printing any part of the
language syntax.
2023-04-16 13:10:45 +02:00
Robert Hensing 1e2dd669bc printLiteral: Do not overload 2023-04-16 13:04:35 +02:00
Robert Hensing 9c74df5bb4
Format
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
Co-authored-by: John Ericson <git@JohnEricson.me>
2023-04-15 20:56:51 +02:00
John Ericson 24866b71c4 Introduce SingleDrvOutputs
In many cases we are dealing with a collection of realisations, they are
all outputs of the same derivation. In that case, we don't need
"derivation hashes modulos" to be part of our map key, because the
output names alone will be unique. Those hashes are still part of the
realisation proper, so we aren't loosing any information, we're just
"normalizing our schema" by narrowing the "primary key".

Besides making our data model a bit "tighter" this allows us to avoid a
double `for` loop in `DerivationGoal::waiteeDone`. The inner `for` loop
was previously just to select the output we cared about without knowing
its hash. Now we can just select the output by name directly.

Note that neither protocol is changed as part of this: we are still
transferring `DrvOutputs` over the wire for `BuildResult`s. I would only
consider revising this once #6223 is merged, and we can mention protocol
versions inside factored-out serialization logic. Until then it is
better not change anything because it would come a the cost of code
reuse.
2023-04-15 12:51:19 -04:00
John Ericson 0f2b5146c7 Make restarting state machines explicit
If my memory is correct, @edolstra objected to modifying `wantedOutputs`
upon falling back to doing a build (as we did before), because we should
only modify it in response to new requests --- *actual* wants --- and
not because we are "incidentally" building all the outptus beyond what
may have been requested.

That's a fair point, and the alternative is to replace the boolean soup
with proper enums: Instead of modifying `wantedOuputs` som more, we'll
modify `needsRestart` to indicate we are passed the need.
2023-04-15 11:01:31 -04:00
John Ericson 37fca662b0 Make KeyedBuildResult, BuildResult like before, and fix bug another way
In https://github.com/NixOS/nix/pull/6311#discussion_r834863823, I
realized since derivation goals' wanted outputs can "grow" due to
overlapping dependencies (See `DerivationGoal::addWantedOutputs`, called
by `Worker::makeDerivationGoalCommon`), the previous bug fix had an
unfortunate side effect of causing more pointless rebuilds.

In paticular, we have this situation:

1. Goal made from `DerivedPath::Built { foo, {a} }`.

2. Goal gives on on substituting, starts building.

3. Goal made from `DerivedPath::Built { foo, {b} }`, in fact is just
   modified original goal.

4. Though the goal had gotten as far as building, so all outputs were
   going to be produced, `addWantedOutputs` no longer knows that and so
   the goal is flagged to be restarted.

This might sound far-fetched with input-addressed drvs, where we usually
basically have all our goals "planned out" before we start doing
anything, but with CA derivation goals and especially RFC 92, where *drv
resolution* means goals are created after some building is completed, it
is more likely to happen.

So the first thing to do was restore the clearing of `wantedOutputs` we
used to do, and then filter the outputs in `buildPathsWithResults` to
only get the ones we care about.

But fix also has its own side effect in that the `DerivedPath` in the
`BuildResult` in `DerivationGoal` cannot be trusted; it is merely the
*first* `DerivedPath` for which this goal was originally created.

To remedy this, I made `BuildResult` be like it was before, and instead
made `KeyedBuildResult` be a subclass wit the path. Only
`buildPathsWithResults` returns `KeyedBuildResult`s, everything else
just becomes like it was before, where the "key" is unambiguous from
context.

I think separating the "primary key" field(s) from the other fields is
good practical in general anyways. (I would like to do the same thing
for `ValidPathInfo`.) Among other things, it allows constructions like
`std::map<Key, ThingWithKey>` where doesn't contain duplicate keys and
just precludes the possibility of those duplicate keys being out of
sync.

We might leverage the above someday to overload `buildPathsWithResults`
to take a *set* of return a *map* per the above.

-----

Unfortunately, we need to avoid C++20 strictness on designated
initializers.

(BTW
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2287r1.html
this offers some new syntax for this use-case. Hopefully this will be
adopted and we can eventually use it.)

No having that yet, maybe it would be better to not make
`KeyedBuildResult` a subclass to just avoid this.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
2023-04-15 11:01:31 -04:00
John Ericson 9df7f3f537 Introduce Worker::makeGoal
This takes a `DerivedPath` so the caller doesn't need to care about
which sort of goal does what.
2023-04-15 11:01:31 -04:00
Robert Hensing ab228d73db
Merge pull request #8214 from raphaelr/installable-derived-path-warn
Display valid installable in `InstallableDerivedPath::parse` warning
2023-04-15 14:15:02 +02:00
John Ericson ee420ac64e Legacy vs non-legacy to_string/parse for DerivedPath
As requested by @roberth, it is good to call out the specific instances
we care about, which is `!` for the RPC protocols, and `^` for humans.

This doesn't take advantage of parametricity as much, but since the
human and computer interfaces are good to decouple anyways (we don't
care if they drift further apart over time in the slightest) some
separation and slight duplication is fine.

Also, unit test both round trips.
2023-04-14 20:45:11 -04:00
John Ericson 92346d1ed4
Merge pull request #8174 from NixOS/gloss-xp-features-note-installables
Start cross-referencing experimental features
2023-04-14 08:48:10 -04:00
Robert Hensing f5ab38a688
Merge pull request #8217 from obsidiansystems/push-get-fs-accessor-unsupported-down
Push `getFSAccessor` `unsupported(...)` down `Store` class hierarchy
2023-04-14 14:23:23 +02:00
John Ericson d93e76fbb8 Start cross-referencing experimental features
- Create a glossary entry for experimental features.

- Have the man page experimental feature notice link `nix-commmand`.

  (Eventually this should be programmed, based on whether the command is
  experimental, and if so what experimental feature does it depend on.)

- Document which installables depend on which experimental features.

  I tried to use the same style (bold warning and block quote) that the
  top of the man page uses.

Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
2023-04-14 07:45:08 -04:00
Eelco Dolstra 7eac8838df
Merge pull request #8199 from tweag/fix-sqlite-busy-reporting
Fix unnecessary reporting of SQLite busy errors
2023-04-14 10:50:50 +02:00
Eelco Dolstra 33fc09c2a5
Merge pull request #8176 from tweag/rename-confusing-write-method
Rename and protect `BufferedSink::write`
2023-04-14 10:44:36 +02:00
Raphael Robatsch 9e8f209036 Display valid installable in InstallableDerivedPath::parse warning
The warning message should produce an installable name that can be
passed to `nix build`, `nix path-info`, etc. again. Since the CLI
expects that the .drv path and the output names are separated by
a caret, the warning message must also separate the .drv path and output
names with a caret.

However, `DerivedPath::Built.to_string()` uses an exclamation point as
the separator instead. This commit adds a `separator` argument to the
to_string method.

This changes the warning message from:
If this command is now failing try again with '/nix/store/foo.drv!*'
to:
If this command is now failing try again with '/nix/store/foo.drv^*'
2023-04-14 10:32:03 +02:00
John Ericson ee97f107e8 Push getFSAccessor unsupported(...) down Store class hierarchy
More progress on issue #5729.

Instead of having it by the default method in `Store` itself, have it be
the implementation in `DummyStore` and `LegacySSHStore`. Then just the
implementations which fail to provide the method pay the "penalty" of
dealing with the icky `unimplemented` function for non-compliance.

Combined with my other recent PRs, this finally makes `Store` have no
`unsupported` calls!
2023-04-13 13:39:44 -04:00
Ben Radford de3df3009b
Move warning timing logic into handleSQLiteBusy. 2023-04-11 16:14:16 +01:00
John Ericson 450e5ec618 Do not gate or hide experimental settings
This is somewhat hacky fix just for 2.15. I unintentionally hid them
from the manual, when no one wanted to hide them that (including
myself). I also required the experimental feature to be enabled in an
order-dependent way, which is not good.

The simplest fix for this immanent release is just to always show them,
and always allow them to be set.

Effectively undoes some changes from aa663b7e89
2023-04-11 10:56:48 -04:00
Eelco Dolstra 162c3f5f7c
Merge pull request #8170 from tweag/fix-aarch64-build
Explicitely define `LockFile::operator!=`
2023-04-11 12:35:23 +02:00
Ben Radford 7c56e84213
Warn after a second of being busy instead of immediately.
Getting the occasional SQLITE_BUSY is expected when the database is being
accessed concurrently. The retry will likely succeed so it is pointless to warn
immediately. Instead we track how long each retrySQLite block has been running,
and only begin warning after a second has elapsed (and then every 10 seconds
subsequently).
2023-04-11 11:15:34 +01:00
Eelco Dolstra 60a1bf08b6
Merge pull request #7798 from peeley/list-experimental-features
Documentation: list experimental features in manual
2023-04-11 11:55:35 +02:00
Eelco Dolstra deb7f4b466
Nitpicks 2023-04-11 11:29:35 +02:00
Robert Hensing 4e0804c920 Deduplicate string literal rendering, fix 4909 2023-04-09 22:42:20 +02:00
John Ericson fd21f9d76e Merge remote-tracking branch 'upstream/master' into path-info 2023-04-07 20:39:04 -04:00
John Ericson 8f44edc83e Tidy up and comment daemon CLI
Some of the factoring out was taken from #7912 by @mupdt. Thanks!

No behavior should be changed in this commit.

Co-Authored-By: mupdt <25388474+mupdt@users.noreply.github.com>
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
2023-04-07 16:49:52 -04:00
Robert Hensing 4411c7d7e0
Merge pull request #8179 from tweag/disable-gc-on-coroutine
disable gc on coroutine
2023-04-07 18:21:11 +02:00
Robert Hensing ddebeb934a libstore: Remove lockCPU dead code
Left over from 9747ea84b, https://github.com/NixOS/nix/pull/5821
2023-04-07 17:50:40 +02:00
Robert Hensing 1c0b680ef9 libstore: Remove lockCPU dead code
Left over from 9747ea84b, https://github.com/NixOS/nix/pull/5821
2023-04-07 17:50:40 +02:00
Robert Hensing 2445afd92c Require openssl >= 1.1.1
Versions older this are sufficiently old that we don't want to support
them, and they require extra support code.
2023-04-07 17:50:40 +02:00
Robert Hensing 2196fd1146 libutil: Provide alternatives to startSignalHandlerThread
How signals should be handled depends on what kind of process Nix
is integrated into. The signal handler thread used by the stand-alone
Nix commands / processes may not work well in the context of other
runtime systems, such as those of Python, Perl, or Haskell.
2023-04-07 17:50:40 +02:00
Robert Hensing 781d3dceb3 Move initLibUtil() from initNix to initLibStore
libutil is a dependency of libstore, so it should always be
initialized as such.
libutil is also a dependency of libmain. Being explicit about this
dependency might be good, but not worth the slight code complexity
until the library structure gets more advanced.

Part of an effort to make it easier to initialize the right things,
by moving code into the appropriate libraries.
2023-04-07 16:24:18 +02:00
Robert Hensing 1107ea363f libmain: Clarify the lack of initLibExpr()
Quote

    Why not initLibExpr()? initGC() is essentially that, but
    detectStackOverflow is not an instance of the init function concept, as
    it may have to be invoked more than once per process.

Furthermore, renaming initGC to initLibExpr is more trouble than it's
worth at this time.
2023-04-07 16:24:18 +02:00
Robert Hensing 52d6ce6515 Move macOS TMPDIR hack from initNix to initLibStore
This code is bad. We shouldn't unset variables in programs whose
children may need them. Fixing one issue at a time, so postponing.
See https://github.com/NixOS/nix/issues/7731

Part of an effort to make it easier to initialize the right things,
by moving code into the appropriate libraries.
2023-04-07 16:24:18 +02:00
Robert Hensing e706ffa007 Move preloadNSS() from initNix to initLibStore
It is required for the sandbox, which is a libstore responsibility;
not just libmain.

Part of an effort to make it easier to initialize the right things,
by moving code into the appropriate libraries.
2023-04-07 16:24:18 +02:00
Robert Hensing a58be39476 Move sodium_init() to initLibStore()
Part of an effort to make it easier to initialize the right things,
by moving code into the appropriate libraries.
2023-04-07 16:24:18 +02:00
Robert Hensing 9693076715 Move initLibStore() immediately after initLibUtil()
Part of an effort to make it easier to initialize the right things,
by moving code into the appropriate libraries.

The goal of this reordering is to make initLibStore self-sufficient
in a following commit.
2023-04-07 16:24:18 +02:00
Robert Hensing a692c43729 Move loadConfFile() to initLibStore
Part of an effort to make it easier to initialize the right things,
by moving code into the appropriate libraries.

Using libstore without loading the config file is risky, as sqlite
may then be misconfigured. See https://github.com/cachix/cachix/issues/475
2023-04-07 16:24:18 +02:00
Robert Hensing 6e0b7109ab Move OpenSSL init to initLibUtil
Part of an effort to make it easier to initialize the right things,
by moving code into the appropriate libraries.
2023-04-07 16:24:18 +02:00
John Ericson 0746951be1
Finish converting existing comments for internal API docs (#8146)
* Finish converting existing comments for internal API docs

99% of this was just reformatting existing comments. Only two exceptions:

- Expanded upon `BuildResult::status` compat note

- Split up file-level `symbol-table.hh` doc comments to get
  per-definition docs

Also fixed a few whitespace goofs, turning leading tabs to spaces and
removing trailing spaces.

Picking up from #8133

* Fix two things from comments

* Use triple-backtick not indent for `dumpPath`

* Convert GNU-style `\`..'` quotes to markdown style in API docs

This will render correctly.
2023-04-07 13:55:28 +00:00