Replaces these functions with corresponding functions from Abseil,
namely absl::StripAsciiWhitespace and absl::SimpleAtoi.
In the course of doing this some minor things I encountered along the
way were also refactored.
This also changes the signatures of the various custom readFile
functions to use absl::string_view types.
It is considered bad form to use things from includes in headers, as
these directives propagate to everywhere else and can make it
confusing.
types.hh (which is includes almost literally everywhere) had some of
these directives, which this commit removes.
Suppose I have a path /nix/store/[hash]-[name]/a/a/a/a/a/[...]/a,
long enough that everything after "/nix/store/" is longer than 4096
(MAX_PATH) bytes.
Nix will happily allow such a path to be inserted into the store,
because it doesn't look at all the nested structure. It just cares
about the /nix/store/[hash]-[name] part. But, when the path is deleted,
we encounter a problem. Nix will move the path to /nix/store/trash, but
then when it's trying to recursively delete the trash directory, it will
at some point try to unlink
/nix/store/trash/[hash]-[name]/a/a/a/a/a/[...]/a. This will fail,
because the path is too long. After this has failed, any store deletion
operation will never work again, because Nix needs to delete the trash
directory before recreating it to move new things to it. (I assume this
is because otherwise a path being deleted could already exist in the
trash, and then moving it would fail.)
This means that if I can trick somebody into just fetching a tarball
containing a path of the right length, they won't be able to delete
store paths or garbage collect ever again, until the offending path is
manually removed from /nix/store/trash. (And even fixing this manually
is quite difficult if you don't understand the issue, because the
absolute path that Nix says it failed to remove is also too long for
rm(1).)
This patch fixes the issue by making Nix's recursive delete operation
use unlinkat(2). This function takes a relative path and a directory
file descriptor. We ensure that the relative path is always just the
name of the directory entry, and therefore its length will never exceed
255 bytes. This means that it will never even come close to AX_PATH,
and Nix will therefore be able to handle removing arbitrarily deep
directory hierachies.
Since the directory file descriptor is used for recursion after being
used in readDirectory, I made a variant of readDirectory that takes an
already open directory stream, to avoid the directory being opened
multiple times. As we have seen from this issue, the less we have to
interact with paths, the better, and so it's good to reuse file
descriptors where possible.
I left _deletePath as succeeding even if the parent directory doesn't
exist, even though that feels wrong to me, because without that early
return, the linux-sandbox test failed.
Reported-by: Alyssa Ross <hi@alyssa.is>
Thanks-to: Puck Meerburg <puck@puckipedia.com>
Tested-by: Puck Meerburg <puck@puckipedia.com>
Reviewed-by: Puck Meerburg <puck@puckipedia.com>
(cherry picked from commit c05e20daa1abb3446e378331697938b78af2b3d7)
Replaces the previous implementations which performed sorting with one
that instead walks through the map (which is already sorted) and
yields values from it.
This fixes a handful of language tests because the previous
implementation did not actually yield useful values on the new implementation.
In the change to the backing structure of attribute sets, the
requirement to manually balance the capacity of the structure went
away.
This is a) because Abseil's data structures manage this on their own,
and b) because the new Bindings class is allocated using `new (GC)`
rather than writing into a predefined memory area.
As part of this change functions related to the capacity were
deprecated and set to 0 values, which in turn caused the creation of
new attribute sets to return the same (mutable!) default value in
various cases, leading to "side effects" that caused evaluation
failures.
FWIW, I'm not sure if this optimisation had noticeable performance
impact, but while untangling libexpr it definitely doesn't help trying
to follow what it's doing - so bye, bye!
This wrapper derivation (which assumes that the depot is available at
~/depot) can be used to actually get clangd working with
//third_party/nix.
In my setup I can launch this with M-x eglot, followed by
env
CLANGD_FLAGS='--compile-commands-dir=/home/tazjin/projects/nix-build'
nix-shell -A third_party.nix --run 'nix-clangd' /home/tazjin/depot
This is closer to bug-for-bug compatibility with the previous version,
which would put new elements at the end of the array and (due to the
linear scan) return previous ones.
The new attribute set API uses the iterators of the btree_map
directly. This requires changes in various files because the internals
of libexpr are very entangled.
This code runs and compiles, but there is a bug causing empty
attribute sets to be assigned incorrectly.
Instead of doing some sort of inline merge-sort of the two attribute
sets, use the attribute sets merge function.
This commit alone does not build and is not supposed to.
This is the first step towards replacing the implementation of
attribute sets with an absl::btree_map.
Currently many access are done using array offsets and pointer
arithmetic, so this change is currently causing Nix to fail in various
ways.
Replaces most uses of `string` with `std::string`.
This came up because I removed the "types.hh" import from
"symbol-table.hh", which percolated through a bunch of files where
`string` was suddenly no longer defined ... *sigh*
This replaces the previous use of std::unordered_set with
absl::node_hash_set.
This type was chosen because the current implementation requires
pointer stability.
This does not yet touch the 'Attr' struct.
As a bonus, the implementation of the SymbolTable struct is now
consolidated into a single header/implementation file pair.
Meson is unable to use CMake in Nix to determine the internal
structure of the Abseil libraries.
This commit adds an explicit list of most of the Abseil targets that
are relevant (so far) and bundles them into a list that is linked
together.
cmake automatically runs a configure hook which breaks the build,
since this isn't actually a cmake project. This hook is now disabled.
Additionally Abseil's sources are linked to an absolute derivation
path when the build launches, as opposed to the relative path used for
development builds.
This applies the modernization fixes listed here:
https://clang.llvm.org/extra/clang-tidy/checks/list.html
The 'modernize-use-trailing-return-type' fix was excluded due to my
personal preference (more specifically, I think the 'auto' keyword is
misleading in that position).
This last change set was generated by a full clang-tidy run (including
compilation):
clang-tidy -p ~/projects/nix-build/ \
-checks=-*,readability-braces-around-statements -fix src/*/*.cc
Actually running clang-tidy requires some massaging to make it play
nice with Nix + meson, I'll be adding a wrapper or something for that soon.
These were not caught by the previous clang-tidy invocation, but were
instead sorted out using amber[0] as such:
ambr --regex 'for (\(.+\))\s([a-z].*;)' 'for $1 { $2 }'
[0]: https://github.com/dalance/amber
These were not caught by the previous clang-tidy invocation, but were
instead sorted out using amber[0] as such:
ambr --regex 'if (\(.+\))\s([a-z].*;)' 'if $1 { $2 }'
[0]: https://github.com/dalance/amber
Previously these structs were declared anonymously inside of the -
anonymous - union. This is not actually supported by the C++ standard,
but is merely a compiler-specific extension.
Unfortunately untangling this required a forward-declaration of the
Value type.
This change was generated with:
fd -e cc -e hh | xargs -I{} clang-tidy {} -p ~/projects/nix-build/ \
--checks='-*,readability-braces-around-statements' --fix \
-fix-errors
Some manual fixes were applied because some convoluted unbraced
statements couldn't be untangled by clang-tidy.
This commit still includes invalid files, but I decided to clean them
up in a subsequent commit so that it becomes more obvious where
clang-tidy failed. Maybe this will allow for a bug-report to
clang-tidy.
Changes the configuration to regroup all includes. The include groups
will be (in this order):
1. (in .cc): Include of the corresponding header
2. Includes of C++ standard library headers
3. Includes of other external headers
4. Includes of local headers
Removes the activity transfer that was previously nulled out from the
daemon protocol completely.
This might actually break Nix completely, I haven't tried yet, but
that's fine because this will be replaced with gRPC.
The progress bar has lots of complexity for little benefit. The
previous activity tracking stuff has been deleted as part of the
logging refactoring and I am not going to implement support for this
again for now.
1. First of all, this doesn't work in nixpkgs. Per [1], gcc ignores `-L`
for purposes of `--print-file-dirs`, which breaks horribly on linux. But
if we don't pass extra dirs, meosn first just tries `-l...`, which does
work.
2. Even if it did work, `libdir` means where we are installing libs, not
where libs are expected to be found. Those are not necessarily the
same (again, nixpkgs), and even when they are and non-standard, it is
better to use DESTDIR or have a modified toolchain.
[1]: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87758
(cherry picked from commit a142164e746644e20f66908c156ca913bef4664f)
