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

cabal-install: wrap to put binutils and ghc into scope #85136

Closed
wants to merge 1 commit into from

Conversation

@xaverdh
Copy link
Contributor

@xaverdh xaverdh commented Apr 13, 2020

Motivation for this change

Fixes #55995.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@iblech
Copy link
Contributor

@iblech iblech commented Apr 26, 2020

The issue referenced in this pull request affects me as well, and I can confirm that this pull request fixes the issue. Also it looks to be exactly the proper way to fix it. Thank you for your work, @xaverdh!

While we are at it, we could think about adding a further runtime dependency: namely ghc itself. If ghc is not in the path, cabal immediately aborts with a message like:

$ cabal new-build
cabal: The program 'ghc' version >=7.0.1 is required but it could not be

found.

Of course not many people use cabal without having ghc in scope. But some might: People who don't know much about Haskell but still want to try some Haskell program and are following some README instructions telling them to "just run cabal new-build".

@louwers
Copy link

@louwers louwers commented Apr 26, 2020

Agreed that ghc should be included as well.

@xaverdh xaverdh force-pushed the cabal-install-binutils branch from f0dfc37 to 1549d29 Apr 27, 2020
@xaverdh
Copy link
Contributor Author

@xaverdh xaverdh commented Apr 27, 2020

Agreed that ghc should be included as well.

Done.

@xaverdh xaverdh changed the title cabal-install: wrap to put binutils into scope cabal-install: wrap to put binutils and ghc into scope Apr 27, 2020
@pbogdan pbogdan requested a review from cdepillabout Apr 27, 2020
@cdepillabout
Copy link
Member

@cdepillabout cdepillabout commented Apr 28, 2020

Hmm, as far as I know, we haven't done this in the past because cabal can be used with multiple GHCs at the same time.

That is to say, you can create an environment with multiple GHCs available and tell cabal-install at runtime which GHC to use.

As far as I know, you can also use the same cabal-install with other Haskell compilers, like ghcjs.

Although I do agree that wrapping cabal-install to point to binutils may be a good idea.

cc @peti on this one.

Copy link
Member

@cdepillabout cdepillabout left a comment

See comment above.

Copy link
Member

@peti peti left a comment

That change causes trouble for people with non-trivial setups, especially those who use Nix on a non-NixOS host system.

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
@cdepillabout
Copy link
Member

@cdepillabout cdepillabout commented Apr 28, 2020

@xaverdh Okay, so it sounds like we'll be willing to merge this in as long as you remove the reference to ghc in the wrapped cabal-install.

I think it makes sense to reference binutils to get ar.

@peti
Copy link
Member

@peti peti commented Apr 28, 2020

@xaverdh xaverdh force-pushed the cabal-install-binutils branch from 1549d29 to 6594554 Apr 28, 2020
@ofborg ofborg bot removed the 6.topic: haskell label Apr 28, 2020
@xaverdh
Copy link
Contributor Author

@xaverdh xaverdh commented Apr 28, 2020

I think it makes sense to reference binutils to get ar.
Does cabal-install need 'ar'?

Yes indeed, see the original issue (#55995).

@cdepillabout
Copy link
Member

@cdepillabout cdepillabout commented Apr 28, 2020

@GrahamcOfBorg build cabal-install

@peti
Copy link
Member

@peti peti commented Apr 28, 2020

Does cabal-install need 'ar'?

Yes indeed, see the original issue (#55995).

So what happens if I install cabal-install on a openSUSE Tumbleweed Linux and Tumbleweed provides binutils, gcc, and ghc? (This is not a theoretical question, this is what I'm actually doing right now.)

I don't know ... I'm not convinced that wrapping cabal-install at all is appropriate.

@xaverdh
Copy link
Contributor Author

@xaverdh xaverdh commented Apr 28, 2020

@peti so you are saying that using the same cabal version with multiple ghc versions works, but binutils / gcc has to somehow match? Is this documented somewhere?

I guess instead of prefixing we could also suffix the nixpkgs binutils to PATH. This would allow cabal-install on e.g. NixOS to work out of the box while also hopefully not changing anything for users on other setups (because they will have all binutils tools in PATH anyway, taking precedence over nixpkgs binutils).
The cost of this impurity is obviously that things might silently break if they don't have binutils stuff installed on their system and there is some incompatibility of course.

@cdepillabout
Copy link
Member

@cdepillabout cdepillabout commented May 5, 2020

@xaverdh It sounds like peti has a use-case for cabal-install not pulling in binutils or ghc.

As far as I know, we don't have any firm guidelines in nixpkgs as to whether build-tools that don't require reference to a compiler should or should not include a reference to the compiler.

I think peti has a good point that you can install cabal-install and use it with a ghc or binutils installed a separate way.

However, I'm not sure what other build-tools that can work with multiple compiler versions do in this case. If you wanted to create an issue (or a post on discourse) and ask for input from other members of the nix community, it would be helpful in deciding what to do here.

Also, it would be helpful to know what exactly cabal-install requires ar (and bintools) for. Would there ever be a time when you'd want to use a system-installed bintools vs a nixpkgs wrapped bintools? Would there ever be a time you wouldn't want to use a nixpkgs wrapped bintools? Is it just to reduce closure size?


Also, as for your suggestion, I can't recall anywhere in nixpkgs suffixing things to PATH, because of the impurity you mention. Although I guess this would be a potential solution.

If we figured out why cabal-install required bintools, it might make it easier to figure out whether or not this solution would be reasonable.

@louwers
Copy link

@louwers louwers commented May 10, 2020

Perhaps relevant for this discussion: cabal has a --with-compiler flag that allows you to specify a particular compiler. https://www.haskell.org/cabal/users-guide/installing-packages.html#cmdoption-setup-configure-with-compiler

@xaverdh
Copy link
Contributor Author

@xaverdh xaverdh commented Oct 25, 2020

Ok, so here is the list of external runtime dependencies of cabal-install. Most of it is quite niche, but things linke ghc (defaulting to haskellPackages.ghc), ar,ld and tar should probably be added to PATH by default on nixos.

Since there seems to be somewhat of a clash here between the NixOS use-case and the Nix on other linux use-case, maybe the right thing to do is to add a programs.cabal-install module to nixos, which wraps cabal-install?

Will open a pr which does that when I find the time.

@xaverdh xaverdh closed this Oct 25, 2020
@xaverdh xaverdh deleted the cabal-install-binutils branch Oct 27, 2020
@xaverdh xaverdh mentioned this pull request Oct 27, 2020
10 tasks
@cdepillabout cdepillabout mentioned this pull request Jan 26, 2021
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

6 participants