Skip to content

Add flake.nix #4328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Add flake.nix #4328

wants to merge 15 commits into from

Conversation

SignalWalker
Copy link

@SignalWalker SignalWalker commented May 22, 2025

Adds a flake.nix (and its attendant flake.lock), containing outputs for all of the packages and devshells output by default.nix and shell.nix, an overlay output, and an apps output (to make it possible to run emuhawk with just nix run github:TASEmulators/BizHawk).

Notes:

  • The flake description is just copied straight from the github repo short description
  • I set the nixpkgs input to match the default already present in default.nix
  • I also set a formatter output (which tells nix fmt what formatter to use), which may or may not be desired
  • The default package and app are emuhawk-latest-bin, and the default devshell is emuhawk-latest, because that made sense to me, but it could also make sense to have them all be emuhawk-latest
  • The apps output may not be completely correct -- it works for the default, but I haven't checked that the generated program path is accurate for all packages
  • nix flake show acts weird if you don't set --option allow-import-from-derivation true

Check if completed:

@YoshiRulz YoshiRulz self-requested a review May 22, 2025 17:54
@SignalWalker

This comment was marked as resolved.

@YoshiRulz

This comment was marked as resolved.

@YoshiRulz YoshiRulz dismissed their stale review May 23, 2025 23:37

Resolved

@YoshiRulz
Copy link
Member

Is https://github.com/numtide/flake-utils considered good practice or nah?

launchScriptsForLocalBuild = launchScriptsFor emuhawk-local.assemblies true;
};
shellHook = drv: ''
export BIZHAWKBUILD_HOME='${builtins.toString ./.}'
Copy link
Member

Choose a reason for hiding this comment

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

Wait does this work in Flakes?

Copy link
Author

@SignalWalker SignalWalker May 25, 2025

Choose a reason for hiding this comment

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

Oh, when evaluating as part of a flake, that ./. gets interpreted as a nix store path, so BIZHAWKBUILD_HOME is set to something like /nix/store/1igqk0g3q3bj6zhcfmrgzirzjsqbgpx9-source instead of something like /home/tasbot/bizhawk. That's fine for reading data, but the store is (usually) read-only, so $BIZHAWK_HOME doesn't work as-is (I assume).

I'm not really sure what the best way to solve this would be (especially in the case of someone doing something like nix develop github:tasemulators/bizhawk, since that would mean that the source dir only exists in the store).

nix develop sets environment variables for each of the derivation outputs (ex. $out), which might be useful. They're relative to the directory in which you invoke nix develop, though (ex. if you invoke nix develop ./.#emuhawk-latest in BizHawk/Dist, $out is set to BizHawk/Dist/outputs/out rather than BizHawk/outputs/out).

Do builds output to $BIZHAWKBUILD_HOME/output, or do they output to $BIZHAWK_HOME? If it's the latter, does anything expect $BIZHAWKBUILD_HOME to be writable? If things only expect to be able to write to $BIZHAWK_HOME, we could add an isFlake parameter to the shellHook function and export $BIZHAWK_HOME=$out, if we don't mind that $out isn't guaranteed to be relative to the project root.

Copy link
Member

Choose a reason for hiding this comment

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

IMO it's fine to assume nix develop will only be called from the repo root, already checked-out.

BIZHAWKBUILD_HOME is used at build-time to locate the repo root (not for the main solution though). BIZHAWK_HOME is used at run-time, and it was mainly added for NixHawk so I don't think anything expects it to be writable, BUT it can't point to ${BizHawk-source}/output since that doesn't exist.

Copy link
Author

Choose a reason for hiding this comment

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

Some scripts seem to expect to be able to find (and write to) $BIZHAWKBUILD_HOME/output, ex.:

cp target/debug/libwaterboxhost.so "$BIZHAWKBUILD_HOME/output/dll"

For $BIZHAWK_HOME, could we just mkdir -p it in the launch script, since that and PathExtensions.cs seem to be the only places it gets used?

Copy link
Member

Choose a reason for hiding this comment

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

You can do that, but again, it needs to exist and contain the assemblies and other assets. The launch script is essentially cd $BIZHAWK_HOME; mono ./EmuHawk.exe.

No-one is going to be using EmuHawk's dev shell to build WaterboxHost. It can get its own shell when I eventually package it.

Copy link
Author

Choose a reason for hiding this comment

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

I mean, if the only thing we care about that uses it is the launch script, and we're assuming that nix develop is only being called from the repo root, then could we just do something like:

shellHook = isFlake: drv: ''
    export BIZHAWKBUILD_HOME="${builtins.toString ./.}"
    export BIZHAWK_HOME="${if isFlake then "$(pwd -P)" else "$BIZHAWKBUILD_HOME"}/output/"
    # ...
'';

As-is, if you're in a checked-out repo root, running Dist/BuildDebug.sh in nix develop seems to work completely fine and outputs to ${repo_root}/output (as in, not $BIZHAWKBUILD_HOME/output).

(This would mean, though, that the printf later on that tells the user where the build scripts are is misleading if you're in a flake, so I'll need to fix that.)

@SignalWalker
Copy link
Author

Is https://github.com/numtide/flake-utils considered good practice or nah?

Honestly, I just don't use it because downloading it adds a bit of time to flake commands and because nix error messages are so obtuse that I'd rather have as little abstraction as possible. As far as I know, though, it's not considered bad practice; that's just my own preference.

@YoshiRulz
Copy link
Member

Fair enough, I just don't like Flakes (as you've probably gathered) and would prefer to minimise the amount of code used for it.

@SignalWalker
Copy link
Author

Fair. I could rewrite it to use flake-utils, if you'd prefer that.

@YoshiRulz
Copy link
Member

Pinging @OmnipotentEntity who was working with Flakes at some point (#3564).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants