From f68947dd14ce99da0b6862f0f93db692227f5380 Mon Sep 17 00:00:00 2001 From: Matt Sturgeon Date: Tue, 2 Dec 2025 23:03:52 +0000 Subject: [PATCH] modules/extraFiles: refactor to use symlinks and support directories Instead of copying source files to the target, use a symlink. This reduces nix store redundancy and enables using entire directories as sources. To support this, additional validation is done on file targets to prevent unexpected conflicts. --- modules/top-level/files/default.nix | 88 ++++++-- tests/test-sources/modules/extra-files.nix | 234 ++++++++++++++++++++- 2 files changed, 292 insertions(+), 30 deletions(-) diff --git a/modules/top-level/files/default.nix b/modules/top-level/files/default.nix index f2fc052f58..ac7e2e7dc3 100644 --- a/modules/top-level/files/default.nix +++ b/modules/top-level/files/default.nix @@ -56,6 +56,19 @@ in config = let extraFiles = lib.filter (file: file.enable) (lib.attrValues config.extraFiles); + targets = lib.pipe extraFiles [ + (builtins.groupBy (entry: entry.target)) + # Explicitly copy to store using "${}" + (lib.mapAttrs (_: map (entry: "${entry.finalSource}"))) + ]; + prefixConflicts = lib.optionals (targets != { }) ( + let + names = lib.attrNames targets; + pairs = lib.zipLists names (lib.tail names); + in + lib.filter ({ fst, snd }: lib.hasPrefix "${fst}/" snd) pairs + ); + concatFilesOption = attr: lib.flatten (lib.mapAttrsToList (_: builtins.getAttr attr) config.files); in { @@ -63,7 +76,15 @@ in extraPlugins = concatFilesOption "extraPlugins"; extraPackages = concatFilesOption "extraPackages"; warnings = concatFilesOption "warnings"; - assertions = concatFilesOption "assertions"; + assertions = + concatFilesOption "assertions" + ++ lib.nixvim.mkAssertions "extraFiles" { + assertion = prefixConflicts == [ ]; + message = '' + Conflicting target prefixes: + ${lib.concatMapStringsSep "\n" ({ fst, snd }: " - ${fst} ↔ ${snd}") prefixConflicts} + ''; + }; # Add files to extraFiles extraFiles = lib.mkDerivedConfig options.files ( @@ -76,28 +97,53 @@ in ); # A directory with all the files in it - # Implementation based on NixOS's /etc module - build.extraFiles = pkgs.runCommandLocal "nvim-config" { passthru.vimPlugin = true; } '' - set -euo pipefail + # Implementation inspired by NixOS's /etc module + build.extraFiles = + pkgs.runCommandLocal "nvim-config" + { + __structuredAttrs = true; + nativeBuildInputs = [ pkgs.jq ]; + inherit targets; + passthru.vimPlugin = true; + } + '' + set -euo pipefail + + # Ensure $out is created, in case there are no targets defined + mkdir -p "$out" + + # Extract targets into a dedicated file, + # to avoid reading all of attrs.json multiple times + jq .targets "$NIX_ATTRS_JSON_FILE" > targets.json + + # Iterate target keys and read sources into a bash array + jq --raw-output 'keys[]' targets.json | + while IFS= read -r target; do + # read the list of source strings for this target into a Bash array + mapfile -t sources < <( + jq --raw-output --arg target "$target" '.[$target][]' targets.json + ) - makeEntry() { - src="$1" - target="$2" - mkdir -p "$out/$(dirname "$target")" - cp "$src" "$out/$target" - } + # If there are multiple sources, ensure all are identical + if (( ''${#sources[@]} > 1 )); then + base="''${sources[0]}" + for src in "''${sources[@]:1}"; do + # Optimistically check store-path equality, then fallback to diff + if [ "$src" != "$base" ] && ! diff -q "$base" "$src" >/dev/null + then + echo "error: target '$target' defined multiple times with different sources:" >&2 + printf ' %s\n' "''${sources[@]}" >&2 + exit 1 + fi + done + fi - mkdir -p "$out" - ${lib.concatMapStringsSep "\n" ( - { target, finalSource, ... }: - lib.escapeShellArgs [ - "makeEntry" - # Force local source paths to be added to the store - "${finalSource}" - target - ] - ) extraFiles} - ''; + # Install target symlink to the canonical source + dest="$out/$target" + mkdir -p "$(dirname "$dest")" + ln -s "''${sources[0]}" "$dest" + done + ''; # Never combine user files with the rest of the plugins performance.combinePlugins.standalonePlugins = [ config.build.extraFiles ]; diff --git a/tests/test-sources/modules/extra-files.nix b/tests/test-sources/modules/extra-files.nix index ba6a645a63..803036d407 100644 --- a/tests/test-sources/modules/extra-files.nix +++ b/tests/test-sources/modules/extra-files.nix @@ -1,15 +1,231 @@ +let + checkExtraFiles = + pkgs: + { + name, + extraFilesDir, + expectedTargets, + }: + pkgs.runCommand name + { + inherit extraFilesDir expectedTargets; + __structuredAttrs = true; + } + '' + set -euo pipefail + + for target in "''${!expectedTargets[@]}"; do + file="$extraFilesDir/$target" + expected="''${expectedTargets["$target"]}" + + if [ ! -e "$file" ]; then + echo "Missing target: $file" + exit 1 + fi + + if [ -n "$expected" ] && ! grep -qF "$expected" "$file"; then + echo "Content mismatch: $file" + exit 1 + fi + done + + touch $out + ''; +in { + # Example query = { + extraFiles."queries/lua/injections.scm".text = '' + ;; extends + ''; + }; + + empty = { + extraFiles = { }; + }; + + happy-path = + { config, pkgs, ... }: + { + extraFiles = { + # Basic tests + "testing/test-case.nix".source = ./extra-files.nix; + "testing/123.txt".text = '' + One + Two + Three + ''; + + # Multiple files in the same directory + "foo/a".text = "A"; + "foo/b".text = "B"; + "foo/c".text = "C"; + + # Nested deep path + "a/b/c/d/e.txt".text = "deep"; + + # Directory source + "beta/dir".source = pkgs.emptyDirectory; + + # Mixed: source file + hello source + "hello/source".source = pkgs.hello; + }; + test.runNvim = false; + test.extraInputs = [ + (checkExtraFiles pkgs { + name = "check-happy-path"; + extraFilesDir = config.build.extraFiles; + expectedTargets = { + "testing/test-case.nix" = null; + "testing/123.txt" = "One\nTwo\nThree"; + "foo/a" = "A"; + "foo/b" = "B"; + "foo/c" = "C"; + "a/b/c/d/e.txt" = "deep"; + "beta/dir" = null; + "hello/source" = null; + }; + }) + ]; + }; + + # Special content edge cases + special-files = + { config, pkgs, ... }: + { + extraFiles = { + "empty.txt".text = ""; + "whitespace.txt".text = " \n\t "; + "файл.txt".text = "unicode"; + "目录/文件.txt".text = "chinese"; + }; + test.runNvim = false; + test.extraInputs = [ + (checkExtraFiles pkgs { + name = "check-special-files"; + extraFilesDir = config.build.extraFiles; + expectedTargets = { + "empty.txt" = ""; + "whitespace.txt" = " "; + "файл.txt" = "unicode"; + "目录/文件.txt" = "chinese"; + }; + }) + ]; + }; + + identical-duplicates = + { config, pkgs, ... }: + { + extraFiles.a = { + target = "hello.txt"; + text = "hello"; + }; + extraFiles.b = { + target = "hello.txt"; + text = "hello"; + }; + extraFiles.c = { + target = "empty.txt"; + source = pkgs.emptyFile; + }; + extraFiles.d = { + target = "empty.txt"; + source = pkgs.emptyFile; + }; + extraFiles.e = { + target = "emptyDir"; + source = pkgs.emptyDirectory; + }; + extraFiles.f = { + target = "emptyDir"; + source = pkgs.emptyDirectory; + }; + test.buildNixvim = false; + test.extraInputs = [ + (checkExtraFiles pkgs { + name = "check-identical-duplicates"; + extraFilesDir = config.build.extraFiles; + expectedTargets = { + "hello.txt" = "hello"; + "empty.txt" = null; + "emptyDir" = null; + }; + }) + ]; + }; + + duplicate-mismatch = + { config, pkgs, ... }: + { + extraFiles.a = { + target = "conflict.txt"; + text = "abc"; + }; + extraFiles.b = { + target = "conflict.txt"; + text = "xyz"; + }; + test.buildNixvim = false; + test.extraInputs = [ + (pkgs.runCommand "expect-duplicate-failure" + { failure = pkgs.testers.testBuildFailure config.build.extraFiles; } + '' + exit_code=$(cat "$failure/testBuildFailure.exit") + (( exit_code == 1 )) || { + echo "Expected exit code 1" + exit 1 + } + grep -q "target 'conflict.txt' defined multiple times with different sources:" "$failure/testBuildFailure.log" + drv_count=$(grep -c '/nix/store/.*-nvim-.' "$failure/testBuildFailure.log") + (( drv_count == 2 )) || { + echo "Expected 2 store path matches, got $drv_count" + exit 1 + } + touch $out + '' + ) + ]; + }; + + prefix-conflicts = { extraFiles = { - "testing/test-case.nix".source = ./extra-files.nix; - "testing/123.txt".text = '' - One - Two - Three - ''; - "queries/lua/injections.scm".text = '' - ;; extends - ''; + # 2-level conflict + "foo".text = "root"; + "foo/bar.txt".text = "child"; + + # 3-level conflict + "baz".text = "parent"; + "baz/qux".text = "child"; + "baz/qux/quux.txt".text = "grandchild"; + + # 5-level conflict + "a".text = "1"; + "a/b".text = "2"; + "a/b/c".text = "3"; + "a/b/c/d".text = "4"; + "a/b/c/d/e.txt".text = "5"; + + # Non-conflicting paths + "abc".text = "solo"; + "def/ghi.txt".text = "leaf"; + "solo".text = "single"; + "x/y.txt".text = "leaf"; }; + + test.assertions = expect: [ + (expect "count" 1) + (expect "anyExact" '' + Nixvim (extraFiles): Conflicting target prefixes: + - a ↔ a/b + - a/b ↔ a/b/c + - a/b/c ↔ a/b/c/d + - a/b/c/d ↔ a/b/c/d/e.txt + - baz ↔ baz/qux + - baz/qux ↔ baz/qux/quux.txt + - foo ↔ foo/bar.txt'') + ]; + + test.buildNixvim = false; }; }