Skip to content

Conversation

akshaymankar
Copy link
Member

@akshaymankar akshaymankar commented Oct 19, 2022

Using haskellPacakges.shellFor directly forced us to use nix-shell which exports too many environment variables. These environment variables cause a lot of problems, specially for people not using NixOS.

This change reads buildInputs and nativeBuildInputs for the derivation produced by haskellPacakges.shellFor and adds it to paths of pkgs.buildEnv. To allow cabal to find C dependencies, we also have to export PKG_CONFIG_PATH and LIBRARY_PATH

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

Using haskellPacakges.shellFor directly forced us to use nix-shell which exports
too many environment variables. These environment variables cause a lot of
problems, specially for people not using NixOS.

This change reads buildInputs and nativeBuildInputs for the derivation produced
by haskellPacakges.shellFor and adds it to paths of pkgs.buildEnv. To allow
cabal to find C dependencies, we also have to export PKG_CONFIG_PATH.
@akshaymankar akshaymankar temporarily deployed to cachix October 19, 2022 16:00 Inactive
@akshaymankar akshaymankar temporarily deployed to cachix October 19, 2022 16:00 Inactive
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Oct 19, 2022
@akshaymankar akshaymankar temporarily deployed to cachix October 19, 2022 17:23 Inactive
@akshaymankar akshaymankar temporarily deployed to cachix October 19, 2022 17:25 Inactive
Copy link
Contributor

@elland elland left a comment

Choose a reason for hiding this comment

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

Works for me!

@akshaymankar
Copy link
Member Author

@pcapriotti @battermann @jschaul can y'all also please test this to see if this also gets rid of your locale problem and other things that we worked around yesterday?

Copy link
Member

@jschaul jschaul left a comment

Choose a reason for hiding this comment

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

Much better, my warnings went away.

@akshaymankar
Copy link
Member Author

@pcapriotti @battermann in case this works for both of you, please merge the PR. If not, let's look at it on Monday.

@battermann
Copy link
Contributor

@pcapriotti @battermann in case this works for both of you, please merge the PR. If not, let's look at it on Monday.

Warnings went away, but the problem with signed commits remains. However, the latter does not speak against merging.

@akshaymankar akshaymankar merged commit eba4b7f into develop Oct 24, 2022
@akshaymankar akshaymankar deleted the akshaymankar/use-build-env branch October 24, 2022 08:17
elland pushed a commit that referenced this pull request Oct 24, 2022
Using haskellPacakges.shellFor directly forced us to use nix-shell which exports
too many environment variables. These environment variables cause a lot of
problems, specially for people not using NixOS.

This change reads buildInputs and nativeBuildInputs for the derivation produced
by haskellPacakges.shellFor and adds it to paths of pkgs.buildEnv. To allow
cabal to find C dependencies, we also have to export PKG_CONFIG_PATH 
and LIBRARY_PATH
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants