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

Jump to bottom

co-log, chronos: unmarkBroken by disabling tests for chronos #108780

Merged
merged 1 commit into from Jan 9, 2021
Merged

co-log, chronos: unmarkBroken by disabling tests for chronos #108780

merged 1 commit into from Jan 9, 2021

Conversation

cideM
Copy link
Contributor

@cideM cideM commented Jan 8, 2021 •
edited

Motivation for this change

The only reason co-log is broken because one of its dependencies chronos fails the doctests on GHC 8.10. I created an issue in the chronos repo but I think we can unbreak both packages until that is resolved

Will run nix review once PR is up

Strangely nix review says nothing to be built

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.

@cideM cideM changed the base branch from master to haskell-updates January 8, 2021 16:06
cdepillabout
cdepillabout reviewed Jan 9, 2021
View reviewed changes
pkgs/development/haskell-modules/configuration-common.nix Outdated
Comment on lines 933 to 937
chronos = unmarkBroken (dontCheck super.chronos);

# The chronos dependency is what's causing this to fail, due to the above
# issue with doctests
co-log = unmarkBroken super.co-log;
Copy link
Member

Choose a reason for hiding this comment

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

Instead of marking these unbroken here, can you just remove them from the broken-packages list in configuration-hackage2nix.yaml?

Here is a PR doing something similar: https://github.com/NixOS/nixpkgs/pull/108788/files#diff-3139f03f40f0c7740a4003f88522ac0af603af77f746f3e6bb8e26cccc1eaf88L8334

This this, you can remove the co-log = line, and just keep the chronos = dontCheck super.chronos; line.

Also, thanks for linking to the upstream issue for chronos!

@cideM
co-log, chronos: unmarkBroken
3947695 
co-log fails to compile due to its dependency on chronos, which fails to
compile because the doctests fail with newer GHC versions. So we just
disable the doctests and thus unbreak both.

byteverse/chronos#62
@cideM
Copy link
Contributor Author

cideM commented Jan 9, 2021

Thanks @cdepillabout it's a bit embarrassing that I didn't check this myself 🙈 I just looked at the rest of the file and saw lots of unmarkBroken and just kind of went with it. A bit lazy in hindsight.

The diff is now down to removing co-log and chronos from broken packages and marking chronos as dontTest

@cdepillabout
Copy link
Member

Looks good, thanks for this change!

@cdepillabout cdepillabout merged commit 0127170 into NixOS:haskell-updates Jan 9, 2021
@cideM cideM deleted the unbreak-co-log branch January 9, 2021 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdepillabout cdepillabout Awaiting requested review from cdepillabout

Assignees
No one assigned
Labels
6.topic: haskell 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@cideM @cdepillabout