Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,8 @@ repos:
- id: ruff-format
- id: ruff
args: [ --fix ]
- repo: https://github.com/kamadorueda/alejandra
rev: 3.0.0
hooks:
# Requires Alejandra to be previously installed in the system
- id: alejandra-system
18 changes: 17 additions & 1 deletion flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

96 changes: 49 additions & 47 deletions flake.nix
Original file line number Diff line number Diff line change
@@ -1,58 +1,60 @@
{
description = "brain";
inputs.nixpkgs.url = "github:nixos/nixpkgs/nixos-unstable";
description = "A script to patch the MonoLisa font with Nerd Fonts glyphs.";

outputs = inputs @ {
inputs = {
nixpkgs.url = "github:nixos/nixpkgs/nixos-unstable";
systems.url = "github:nix-systems/default";
};

outputs = {
self,
nixpkgs,
systems,
}: let
inherit (nixpkgs.lib) genAttrs;
forAllSystems = f:
genAttrs
["x86_64-linux" "x86_64-darwin" "aarch64-linux" "aarch64-darwin"]
(system: f nixpkgs.legacyPackages.${system});
inherit (nixpkgs.lib) genAttrs makeBinPath;
eachSystem = fn:
genAttrs (import systems)
(system:
fn system
(import nixpkgs {
localSystem.system = system;
overlays = [self.overlays.default];
}));
in {
packages = forAllSystems (
pkgs:
with pkgs; {
default = stdenv.mkDerivation {
name = "monolisa-nerdfont-patch";
src = ./.;
nativeBuildInputs = [ makeWrapper ];
buildInputs = [
fontforge
python3
];
unpackPhase = ":";
buildPhase = ":";
installPhase = ''
mkdir -p $out/bin
install -m755 -D ${./patch-monolisa} $out/bin/monolisa-nerdfont-patch
install -m755 -D ${./font-patcher} $out/bin/font-patcher
cp -r ${./bin} $out/bin/bin
cp -r ${./src} $out/bin/src
'';
postFixup = ''
wrapProgram $out/bin/monolisa-nerdfont-patch \
--set PATH ${lib.makeBinPath [
fontforge
]}
overlays = {
default = final: _prev: let
pkgs = final;
in {
monolisa-nerdfont-patch = pkgs.stdenv.mkDerivation {
name = "monolisa-nerdfont-patch";
src = ./.;
nativeBuildInputs = with pkgs; [makeWrapper];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed, this change is bad, don't ever use an external pkgs from inside an overlay. You should treat final as if it were pkgs, otherwise you evaluate Nixpkgs stages twice. See here for some relevant discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind the above comment. I now see line 26. If this is your preference (and you should be consistent with your change on lines 16-22) consider using the naming scheme that I have come up with in the thread I have already linked. It does make sense to rename final to pkgs, but as final/self and prev/super can also be used for overrideAttrs (and others) inside overlays, to avoid shadowing, consider pkgs: pkgs0.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider using the naming scheme that I have come up with in the thread I have already linked. It does make sense to rename final to pkgs, but as final/self and prev/super can also be used for overrideAttrs (and others) inside overlays, to avoid shadowing, consider pkgs: pkgs0.

For one they aren't used within this overlay so avoiding name confusion is a moot point. And secondly, your scheme is incompatible with nix flake check as I previously demonstrated when you first submitted a PR with this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, nixfmt caused the check failure. Not this.

buildInputs = with pkgs; [fontforge python3];
buildPhase = ":";
installPhase = ''
mkdir -p $out/bin
install -m755 -D ${./patch-monolisa} $out/bin/monolisa-nerdfont-patch
install -m755 -D ${./font-patcher} $out/bin/font-patcher
cp -r ${./bin} $out/bin/bin
cp -r ${./src} $out/bin/src
'';
postFixup = ''
wrapProgram $out/bin/monolisa-nerdfont-patch \
--set PATH ${makeBinPath (with final; [fontforge])}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is subjective, and I understand that it is preference, but parts of Nixpkgs are moving away from this, which is why I changed it. The idea is to keep it clear what is in lib and what is in the prelude. You may safely ignore this comment, especially since I can't find the relevant discussion that I read and I can provide no evidence other than anecdotal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the issue I am thinking of in Nixpkgs is the over-usage of with which causes name-clashing. I don't remember well.

'';
};
};
}
);
};

devShells = forAllSystems (
pkgs:
with pkgs; {
default = mkShell {
buildInputs = [
fontforge
python3
pre-commit
];
};
}
);
packages = eachSystem (system: pkgs: {
default = self.packages.${system}.monolisa-nerdfont-patch;
monolisa-nerdfont-patch = pkgs.monolisa-nerdfont-patch;
});

devShells = eachSystem (_: pkgs: {
default = pkgs.mkShell {
buildInputs = with pkgs; [fontforge python3 pre-commit];
Copy link
Contributor Author

@spikespaz spikespaz Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question, not a suggestion or correction: why did you choose to use buildInputs over packages?

Now a little pushy: mkShell is a smart function, is documented, and it does not suggest using buildInputs (though this is common practice). See the manual.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It make no difference they are all just merged.

};
});
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand not wanting to take the formatter's flake as an input, however consider re-introducing the formatter output because it is special. You can use the package from Nixpkgs instead. The output allows nix fmt to work. This is useful: I manually run nix fmt to keep myself organized while I am working on dirty code, and additionally, configure my editor to format the code using the nix fmt command when the file is saved. This allows me to write long lines, and then semi-automatically format to keep my head clear and the code readable.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am well aware of how nix fmt works.

}