Skip to content

dhall-json: fix build#67438

Merged
peti merged 0 commit intoNixOS:haskell-updatesfrom
marsam:fix-dhall-json
Aug 28, 2019
Merged

dhall-json: fix build#67438
peti merged 0 commit intoNixOS:haskell-updatesfrom
marsam:fix-dhall-json

Conversation

@marsam
Copy link
Contributor

@marsam marsam commented Aug 25, 2019

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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.
Notify maintainers

cc @

@marsam marsam requested a review from basvandijk as a code owner August 25, 2019 12:31
@ofborg ofborg bot added 6.topic: haskell General-purpose, statically typed, purely functional programming language 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Aug 25, 2019
Copy link
Member

@cdepillabout cdepillabout left a comment

Choose a reason for hiding this comment

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

LGTM

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/57

Copy link
Member

@timokau timokau left a comment

Choose a reason for hiding this comment

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

To me as well (though I'd prefer to backport the PR), but I don't feel comfortable merging a haskell PR since I know next to nothing about how those are managed.

@timokau
Copy link
Member

timokau commented Aug 27, 2019

CC @infinisil

@cdepillabout
Copy link
Member

It is possible this PR should be against the haskell-updates branch instead of master. However, I'm not clear on when the haskell-updates branch should be used and when master should be used.

I think @domenkozar and @matthewbauer also merge Haskell stuff.

@infinisil
Copy link
Member

Yeah this should go to haskell-updates, but the hackage-packages.nix change isn't needed there because the file gets regenerated regularly anyways

@cdepillabout
Copy link
Member

@infinisil When should PRs be based on haskell-updates vs. master? Should all changes to stuff in pkgs/development/haskell/ be to haskell-updates? Are there ever any times Haskell stuff should be merged in to master?

@infinisil
Copy link
Member

I'm pinging @peti for this, but as far as my understanding goes:

I think haskell-updates should only be used for updates that influence Haskell package builds, like the configuration-* files or generic-builder.nix. On that branch hackage2nix will get run with configuration-hackage2nix.yaml to regenerate hackage-packages.nix, which then gets used by hydra to build everything (with potential fixes in configuration-*.nix files), whose results then get used to mark packages as broken, which in turn then configuration-hackage2nix.yaml again.

And doing this on a separate branch allows Haskell updates to be bunched together so they can be fixed up if necessary and be built by hydra before merging to master

While I guess updates to these files in master would also make it to that branch eventually, this might be annoying with merge conflicts, it might take a while for it to apply (if you change configuration-hackage2nix.nix), and the changes might make some packages not be cached for some time (since hydra doesn't build haskell changes in master I don't think).

@peti
Copy link
Member

peti commented Aug 28, 2019

@infinisil's explanation in #67438 (comment) is exactly right. Manual changes to hackage-packages.nix in master may cause merge errors on haskell-updates that break the automatic scripts and should be avoided. Generally speaking, changes to configuration-hackage2nix.yaml almost always need to be merged to haskell-updates to achieve the correct effect.

@peti peti changed the base branch from master to haskell-updates August 28, 2019 07:45
@peti peti merged this pull request into NixOS:haskell-updates Aug 28, 2019
@cdepillabout
Copy link
Member

Thanks a lot for this information! So it sounds like the haskell-updates branch should always be used for changes to configuration-hackage2nix.yaml, and it general it should also be used for any changes in the files in pkgs/development/haskell-modules/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: haskell General-purpose, statically typed, purely functional programming language 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants