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

haskell generic-builder: add doHpack (defaults to false) #40557

Closed
wants to merge 1 commit into from

Conversation

domenkozar
Copy link
Member

Follow up on #25010

I've verified that nix-build -A haskellPackages.mtl is a noop with this change.

Copy link
Member

@peti peti left a comment

Choose a reason for hiding this comment

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

I am not a fan of this addition. I am not aware of any single package in Nixpkgs that actually needs this feature, and user packages that do need it can easily achieve this functionality by calling hpack in the preConfigure hook. Therefore. I'd rather not add this feature to our generic builder.

I would prefer to have this feature as a re-usable combinator function in haskell.lib.

@LnL7
Copy link
Member

LnL7 commented May 16, 2018

There is at least one

buildTools = [ hpack ];
preConfigure = "hpack .";
but I assume the usecase is mostly for packages that are not part of nixpkgs.

@Profpatsch
Copy link
Member

I understand @peti’s concern and to be fair, adding a line to preConfigure and creating a helper in lib sounds like a sensible idea.

@domenkozar
Copy link
Member Author

domenkozar commented May 17, 2018

The reason it's exposed as a flag to the generic builder is that cabal2nix (or any other tool using generic builder) could support hpack detection and enable the switch for packages that use hpack (for haksell development).

If we use the combinator approach we lose composability due to stringly-typed abstraction of preConfigure, etc.

This change adds two lines and introduces no recompilations.

@peti
Copy link
Member

peti commented May 17, 2018

This change adds two lines and introduces no recompilations.

Yes, it is true that the implementation is very straightforward. I'm not so much concerned by code complexity here but rather by "featureritis", i.e. we're adding specialized capabilities to our generic builder here that (a) Nixpkgs doesn't need and that (b) can be easily accomplished by other means. Therefore, I'd I would prefer not to commit to this API. It's just not necessary.

@Profpatsch
Copy link
Member

@domenkozar You are not the first to propose this change. :P

@Profpatsch
Copy link
Member

@domenkozar you are not the first to propose this change. :P

@Profpatsch
Copy link
Member

Profpatsch commented May 17, 2018

@domenkozar you are not the first to propose this change, I think I did at least once. :)

@domenkozar domenkozar closed this Jul 16, 2018
@Profpatsch
Copy link
Member

Huh, I seem to have made the last one (three) comments on a flaky internet connection. :)

@domenkozar domenkozar reopened this Oct 6, 2018
@domenkozar
Copy link
Member Author

I'd like to add how this is biting me, in slight hope that it changes our minds.

  1. I'm using stack2nix which generates src = ./cachix/cachix-api;
  2. I'm using stack for development, which when switching git branches, results into different cabal files
  3. When using generated Nix expressions, Nix bails out with cachix-api.cabal was modified manually, please use --force to overwrite.

@domenkozar
Copy link
Member Author

Probably best to discuss in cabal2nix.

@domenkozar domenkozar closed this Oct 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants