diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 91928d07..4615ba14 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -51,26 +51,33 @@ When you provide an option to `enable` or `disable` something, you should call i This prevents people from needing to look it up to use it, and prevents contributors from having to think too hard about which to call it. -When you provide a `wlib.types.file` option, you should name it the actual filename, especially if there are multiple, but `configFile` is also OK, especially if it is unambiguous. - -Keep in mind that even if you do not choose to use `wlib.types.file`, the user can usually still override the option that you set to provide the generated path if needed. - -However, this makes the user of your module search for it, and in some situations, such as when your module is adding stuff to `list` or `DAL` type options, this can be slightly harder to override later. - -So making use of the `wlib.types.file` type or giving some other method of overriding the filepath when providing a file is generally recommended for this reason. - - Placeholders When you generate a file, it is generally better to do so as a string, and create it using the `constructFiles` option. -This is because, this will make placeholders such as `${placeholder "out"}` work consistently across all your options. +This is because, this will make placeholders such as `${placeholder "out"}` work consistently across all your options, +allowing them to all point to the final wrapper derivation rather than several intermediate ones. -What this allows you to do, is manually build files later using `buildCommand` option or a stdenv phase, and then refer to that created file within your settings! +What this allows you to do, is manually build files via another option like `constructFiles`, and then refer to that created file within your settings! Making placeholders work in your module makes your modules generally more easily extensible, and is preferred when it is possible to generate a usable string. It works by using `drv.passAsFile` and making a derivation attribute with the file contents, which is copied into place. +- `wlib.types.file` + +When you provide a `wlib.types.file` option, you should name it the actual filename, especially if there are multiple, but `configFile` is also OK, especially if it is unambiguous. + +Keep in mind that even if you do not choose to use `wlib.types.file`, the user can usually still override the option that you set to provide the generated path if needed. + +So using something like `wlib.types.file` is only truly important when the file you are making an option for is passed to a list-style option, but may still be nice more generally. + +However: + +For the same reason that we use `constructFiles` to build files, using `wlib.types.file` without overriding its default `path` value is discouraged in modules to be submitted to this repository. + +It builds the file using `pkgs.writeText` by default, and because this creates an intermediate derivation, it means placeholders in that file will not point to the final wrapper derivation. + Example: ```nix @@ -95,8 +102,10 @@ Example: configFile = lib.mkOption { type = wlib.types.file pkgs; default = { - path = config.constructFiles.gitconfig.path; # <- we can refer to the placeholder of our constructed file! content = ""; + # we should override the default path value for wlib.types.file if we can to make placeholders work + # to do that, we can refer to the placeholder of our constructed file! + path = config.constructFiles.gitconfig.path; }; description = "Generated git configuration file."; }; @@ -138,7 +147,8 @@ or You may also include a `check.nix` file in your module's directory. -It will be provided with the flake `self` value and `pkgs` +It will be called via `pkgs.callPackage`, provided with the flake `self` value. +(i.e. `pkgs.callPackage your_check.nix { inherit self; }`) It should build a derivation which tests the wrapper derivation as best you can. @@ -153,6 +163,7 @@ Example: ```nix { pkgs, + runCommand, self, }: let @@ -165,9 +176,8 @@ let }; }; }; - in -pkgs.runCommand "git-test" { } '' +runCommand "git-test" { } '' "${gitWrapped}/bin/git" config user.name | grep -q "Test User" "${gitWrapped}/bin/git" config user.email | grep -q "test@example.com" touch $out @@ -186,6 +196,8 @@ else null ``` +If you are writing a helper module, or something very complex, you may wish to have multiple derivations. Simply return a set of them instead. + # Commit Messages Changes to wrapper modules should be titled `(wrapperModules.): some description`. diff --git a/ci/checks/no-module-prefix-in-checks.nix b/ci/checks/no-module-prefix-in-checks.nix deleted file mode 100644 index 3311e10d..00000000 --- a/ci/checks/no-module-prefix-in-checks.nix +++ /dev/null @@ -1,50 +0,0 @@ -{ - pkgs, - self, -}: - -let - # Get all check files in checks/ directory - checkFiles = builtins.readDir ./.; - - # Filter for .nix files that start with "module-" - checksWithPrefix = - prefix: - pkgs.lib.filter (name: pkgs.lib.hasPrefix prefix name && pkgs.lib.hasSuffix ".nix" name) ( - builtins.attrNames checkFiles - ); - - invalidChecksMod = checksWithPrefix "module-"; - invalidChecksWrap = checksWithPrefix "wrapperModule-"; - -in -pkgs.runCommand "no-module-prefix-in-checks-test" { } '' - echo "Checking that no checks in ci/checks/ directory start with 'module-' or 'wrapperModule-'..." - - ${ - if invalidChecksMod != [ ] then - '' - echo "FAIL: The following checks have invalid 'module-' prefix:" - ${pkgs.lib.concatMapStringsSep "\n" (name: ''echo " - ${name}"'') invalidChecksMod} - echo "" - echo "Checks starting with 'module-' are reserved for module-specific checks (modules/*/check.nix)." - echo "Please rename these checks to avoid namespace collision." - exit 1 - '' - else if invalidChecksWrap != [ ] then - '' - echo "FAIL: The following checks have invalid 'wrapperModule-' prefix:" - ${pkgs.lib.concatMapStringsSep "\n" (name: ''echo " - ${name}"'') invalidChecksWrap} - echo "" - echo "Checks starting with 'wrapperModule-' are reserved for wrapperModule-specific checks (wrapperModules/*/*/check.nix)." - echo "Please rename these checks to avoid namespace collision." - exit 1 - '' - else - '' - echo "SUCCESS: No checks start with 'module-' or 'wrapperModule-' prefixes" - '' - } - - touch $out -'' diff --git a/ci/flake.nix b/ci/flake.nix index b0e585cc..e0d45780 100644 --- a/ci/flake.nix +++ b/ci/flake.nix @@ -21,38 +21,46 @@ config.allowUnfree = true; }; - # Load checks from checks/ directory - checkFiles = builtins.readDir ./checks; - importCheck = name: { - name = lib.removeSuffix ".nix" name; - value = import (./checks + "/${name}") { - inherit pkgs; - self = self; - }; - }; - checksFromDir = builtins.listToAttrs ( - map importCheck (builtins.filter (name: lib.hasSuffix ".nix" name) (builtins.attrNames checkFiles)) - ); + # Load checks from ci/checks/ directory + coreAndCiChecks = lib.pipe ./checks [ + builtins.readDir + builtins.attrNames + (builtins.filter (name: lib.hasSuffix ".nix" name)) + (map (n: { + name = lib.removeSuffix ".nix" n; + value = ./checks + "/${n}"; + })) + builtins.listToAttrs + ]; - importModuleCheck = prefix: name: value: { - name = "${prefix}-${name}"; - value = import value { - inherit pkgs; - self = self; - }; - }; - checksFromModules = builtins.listToAttrs ( - builtins.filter (v: v.value or null != null) ( - lib.mapAttrsToList (importModuleCheck "module") (wlib.checks.helper or { }) - ) - ); - checksFromWrapperModules = builtins.listToAttrs ( - builtins.filter (v: v.value or null != null) ( - lib.mapAttrsToList (importModuleCheck "wrapperModule") (wlib.checks.wrapper or { }) - ) - ); + checksFrom = + prefix: attrset: + let + importModuleCheck = + name: value: + let + helper = prefix: name: value: { + name = "${prefix}-${name}"; + inherit value; + }; + result = pkgs.callPackage value { inherit self; }; + in + if isNull result then + [ ] + else if result ? outPath then + [ (helper prefix name result) ] + else + lib.mapAttrsToList (helper "${prefix}-${name}") (lib.filterAttrs (_: v: v ? outPath) result); + in + lib.pipe attrset [ + (lib.mapAttrsToList importModuleCheck) + builtins.concatLists + builtins.listToAttrs + ]; in - checksFromDir // checksFromModules // checksFromWrapperModules + checksFrom "wlib" coreAndCiChecks + // checksFrom "module" (wlib.checks.helper or { }) + // checksFrom "wrapperModule" (wlib.checks.wrapper or { }) ); formatter = forAllSystems ( system: (wlib-flake (import nixpkgs { inherit system; })).formatter.${system} diff --git a/modules/makeWrapper/check.nix b/modules/makeWrapper/check.nix new file mode 100644 index 00000000..18541618 --- /dev/null +++ b/modules/makeWrapper/check.nix @@ -0,0 +1,16 @@ +{ + callPackage, + lib, + self, + ... +}: +lib.pipe ./checks [ + builtins.readDir + (lib.filterAttrs (name: type: type == "regular" && lib.hasSuffix ".nix" name)) + (lib.mapAttrs' ( + name: _: + lib.nameValuePair (lib.removeSuffix ".nix" name) ( + callPackage (./checks + "/${name}") { inherit self; } + ) + )) +] diff --git a/ci/checks/args-direct.nix b/modules/makeWrapper/checks/args-direct.nix similarity index 100% rename from ci/checks/args-direct.nix rename to modules/makeWrapper/checks/args-direct.nix diff --git a/ci/checks/env-null.nix b/modules/makeWrapper/checks/env-null.nix similarity index 100% rename from ci/checks/env-null.nix rename to modules/makeWrapper/checks/env-null.nix diff --git a/ci/checks/flags-equals-separator.nix b/modules/makeWrapper/checks/flags-equals-separator.nix similarity index 100% rename from ci/checks/flags-equals-separator.nix rename to modules/makeWrapper/checks/flags-equals-separator.nix diff --git a/ci/checks/flags-list.nix b/modules/makeWrapper/checks/flags-list.nix similarity index 100% rename from ci/checks/flags-list.nix rename to modules/makeWrapper/checks/flags-list.nix diff --git a/ci/checks/flags-null-false.nix b/modules/makeWrapper/checks/flags-null-false.nix similarity index 100% rename from ci/checks/flags-null-false.nix rename to modules/makeWrapper/checks/flags-null-false.nix diff --git a/ci/checks/flags-space-separator.nix b/modules/makeWrapper/checks/flags-space-separator.nix similarity index 100% rename from ci/checks/flags-space-separator.nix rename to modules/makeWrapper/checks/flags-space-separator.nix diff --git a/ci/checks/flags-module.nix b/modules/makeWrapper/checks/flags.nix similarity index 100% rename from ci/checks/flags-module.nix rename to modules/makeWrapper/checks/flags.nix diff --git a/modules/symlinkScript/check.nix b/modules/symlinkScript/check.nix new file mode 100644 index 00000000..18541618 --- /dev/null +++ b/modules/symlinkScript/check.nix @@ -0,0 +1,16 @@ +{ + callPackage, + lib, + self, + ... +}: +lib.pipe ./checks [ + builtins.readDir + (lib.filterAttrs (name: type: type == "regular" && lib.hasSuffix ".nix" name)) + (lib.mapAttrs' ( + name: _: + lib.nameValuePair (lib.removeSuffix ".nix" name) ( + callPackage (./checks + "/${name}") { inherit self; } + ) + )) +] diff --git a/ci/checks/filesToExclude-glob.nix b/modules/symlinkScript/checks/filesToExclude-glob.nix similarity index 100% rename from ci/checks/filesToExclude-glob.nix rename to modules/symlinkScript/checks/filesToExclude-glob.nix diff --git a/ci/checks/filesToExclude-module.nix b/modules/symlinkScript/checks/filesToExclude-more.nix similarity index 100% rename from ci/checks/filesToExclude-module.nix rename to modules/symlinkScript/checks/filesToExclude-more.nix diff --git a/ci/checks/filesToExclude.nix b/modules/symlinkScript/checks/filesToExclude.nix similarity index 100% rename from ci/checks/filesToExclude.nix rename to modules/symlinkScript/checks/filesToExclude.nix diff --git a/ci/checks/filesToPatch.nix b/modules/symlinkScript/checks/filesToPatch.nix similarity index 100% rename from ci/checks/filesToPatch.nix rename to modules/symlinkScript/checks/filesToPatch.nix