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

pythonPackages.diff_cover: fix build #36934

Merged
merged 1 commit into from Mar 15, 2018
Merged

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Mar 13, 2018

Motivation for this change

In order to adjust the language with LC_ALL properly the
glibcLocales is needed as checkInput. This was the only thing
preventing the testsuite from passing.

Furthermore I moved the default package from diff_cover to
diff-cover and added an alias for diff_cover for compatibility
reasons.

See ticket #36453
See https://hydra.nixos.org/build/70682982/nixlog/3

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@dotlambda
Copy link
Member

What's the reason for renaming when the package is called diff_cover on PyPI?

@Ma27
Copy link
Member Author

Ma27 commented Mar 14, 2018

@dotlambda well, I actually assumed that we prefer attribute names with dashes as separation (I usually got the feedback the dashes are preferable). However if we want to match the attribute names with the PyPI package names, I can revert this change %)

@dotlambda
Copy link
Member

I think someday this might be changed for all Python packages, but until it is I'd keep it as is.

In order to adjust the language with `LC_ALL` properly the
`glibcLocales` is needed as `checkInput`. This was the only thing
preventing the testsuite from passing.

See ticket NixOS#36453
See https://hydra.nixos.org/build/70682982/nixlog/3
@Ma27
Copy link
Member Author

Ma27 commented Mar 15, 2018

@dotlambda anything else?:)

@dotlambda
Copy link
Member

@GrahamcOfBorg build python2.pkgs.diff_cover python3.pkgs.diff_cover

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: python2.pkgs.diff_cover, python3.pkgs.diff_cover

Partial log (click to expand)

test_quality (diff_cover.tests.test_violations_reporter.pycodestyleQualityReporterTest) ... ok
test_quality_error (diff_cover.tests.test_violations_reporter.pycodestyleQualityReporterTest) ... ok
test_quality_pregenerated_report (diff_cover.tests.test_violations_reporter.pycodestyleQualityReporterTest) ... ok

----------------------------------------------------------------------
Ran 219 tests in 41.018s

OK
/nix/store/pizmd4avjb93pc2hlic0jcsmy0242i59-python2.7-diff_cover-1.0.2
/nix/store/dcarvgbw3ac32f3i8c984ahjg8mmh881-python3.6-diff_cover-1.0.2

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: python2.pkgs.diff_cover, python3.pkgs.diff_cover

Partial log (click to expand)

test_quality (diff_cover.tests.test_violations_reporter.pycodestyleQualityReporterTest) ... ok
test_quality_error (diff_cover.tests.test_violations_reporter.pycodestyleQualityReporterTest) ... ok
test_quality_pregenerated_report (diff_cover.tests.test_violations_reporter.pycodestyleQualityReporterTest) ... ok

----------------------------------------------------------------------
Ran 219 tests in 73.278s

OK
/nix/store/v8qlvzpbpzhqscdg1slpdk0b6p28a742-python2.7-diff_cover-1.0.2
/nix/store/h9s87mh40npcfy0agk6gfpylp0bpy8ix-python3.6-diff_cover-1.0.2

@dotlambda dotlambda merged commit 95836ab into NixOS:master Mar 15, 2018
@dotlambda
Copy link
Member

@Ma27 Thanks a lot!

Cherry-picked in 94ed437.

@Ma27 Ma27 deleted the fix-diffcover branch March 15, 2018 12:05
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

3 participants