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.face_recognition: 1.2.1 -> 1.2.2 #38361

Merged
merged 2 commits into from Apr 4, 2018

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Apr 3, 2018

Motivation for this change
  • Bumped pythonPackages.face_recognition to the latest version to get several important fixes (see the commit message for further reference)
  • Reverted the latest pythonPackages.dlib bump as its build is currently broken
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

I think you should

  1. raise an upstream issue about the dlib incompatibility
  2. override the dlib version in the face_recognition expression instead of reverting the update

@Ma27
Copy link
Member Author

Ma27 commented Apr 3, 2018

@dotlambda that's not the problem. When running nix-build -A pythonPackages.dlib the build fails right now locally and on hydra: https://hydra.nixos.org/build/72222721

@dotlambda
Copy link
Member

@Ma27 That's bad. Do you have an idea what could be causing the segfault? /cc @globin

@GrahamcOfBorg build python2.pkgs.dlib python3.pkgs.dlib

@Ma27
Copy link
Member Author

Ma27 commented Apr 3, 2018

Don't have a spontaneous idea unfortunately. I can have a look at the diff of dlib between 19.9 and 19.10 though %)

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: python2.pkgs.dlib, python3.pkgs.dlib

Partial log (click to expand)

.
.
.
.
.
                      [100%]


========================== 51 passed in 0.59 seconds ===========================

@dotlambda
Copy link
Member

I'm currently running git bisect and will keep you updated.

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: python2.pkgs.dlib, python3.pkgs.dlib

Partial log (click to expand)

tools/python/test/test_matrix.py ......                                  [ 39%]
tools/python/test/test_point.py ...                                      [ 45%]
tools/python/test/test_range.py .....                                    [ 54%]
tools/python/test/test_rgb_pixel.py .                                    [ 56%]
tools/python/test/test_sparse_vector.py ....                             [ 64%]
tools/python/test/test_vector.py ..................                      [100%]

========================== 51 passed in 3.23 seconds ===========================
/nix/store/7dp4adw4r2ah9mpy5bb71bcxyznj1z3k-python2.7-dlib-19.9
/nix/store/saxjq6rnc6wspnqwvpyr1djcpx6crcp1-python3.6-dlib-19.9

@Ma27
Copy link
Member Author

Ma27 commented Apr 3, 2018

awesome, thanks! I updated the description as it is indeed somehow misleading regarding which build was broken by the last dlib bump ;)

@dotlambda
Copy link
Member

It seems like davisking/dlib@14cbb80 is causing this. However, I don't get a segfault but

  tools/python/test/test_svm_c_trainer.py ....../nix/store/6kz2vbh98s2r1pfshidkzhiy2s2qdw0a-stdenv-linux/setup: line 1223:  2914 Aborted                 /nix/store/azw9ys2m2fpfzf730xjcxja890gpyp58-python3-3.6.4/bin/python3.6m nix_run_setup test

Ma27 added 2 commits April 3, 2018 14:45
The following fixes have been applied according to the changelog (https://github.com/ageitgey/face_recognition/releases)

- Added the face_detection CLI command
- Removed dependencies on scipy to make installation easier
- Cleaned up KNN example and fixed a bug with drawing fonts to label detected faces in the demo

Furthermore the maintainer switched to actual GIT tags for PyPI releases
as discussed in ageitgey/face_recognition#417 and NixOS#37566
@Ma27
Copy link
Member Author

Ma27 commented Apr 4, 2018

@dotlambda did you make any progress here or do you have any idea what could be causing this?

@dotlambda
Copy link
Member

No progress, I don't have time right now. Would you mind opening an upstream issue about this? Maybe they can help out.
I'll merge this for now and hope we can eventually update to something like 19.11 which fixes this.

@dotlambda dotlambda merged commit 1ddff47 into NixOS:master Apr 4, 2018
@Ma27 Ma27 deleted the update-face-recognition branch April 4, 2018 16:39
@Ma27
Copy link
Member Author

Ma27 commented Apr 4, 2018

@dotlambda I can have a closer look tonight and file a bug on upstream. Will keep you updated in case there's anything relevant for us %)

Ma27 added a commit to Ma27/nixpkgs-update that referenced this pull request Apr 7, 2018
The current checklist for filing new patches suggests to run `nox-review wip` to ensure
that the current change (e.g. package bumps) don't break package bumps:
https://github.com/NixOS/nixpkgs/blob/e1dee4efcbffc72260025078bf8297a3b732509c/.github/PULL_REQUEST_TEMPLATE.md

I experienced this behavior from time to time (see NixOS/nixpkgs#38513
or NixOS/nixpkgs#38361) which cost me some extra time to spot the
reasons for additional breackage.

Running `nox-review wip` can be quite time consuming, especially when
bumping packages that trigger a mass-rebuild or a stdenv rebuild,
*however* this patch helps to reduce the risk of additional breackage
caused by unwanted package bumps.
@Ma27 Ma27 changed the title Update face recognition pythonPackages.face_recognition: 1.2.1 -> 1.2.2 Apr 11, 2018
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