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

pyexiv2: 0.3.2 -> 2.1.0 #82771

Open
wants to merge 1 commit into
base: master
from
Open

pyexiv2: 0.3.2 -> 2.1.0 #82771

wants to merge 1 commit into from

Conversation

@jchv
Copy link
Contributor

jchv commented Mar 17, 2020

Motivation for this change

pyexiv2 has been broken for quite a while, but since then it has been rewritten. It no longer has a Boost dependency and now only works on Python 3 (formerly, it only worked on Python 2.)

Only one package depends on pyexiv2, jbrout. However, it is old and broken, so I made a PR to remove it in #82770.

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.
@jchv jchv requested review from FRidh and jonringer as code owners Mar 17, 2020
@ofborg ofborg bot added the 6.topic: python label Mar 17, 2020
@jchv jchv force-pushed the jchw-forks:update/pyexiv2 branch from 5713145 to 342d07d Mar 17, 2020
@jchv
Copy link
Contributor Author

jchv commented Mar 19, 2020

Thanks for the feedback. All suggestions are applied and everything seems to be in working order.

@FRidh
Copy link
Member

FRidh commented Mar 27, 2020

@GrahamcOfBorg build pyexiv2

@FRidh
Copy link
Member

FRidh commented Mar 27, 2020

@GrahamcOfBorg build python3Packages.pyexiv2

Copy link
Member

FRidh left a comment

Ran 0 tests in 0.000s

No tests are found. Either the test runner cannot find tests and needs to be patched, or there simply are no tests in which case the tests need to be disabled. Do include a comment explaining why the tests are disabled.

pkgs/development/python-modules/pyexiv2/default.nix Outdated Show resolved Hide resolved
@jchv jchv force-pushed the jchw-forks:update/pyexiv2 branch from ee4c67e to 57f4ec0 Mar 28, 2020
@jchv jchv force-pushed the jchw-forks:update/pyexiv2 branch from 57f4ec0 to 2e660b0 Mar 30, 2020
@jchv
Copy link
Contributor Author

jchv commented Mar 30, 2020

@FRidh

Ran 0 tests in 0.000s

No tests are found. Either the test runner cannot find tests and needs to be patched, or there simply are no tests in which case the tests need to be disabled. Do include a comment explaining why the tests are disabled.

I took a look at what was going on. I'll admit I'm not 100% sure what the deal is because I am not overly familiar with how setuptools integrates with stuff like pytest, however, I suspect that it was not able to resolve the tests because its in a non-standard location (pyexiv2/tests vs tests.) Upstream, it looks like the recommended way of running the tests is by running pytest itself inside the folder, so I made a checkPhase that does this. May be possible to obsolete this by improving the upstream.

Copy link
Contributor

jonringer left a comment

otherwise LGTM, tests run

pushd pyexiv2/tests
pytest
popd
Comment on lines +41 to +43

This comment has been minimized.

Copy link
@jonringer

jonringer Apr 8, 2020

Contributor
Suggested change
pushd pyexiv2/tests
pytest
popd
pytest pyexiv2/tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.