The full clang-tidy check suite is *very* slow. This reduced list is
some kind of middle-ground between running all checks, and having a
useful developer experience.
Crucially most of the static analyzer checks (except for the ones
related to security issues) have been disabled.
We should look into running the full suite in CI only.
Change-Id: I02b96ad3b4d1a43bd6aa90ffdcba800dad966714
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1422
Tested-by: BuildkiteCI
Reviewed-by: glittershark <grfn@gws.fyi>
Replace the custom, rather questionable base64 implementation with
absl::Base64{Une,E}scape. To make sure that the custom implementation
was doing the same thing I've also added a test covering
nix::Hash::to_string, which was one function that used it - the test
passed prior to the replacement, and continued to pass afterwards.
The previous base64Decode function threw an exception on failure - to
avoid going too far down the rabbit hole I've replicated that
functionality at all call sites, but this should be replaced with more
sensible error handling such as StatusOr eventually.
Also, before this change:
❯ nix eval -f . users.tazjin.emacs.outPath
"/nix/store/g6ri2q8nra96ix20bcsc734r1yyaylb1-tazjins-emacs"
And after:
❯ ./result/bin/nix eval -f . users.tazjin.emacs.outPath
"/nix/store/g6ri2q8nra96ix20bcsc734r1yyaylb1-tazjins-emacs"
Change-Id: Id292ffbb82fe808f3f1b34670afbe7b8c13ad615
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1385
Reviewed-by: tazjin <mail@tazj.in>
Tested-by: BuildkiteCI
CMake likes to remember that we overrode it with an empty string and not use the new env var.
Change-Id: I16587f27750c9ebd4f65349bb59b37e0f8117f18
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1406
Tested-by: BuildkiteCI
Reviewed-by: glittershark <grfn@gws.fyi>
The use of vfork() in Nix is entirely illegal. Quote:
If the process created by vfork() returns from the function in which vfork() was
called, or calls any other function before successfully calling _exit() or
one of the exec*() family of functions, the behavior is undefined.
-- Linux man-pages, release 5.05
Add a TODO to use the higher-performance variants of clone() on Linux when it
is available.
Change-Id: I42370e1568ad6e2d00d70d0b66c8aded8f1288bb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1418
Tested-by: BuildkiteCI
Reviewed-by: tazjin <mail@tazj.in>
Reviewed-by: Alyssa Ross <hi@alyssa.is>
nixutil depends on bzip2, lzma, boost::context, brotli{enc,dec}, and
openssl, but wasn't directly linking to them. This was causing linker
errors in a test that only depended on nixutil.
Change-Id: I60e77ea7b18b08e2946fcf9176ae0f355cd71844
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1384
Tested-by: BuildkiteCI
Reviewed-by: kanepyork <rikingcoding@gmail.com>
We need to -isystem the libcxx header files in order for clang-tidy to ignore them, as the Nix clang toolchain isn't doing that automatically.
Change-Id: I05b9e9bd522de4c0e2ad543214f6bf6ab66a306b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1359
Reviewed-by: glittershark <grfn@gws.fyi>
Tested-by: BuildkiteCI
Branches are grouped into their own section to make the "this commit"
option visually distinct.
Adding this option will result in two options being marked as selected
if a branch has the same name as a commit oid. But that would cause
all sorts of other problems anyway (attempting to switch to the branch
would actually give you the commit, etc.), so let's not worry about
that.
A "permalink" link on the blob view next to the "plain" link would
probably be more discoverable, but that would only work for the blob
view. The switch UI is visible everywhere.
This patch is in use already at <https://git.qyliss.net/> and
<https://spectrum-os.org/git/>.
Signed-off-by: Alyssa Ross <hi@alyssa.is>
Message-Id: <20200723204820.16776-1-hi@alyssa.is>
Cc: Profpatsch <mail@profpatsch.de>
Change-Id: I7e88d1231dd402e0ad764e16b28e9a51964c6293
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1382
Tested-by: BuildkiteCI
Reviewed-by: tazjin <mail@tazj.in>
This formerly controlled access to builtins.exec and
builtins.importNative, but both of those are gone now, so there's no
need for this option any more.
Change-Id: I6850cbd6be264fbfb1b209a60026dadbd0ba1232
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1341
Reviewed-by: glittershark <grfn@gws.fyi>
Tested-by: BuildkiteCI
This is the shared object equivalent of builtins.exec, or a plugins
equivalent accessible from the Nix language. Either way, since we
don't have builtins.exec or plugins any more, I think it makes sense
to remove this builtin.
This will also allow us to drop the
allow-unsafe-native-code-during-evaluation option, which formerly
controlled whether builtins.exec and builtins.importNative were
enabled.
Cc: Griffin Smith <grfn@gws.fyi>
Cc: Profpatsch <mail@profpatsch.de>
Change-Id: I8993a8a79d559c102647308a2684c089bbc06713
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1340
Reviewed-by: glittershark <grfn@gws.fyi>
Reviewed-by: Profpatsch <mail@profpatsch.de>
Tested-by: BuildkiteCI
The SQL schemas are included as string constants which are
concatenated into a header file. In the previous Makefiles, this was
done with envsubst or something - we moved it to CMake.
There was a missing quote around the string to be interpolated, which
meant that CMake interpreted the semicolons as part of its language
syntax and did not emit them.
Change-Id: Ibb4512788b26b53f297db3535094dc0194614446
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1342
Reviewed-by: kanepyork <rikingcoding@gmail.com>
Reviewed-by: glittershark <grfn@gws.fyi>
Tested-by: BuildkiteCI
Add a forward-declared factory function for constructing and returning a
WorkerServiceImpl, for eventual use in the main function for the nix
daemon
Change-Id: I9032d69b6ee3bc3b1f39f3d5d55f951cffad8145
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1293
Tested-by: BuildkiteCI
Reviewed-by: isomer <isomer@tvl.fyi>
Plugins seem to not really be used anywhere (I can find one plugin
that's actually defined, and it doesn't seem very useful, especially
since we got rid of builtins.exec) and their presence is adding
additional complexity and potential sources of bugs to an already
unsteady refactor. At some point we may want to bring back
something *like* plugins, but their design will likely be different and
it will definitely be after we have a functioning Nix again.
Change-Id: I3bc40e55917f70bf260fbc208c1705e2e6a7c626
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1291
Tested-by: BuildkiteCI
Reviewed-by: Alyssa Ross <hi@alyssa.is>
Reviewed-by: isomer <isomer@tvl.fyi>
This compiles under `-Wall -Werror`.
The largest chunk of this change is `final` qualifiers for the various
Nix CLI command structs, which inherit from a Command class that has
more virtual functions than are implemented by each command.
Change-Id: I0925e6e1a39013f026773db5816e4a77d50f3b4a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1294
Tested-by: BuildkiteCI
Reviewed-by: isomer <isomer@tvl.fyi>
Reviewed-by: kanepyork <rikingcoding@gmail.com>
This setting was renamed to api.pure about 300 years ago and caused
warnings to be issued.
Change-Id: If883b0667c3afe67ae3d2a9950a796688cfbea7a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1290
Tested-by: BuildkiteCI
Reviewed-by: glittershark <grfn@gws.fyi>
Removes the verbosity enum, which is no longer actively used anywhere
other than a daemon protocol implementation bit that doesn't actually
work.
Since the verbosity was marked deprecated, this removes one of the
last remaining warnings.
Change-Id: Iaee9d1d6c14b30daac83bb44bcacff32a0e07fb0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1289
Tested-by: BuildkiteCI
Reviewed-by: glittershark <grfn@gws.fyi>
The function is renamed to `SortedByKeys`, which is more descriptive,
and annotated with a comment about what it is used for.
The deprecation warning has been removed because this function is
currently functionally required.
Change-Id: I0ee3a76deff05f366feca9ddac8f38ab34bffbd0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1288
Tested-by: BuildkiteCI
Reviewed-by: glittershark <grfn@gws.fyi>
This is the easiest way to get the checks up and running for now, but
we will probably want to separate out things like this into a separate
build step in the future.
Change-Id: I8e1a1095aef09b1eee97abad5b6240bc64d14b8c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1287
Tested-by: BuildkiteCI
Reviewed-by: glittershark <grfn@gws.fyi>
Enables loading of the expected output of evaluator tests from the
corresponding .exp files, and checks that the output matches.
This again leaves some tests behind in the disabled folder, but we now
have almost the entire suite up and running so I can get around to
cleaning up the disabled ones.
Other note: Some tests had XML output, despite not being related to
XML testing at all - I'm not sure why they chose to do this, but have
converted those test outputs to normal Nix instead.
We have a separate test suite for JSON & XML serialisation already,
which was contributed by andi-.
Change-Id: Id7c42c836edfec4c22db9d893e35489f3e6dd559
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1285
Tested-by: BuildkiteCI
Reviewed-by: isomer <isomer@tvl.fyi>
Reviewed-by: glittershark <grfn@gws.fyi>
Enables the `eval-okay-` test suite, with some caveats:
* The output is not yet checked against the expected value, so the
tests only check that pure evaluation succeeds
* A handful of tests have been disabled as they are causing failures
that may be related to the DummyStore implementation.
Both of these will be addressed in followup commits, but there is
already some value in having the pure evaluation tests pass.
Change-Id: I62236c95ebffb06dc64a45455a8ee04344c400b7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1284
Tested-by: BuildkiteCI
Reviewed-by: isomer <isomer@tvl.fyi>
Reviewed-by: glittershark <grfn@gws.fyi>
Add a set of property tests for the attribute set (Bindings) class
checking that the Merge operation satisfies the monoid laws. This
will hopefully become useful to make sure we're not breaking the
language semantics as we work towards optimizing or replacing the
implementation, but also serves as a test bed for adding
rapidcheck-based property tests to the codebase.
Change-Id: I1b4b7b6503d08d80c1c5a8f9408fd4b787d00e8e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1283
Reviewed-by: isomer <isomer@tvl.fyi>
Tested-by: BuildkiteCI
Pulled from the commit at current master, overridden to build with clang
and enable gtest and gmock integration.
Change-Id: I10008e8c591bd0c7cc26566b2a050ef2a55bb346
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1282
Tested-by: BuildkiteCI
Reviewed-by: tazjin <mail@tazj.in>
Moves a function that is not dependent on the generated code over to
parser.hh. This function also looks like it could be improved, but
that is left as an exercise for the reader.
Code that remains in lexer.l has been reformatted, while we're here.
Change-Id: I9c26bb4eed0772a720d0715029e8bc10ab16ac38
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1279
Tested-by: BuildkiteCI
Reviewed-by: Kane York <rikingcoding@gmail.com>
Additionally, add tests for the macros. A future CL will enable the tests in CI.
Change-Id: Id4445a1aa65bf6751b87606f37654f3fc6d20efc
Tested-By: kanepyork <rikingcoding@gmail.com>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1274
Reviewed-by: tazjin <mail@tazj.in>
Reviewed-by: glittershark <grfn@gws.fyi>
Tested-by: BuildkiteCI
This moves the language test suite into Googletest by constructing
parameterised tests out of the same language snippets used for the
previous lang.sh evaluation.
So far this includes support for about 3/4 of all tests, specifically:
* all parser success/failure tests
* all evaluator failure tests
The evaluator success tests will be implemented in a subsequent commit,
because the output comparison contains a whole bunch of additional
logic that I did not want to cram in here.
Change-Id: Icec9f368366cdbaa53b4c7e4472b8b6e8dd72eba
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1278
Reviewed-by: glittershark <grfn@gws.fyi>
Reviewed-by: isomer <isomer@tvl.fyi>
Tested-by: BuildkiteCI
Unfortunately, to guarantee correct behaviour of some evaluation (!)
tests, addToStore needs to actually check whether passed in source
files exist and fail appropriately.
There is a chance that the dependency on this behaviour is actually a
bug in the upstream test suite, but my attempts at finding out more
about this from, say, the git history have so far been unsuccessful.
Change-Id: I311999ea28fcedf5da13a4e627b1c1c8e4e59cbd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1276
Reviewed-by: isomer <isomer@tvl.fyi>
Reviewed-by: glittershark <grfn@gws.fyi>
Tested-by: BuildkiteCI
These files will be integrated into the evaluator unit tests instead
of running separately via a shell script.
Change-Id: I1d229e73b1d862777f5108c86891689900edefbe
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1275
Tested-by: BuildkiteCI
Reviewed-by: Kane York <rikingcoding@gmail.com>
Reviewed-by: isomer <isomer@tvl.fyi>
These options only apply to nix-shell. The fact that nix-build
previously accepted them was an accident that resulted from the two
programs sharing an implementation.
Change-Id: I0047c98e2096010797316bff3ea4faf722fab86a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1273
Tested-by: BuildkiteCI
Reviewed-by: tazjin <mail@tazj.in>
I missed calling store_->assertStorePath on paths in several daemon
proto handlers, which the previous implementation did.
Change-Id: Ifad6eeb03b5a5babec7b4bcf7aca060813f15bb7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1272
Tested-by: BuildkiteCI
Reviewed-by: tazjin <mail@tazj.in>
I got the message proto wrong on this one as well - it needs both a path
and a signatures.
Change-Id: I9a489b1285bda61c15b2a3b47d9cfc3b50e387da
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1270
Tested-by: BuildkiteCI
Reviewed-by: tazjin <mail@tazj.in>
Implement the proto handler on the server side for
Worker::BuildDerivation. This includes several additions to the proto
which I had missed on the first pass, including the actual proto
definition for the Derivation itself and a few sequence number
reorderings which are fine because this is all provisional and not
deployed yet.
A couple things to note
- I implemented a couple constructors for nix classes that initialize
themselves based on their proto variants, which felt nice and didn't
end up causing any issues.
- I've made the conversions between the enum types in nix and in proto
explicit via switch statements rather than using a static_cast, out of
an abundance of caution that the error would get mismatched in the
future and we'd convert the wrong thing to the wrong thing - this is
verbose, but exceptionally future proof.
Change-Id: Iecf6b88e76bc37e49efa05fd65d6cd0cb0deffed
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1249
Tested-by: BuildkiteCI
Reviewed-by: tazjin <mail@tazj.in>
Reviewed-by: Kane York <rikingcoding@gmail.com>
This silences a -Winconsistent-missing-override
Change-Id: I2492a8ab3c0b0a33a794f012bfee6bbd0c2f0a8e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1271
Reviewed-by: Kane York <rikingcoding@gmail.com>
Tested-by: BuildkiteCI
Interpolating a path into a string will copy the referenced path into
the Nix store, so this got a dependency on all of src. By first
constructing a path to the src/proto directory using the + operator,
and then interpolating it, we limit what is copied to the store, and
therefore what code triggers a nix-proto-srcs rebuild.
Change-Id: I8dd750f6bc5902b74ffb56470bc8a5f2c01c8cf1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1263
Tested-by: BuildkiteCI
Reviewed-by: glittershark <grfn@gws.fyi>
Reviewed-by: tazjin <mail@tazj.in>
This store implementation is required in all unit tests that use the
evaluator.
Change-Id: I1cfe8cecab8722cd66dc803747821a2be2b2619f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1269
Tested-by: BuildkiteCI
Reviewed-by: glittershark <grfn@gws.fyi>
This change does away with the previous special-casing of lists of
certain element sizes, and the use of raw C-style arrays.
Lists are now backed by a std::vector of nix::Value*, which uses the
traceable GC allocator.
This change is unfortunately quite noisy because the accessor methods
were updated/removed accordingly, so all callsites of Nix-related
lists have changed.
For some operations in primops.cc where keeping the previous code
structure would have been more difficult with a "proper" vector, the
implementation has been replaced with std::vector methods. For
example, list concatenation now uses appropriate range inserts.
Anecdotally the performance of this is about equal, to even slightly
better, than the previous implementation.
All language tests pass and the depot paths I've used for testing
still evaluate.
Change-Id: Ib5eca6c0207429cb323a330c838c3a2200b2c693
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1266
Tested-by: BuildkiteCI
Reviewed-by: isomer <isomer@tvl.fyi>
Reviewed-by: Kane York <rikingcoding@gmail.com>
Reviewed-by: glittershark <grfn@gws.fyi>
This has several advantages:
* we can ensure that the vector is traced by the GC
* we don't need to unsafely allocate memory to make an Env
Note that there was previously a check about the size of the
environment, but it's unclear why this was the case (git history
yielded nothing interesting) and it seems to have no effect.
Change-Id: I4998b879a728a6fb68e1bd187c521e2304e5047e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1265
Tested-by: BuildkiteCI
Reviewed-by: isomer <isomer@tvl.fyi>
Reviewed-by: Kane York <rikingcoding@gmail.com>
Reviewed-by: glittershark <grfn@gws.fyi>
This breaks tooling for some people, and we aren't actually using the
git submodules.
Change-Id: I5b4dfd4ad76bf72e9dbc7de29f17f28ebf6b8ba0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1255
Tested-by: BuildkiteCI
Reviewed-by: glittershark <grfn@gws.fyi>
all of the executables that get built during regular development depend
on this being set to a directory that contains the nix directory -
previously we had been doing it manually every time, this automates it.
Change-Id: I4c957c0abf0a92ca7122a47d3b141a8ede280e13
Reviewed-on: https://cl.tvl.fyi/c/depot/+/1258
Tested-by: BuildkiteCI
Reviewed-by: tazjin <mail@tazj.in>
