Skip to content
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

Add Nix flake support #370

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Add Nix flake support #370

wants to merge 5 commits into from

Conversation

jakeisnt
Copy link

@jakeisnt jakeisnt commented Dec 24, 2022

This helps anyone who uses NixOS or nix get started with a development environment seamlessly. If someone has nix and direnv installed, upon entering this repository they'll have a development environment with the exact versions of rust and all of the system packages necessary to build the repo.

It:

  • Exposes a development shell with all system and rust dependencies configured
  • Adds a glif-test command adhering to the suggested cargo run command in the README for testing the package
  • Enables nix builds of the glif package, which can then be pulled into other packages

I put this together to get glif to run on my end; merge if others would use, OK to close if not.

Copy link
Collaborator

@ctrlcctrlv ctrlcctrlv left a comment

Choose a reason for hiding this comment

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

Thanks for the very long contribution. I've used nix before but barely, I just used it as some package required it so I don't know how to test this but assume it works.

However I think this ought to also build (at least) MFEKmetadata, MFEKstroke, MFEKpathops, MFEKinit. They're all a lot simpler to build. Some functions will not work if MFEKmetadata is not installed. MFEKinit is very useful for making initial .glif files.

@ctrlcctrlv
Copy link
Collaborator

If you do this then probably these files belong in a new repo, MFEK/flake.nix. I can create it for you and leave it empty if you want me to.

@ctrlcctrlv
Copy link
Collaborator

Oh and last thought. MFEK's IPC uses --version and $PATH only to find its fellow modules so there's nothing special you have to do besides put all those binaries in the NixOS $PATH.

@alerque
Copy link
Contributor

alerque commented Dec 24, 2022

I would suggest a flake in each of those other repositories, then depend on them from the flake in this repository. Having a "parking" repository just for a combined flake kind of defeats the purpose and expected usage.

@ctrlcctrlv
Copy link
Collaborator

Oh, I see. Well, I still think a parking repository makes sense, as even though this is the org's most popular project, it's not the "root" module in any sense. You can use MFEKstroke / MFEKpathops / MFEKmetadata without it just fine.

But perhaps that actually makes this easier for @jakeisnt. If he adds a flake to MFEKmetadata, and makes this rely on that, I'll consider that enough to merge this. (That's the only IPC-checked dependency at current.)

@jakeisnt
Copy link
Author

jakeisnt commented Dec 24, 2022

Agree that the flake-per-repo approach is more reasonable, as it allows developers to make use of nix shell environments. On it!

@ctrlcctrlv
Copy link
Collaborator

Thanks! Sorry for my ignorance of NixOS :-)

If there's enough interest I'll make MFEK/flake.nix myself after merging this and the MFEKmetadata PR.

flake.nix Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
@ctrlcctrlv
Copy link
Collaborator

ctrlcctrlv commented Dec 25, 2022

Agree that the flake-per-repo approach is more reasonable, as it allows developers to make use of nix shell environments.

Do you think that the idea of having an MFEK/flake.nix with all the binary tools isn't useful then?

The best supported installation method, if you can even call it one, is basically that:

@ctrlcctrlv ctrlcctrlv marked this pull request as draft December 26, 2022 21:13
@ctrlcctrlv
Copy link
Collaborator

Marked as draft as blocked on MFEK/metadata#139 (comment) (see subsequent comment on potential fix).

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.

3 participants