fix(filegen): Allow using filegen with mixed architectures#39
Conversation
PR SummaryLow Risk Overview
Repo-facing cleanup: Reviewed by Cursor Bugbot for commit 3ebd448. Bugbot is set up for automated code reviews on this repo. Configure here. |
4602c1c to
1273e28
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Clobber false still overwrites
- Replaced the ineffective '--backup=numbered' (which still overwrites the destination) with a guard that skips the install when clobber is false and the target already exists.
Or push these changes by commenting:
@cursor push f297c84c7f
Preview (f297c84c7f)
diff --git a/.gitattributes b/.gitattributes
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,4 +1,3 @@
.pre-commit-config.yaml linguist-generated
.github/workflows/check-pre-commit-hooks.yml linguist-generated
.gitattributes linguist-generated
-.config/filegen-manifest.json linguist-generated
diff --git a/nix/general/default.nix b/nix/general/default.nix
--- a/nix/general/default.nix
+++ b/nix/general/default.nix
@@ -16,10 +16,7 @@
config.perSystem =
{ config, ... }:
lib.mkMerge [
- {
- filegen.settings.clobber-by-default = true;
- githubActions.enable = true;
- }
+ { githubActions.enable = true; }
(lib.mkIf (config.githubActions.workflows != { }) {
filegen.settings.files = lib.mapAttrsToList (workflow: source: {
diff --git a/nix/modules/filegen.nix b/nix/modules/filegen.nix
--- a/nix/modules/filegen.nix
+++ b/nix/modules/filegen.nix
@@ -1,7 +1,6 @@
{ flake-parts-lib, lib, ... }:
let
inherit (lib) types;
- smfh.version = 3;
in
{
options.perSystem = flake-parts-lib.mkPerSystemOption (
@@ -21,10 +20,6 @@
To generate the files, an "app" named `filegen-activate` is created,
which can be executed with `nix run .#filegen-activate`.
- The configuration is as expected by
- [smfh](https://github.com/feel-co/smfh), which is used
- to perform the actual file manipulations.
-
---
Note: This module does *not* attempt to protect against writes to or
@@ -50,13 +45,7 @@
Normally, this should be set to `copy`.
'';
- type = types.enum [
- "copy"
- "symlink"
- "modify"
- "directory"
- "delete"
- ];
+ type = types.enum [ "copy" ];
};
target = lib.mkOption {
@@ -85,20 +74,6 @@
type = types.nullOr types.bool;
};
- ignore-modification = lib.mkOption {
- description = ''
- Whether to skip content integrity checks during activation.
- '';
- type = types.nullOr types.bool;
- };
-
- deactivate = lib.mkOption {
- description = ''
- If enabled, `filegen-deactivate` will ignore this file.
- '';
- type = types.nullOr types.bool;
- };
-
permissions = lib.mkOption {
description = ''
The permissions of the created file.
@@ -115,52 +90,20 @@
# default.
default = "600";
};
-
- # Not implemented:
- #
- # - permissions/uid/gid, as these should not matter for a
- # project repo.
- # - follow_symlinks, since for a git repo we should never
- # want to symlink to absolute paths.
};
}
)
);
};
-
- clobber-by-default = lib.mkOption {
- description = ''
- Whether files should be overwritten by default.
- '';
- type = types.nullOr types.bool;
- };
};
- smfhPackage = lib.mkOption {
+ scripts.activate = lib.mkOption {
description = ''
- The smfh package to use.
+ A script that applies the files configured with `filegen.files`.
'';
- default = pkgs.smfh;
- type = types.package;
+ readOnly = true;
+ type = types.pathInStore;
};
-
- scripts = {
- activate = lib.mkOption {
- description = ''
- A script that applies the files configured with `filegen.files`.
- '';
- readOnly = true;
- type = types.pathInStore;
- };
-
- deactivate = lib.mkOption {
- description = ''
- A script that removes configured by a previous invocation of the activate script.
- '';
- readOnly = true;
- type = types.pathInStore;
- };
- };
};
}
);
@@ -169,9 +112,7 @@
{ pkgs, config, ... }:
let
cfg = config.filegen;
- new-manifest = pkgs.writers.writeJSON "filegen-manifest.json" (
- config.filegen.settings // { inherit (smfh) version; }
- );
+ new-manifest = pkgs.writers.writeJSON "filegen-manifest.json" config.filegen.settings;
in
{
filegen.settings.files = [
@@ -185,7 +126,7 @@
target = ".gitattributes";
source = pkgs.writeTextFile {
name = ".gitattributes";
- text = lib.pipe (cfg.settings.files ++ [ { target = ".config/filegen-manifest.json"; } ]) [
+ text = lib.pipe cfg.settings.files [
(map (file: lib.removePrefix "./" file.target))
(map (target: "${target} linguist-generated"))
lib.concatLines
@@ -198,22 +139,24 @@
activate = pkgs.writers.writeNuBin "filegen-apply-script" ''
cd (git rev-parse --show-toplevel)
- (${lib.getExe cfg.smfhPackage}
- --impure
- diff
- --fallback
- ${new-manifest}
- .config/filegen-manifest.json)
-
- mkdir .config
- cp --preserve [] ${new-manifest} .config/filegen-manifest.json
- chmod 600 .config/filegen-manifest.json
+ for file in (open '${new-manifest}' | $in.files) {
+ match $file.type {
+ "copy" => {
+ if (($file.clobber | default true) or not ($file.target | path exists)) {
+ (${lib.getExe' pkgs.coreutils "install"}
+ -D
+ --compare
+ $'--mode=($file.permissions)'
+ $file.source $file.target)
+ }
+ },
+ _ => {
+ print $"Type '($file.type)' for '($file.source)' is currently unsupported"
+ exit 1
+ }
+ }
+ }
'';
-
- deactivate = pkgs.writers.writeNuBin "filegen-deactivate-script" ''
- cd (git rev-parse --show-toplevel)
- ${lib.getExe cfg.smfhPackage} deactivate .config/filegen-manifest.json
- '';
in
{
filegen-activate = {
@@ -223,14 +166,6 @@
package = activate;
};
};
-
- filegen-deactivate = {
- program = lib.getExe deactivate;
- meta = {
- description = "Uninstall all files previously installed using `filegen-activate`";
- package = deactivate;
- };
- };
};
};
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 1273e28. Configure here.
1273e28 to
ff12bea
Compare
Since the filegen manifest ends up containing architecture-dependent file hashes, we can't actually use `smfh` ergonomically. We considered doing something crazy using `symlinkJoin`, and even content-addressed derivations, but these solutions didn't pan out. For now we will just remove the manifest, and assume that we will not have to delete files, instead scripting the copy operations ourselves (with generous help from the `install` command). We can add support for manifests and file deletion in the future.
ff12bea to
3ebd448
Compare


Since the filegen manifest ends up containing architecture-dependent file hashes, we can't actually use
smfhergonomically.We considered doing something crazy using
symlinkJoin, and even content-addressed derivations, but these solutions didn't pan out.For now we will just remove the manifest, and assume that we will not have to delete files, instead scripting the copy operations ourselves (with generous help from the
installcommand).We can add support for manifests and file deletion in the future.