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 a nix flake #165

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

Conversation

o1lo01ol1o
Copy link
Contributor

@o1lo01ol1o o1lo01ol1o commented Nov 16, 2023

...that allows gf to be built in a composable way. This will shortly be consumed by a flake in gf-rgl that builds using this flake as an input.

Credit to @anka-213 for the overrides and patches needed.

On top of #164

@inariksit inariksit marked this pull request as draft November 17, 2023 13:04
@inariksit
Copy link
Member

inariksit commented Nov 17, 2023

@anka-213 pointed out that we probably shouldn't have the this as an official part of gf-core, because it reverts 1294269 , which Krasimir seems to have strong preferences against. EDIT. nevermind, I misunderstood, this doesn't revert anything, just adds a patch that is only meaningful for nix users.

Is there a way where Krasimir can keep using his workflow of runghc Setup.hs {configure,build,install} and it works as it does now, but we can also include the changes that @o1lo01ol1o wants to include? EDIT. Krasimir can totally keep his workflow, and if I understood correctly, Anka was just worried about the new change being not aesthetically pleasing, and leading to potentially hard to maintain nix code in the future.

@anka-213
Copy link
Member

@inariksit I'm not sure that I'm against having it in gf-core as such, but we should probably change it a bit if we do so. It doesn't actually revert that patch entirely, it just contains a patch file for doing so when using nix, which I doubt would affect krasimir. It's mostly just that having a patch file in the repo like this feels a bit strange

Copy link
Member

Choose a reason for hiding this comment

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

To be clear, this is the file I'm talking about

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for clarification! I don't have personal opinions other than "great that GF can be built in several different ways and we don't have to choose one over the other".

Based on my experience of the rest of the GF maintainers, any strong opinions are less about aesthetics (is it strange to have a patch file in a repo? You're talking to people who are happy to use the DATA_DIR hack to set GF_LIB_PATH so 🤷), more about disliking backwards incompatibility and bloated dependencies.

@o1lo01ol1o
Copy link
Contributor Author

The only way to nix-ify this repo is by separating out the IO process that tries to contact a server. Short of a refactor, applying this patch seems sufficient. Though sub-par from a maintenance point of view since any fix to the repo upstream of the patch will break this flake. Until that time, it seems like a decent way to get gf (and the rgl, wordnet) as system libraries without building each off master manually.

This flake could also live at, eg, flakeshub.com, but that isn't great for maintainability or discoverability.

@anka-213
Copy link
Member

This flake could also live at, eg, flakeshub.com, but that isn't great for maintainability or discoverability.

it actually can’t, since it uses IFD (import from derivation), which isn’t supported on flakehub.

the main two reasons why i put it in a separate repo are

  1. so it’s on a repo i have push permissions on and can more easily maintain and
  2. to collect all the gf-related nix files in one place

but there are definitely advantages with having them directly in the repo. one is that it improves discoverability and makes it more official, another is that it will automatically be kept up to date with the latest version of gf-core

i’ve also considered adding it to nixpkgs, but i haven’t been completely sure how to do that and what structure to use, especially since it’s a programming language and might want to have access to libraries

@inariksit
Copy link
Member

Thanks for the discussion and clarifications! So as I said earlier, now that my misunderstanding was cleared, for my part I don't see a problem accepting this PR as is. Just double check that I have understood everything correctly:

  • Krasimir can keep his workflow now
  • Everyone using Nix can use this now
  • In the future, if the code in gf-core changes significantly, the patch that reverts cabal-install may become outdated, and then there will be problems for Nix users
  • Even if Nix workflow ends up not working (=needing a major refactor), Krasimir and everyone else who installs GF not via Nix can keep their desired workflow in the future

So if those are the facts, I have no problem whatsoever accepting this PR. (Also I would be happy to give Anka push rights to gf-core!)

@inariksit inariksit marked this pull request as ready for review November 23, 2023 11:55
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