piexif: 1.0.13 -> 1.1.2 (and bunch of other packages) #49409

jluttine · 2018-10-29T16:40:04Z

Motivation for this change

I just wanted to upgrade Piexif but Thumbor didn't have tests enabled so I didn't dare to upgrade without having tests passing successfully. Well, enabling the tests required quite a few new packages to be added...

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 nox --run "nox-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)
Fits CONTRIBUTING.md.

veprbl

Looks great! This could use a rebase and reorder commits to enable thumbor tests after you add pyssim, preggy and cairosvg1.

veprbl · 2018-11-27T01:02:42Z

cc @Ma27

pkgs/development/python-modules/nose-focus/default.nix

pkgs/development/python-modules/nose-of-yeti/default.nix

Ma27 · 2018-11-27T01:18:06Z

First of all thank you @jluttine for the work!
As I was notified here (I guess because I maintain thumbor), I'd ask you to wait with merging until tomorrow, then I can have a look at this as well :)

pkgs/development/python-modules/should-dsl/default.nix

Ma27

Thanks a lot for working hard on improving these python packages (and all the dependency issues in here). I added some minor comments (I'm also happy to discuss about).

I just tested thumbor locally and it seems to still work :)

pkgs/development/python-modules/cairosvg/1_x.nix

pkgs/development/python-modules/remotecv/install_requires.patch

pkgs/development/python-modules/thumbor/default.nix

pkgs/development/python-modules/yanc/default.nix

jluttine · 2018-11-28T07:32:16Z

@veprbl Thanks for the comments!

I tried rebasing on master but got some conflicts not related to this pull request so I didn't dare proceed solving those. This is now based on nixos-unstable.

GitHub shows the commits in weird order. Probably based on some timestamps. The commits are already in the order you are requesting.

Ma27 · 2018-11-28T12:43:53Z

I tried rebasing on master but got some conflicts not related to this pull request so I didn't dare proceed solving those. This is now based on nixos-unstable. GitHub shows the commits in weird order. Probably based on some timestamps. The commits are already in the order you are requesting.

Indeed, the GitHub timeline can be a bit confusing, especially when force-pushing onto your branch. The history of a branch is usually clearer when looking at the "Commits" tab.

I'll have a look at the conflict, but I won't push a resolution (yet).

Ma27 · 2018-11-28T12:56:42Z

@jluttine it seems as you checked out your branch before the change(s) were merge that moved all remaining packages from pkgs/top-level/python-packages.nixinto their own files.

From what I can see it's only one large conflict which I resolved locally. I'll test all the changes now and I'd ask for your permission to push this onto your branch (I can do this as NixOS maintainer, but in such a case I'd ask for permission first ^^)

jluttine · 2018-11-28T13:05:26Z

@Ma27 Great, thanks! Feel free to push to this branch.

Ma27 · 2018-11-28T13:59:48Z

btw I'm getting irreproducible build failures during the Redis tests, that are fixed when restarting the build.

EDIT: I'm not sure though if it's an issue with the patch or with my conflict resolution. Reviewing the commits step by step for now...

Ma27 · 2018-11-28T15:04:32Z

The problem causing the test failures on my checkout is the following commit: 094e643

I'll look into the upstream sources (reverting this locally fixed the thumbor build again. I'll force-push the fixed patches as soon as I've found a suitable solution, but please recheck if everything works for you as well ok @jluttine ?

Ma27 · 2018-11-28T15:32:35Z

@GrahamcOfBorg build pythonPackages.thumbor pythonPackages.piexif python3Packages.piexif

Ma27 · 2018-11-28T15:37:02Z

Let's wait for ofBorg, after that the change should be fine :)

GrahamcOfBorg · 2018-11-28T15:38:38Z

Success on x86_64-linux (full log)

Attempted: pythonPackages.thumbor, pythonPackages.piexif, python3Packages.piexif

Partial log (click to expand)

test_logger_should_not_be_null (tests.test_utils.UtilsTestCase) ... ok

----------------------------------------------------------------------
Ran 492 tests in 33.385s

OK (SKIP=1)
Joining threads....
/nix/store/yybv9iy8q83qjnddxmh5njlmvfyr986h-python2.7-thumbor-6.5.2
/nix/store/rldf72y0rrn5j51pldlmwn3liyv4rh67-python2.7-piexif-1.1.2
/nix/store/w6xxv2x02b529wghg61qzl7lmn6iqsrn-python3.7-piexif-1.1.2

GrahamcOfBorg · 2018-11-28T15:55:52Z

Success on aarch64-linux (full log)

Attempted: pythonPackages.thumbor, pythonPackages.piexif, python3Packages.piexif

Partial log (click to expand)

test_logger_should_not_be_null (tests.test_utils.UtilsTestCase) ... ok

----------------------------------------------------------------------
Ran 492 tests in 48.339s

OK (SKIP=1)
Joining threads....
/nix/store/n7rn86k9f0gm21sbz1m4vphnvk645agn-python2.7-thumbor-6.5.2
/nix/store/37bb0ylbgzi3vv5kh0q996sbln9sxw6f-python2.7-piexif-1.1.2
/nix/store/ab2qml7ys48l29wbc4xjdgnk8pgllh01-python3.7-piexif-1.1.2

GrahamcOfBorg · 2018-11-28T16:21:41Z

Timed out, unknown build status on x86_64-darwin (full log)

Attempted: pythonPackages.thumbor, pythonPackages.piexif, python3Packages.piexif

Partial log (click to expand)

libtool: compile:  clang++ -DHAVE_CONFIG_H -I. -I.. -pthread -DHAVE_PTHREAD=1 -DHAVE_ZLIB=1 -Wall -Wno-sign-compare -O2 -g -std=c++11 -DNDEBUG -c google/protobuf/compiler/java/java_message_builder.cc -o google/protobuf/compiler/java/java_message_builder.o >/dev/null 2>&1
/nix/store/sv35yjk452mggg76bz94nppibj96a66h-bash-4.4-p23/bin/bash ../libtool  --tag=CXX   --mode=compile clang++ -DHAVE_CONFIG_H -I. -I..    -pthread -DHAVE_PTHREAD=1 -DHAVE_ZLIB=1 -Wall -Wno-sign-compare -O2 -g -std=c++11 -DNDEBUG -c -o google/protobuf/compiler/java/java_message_builder_lite.lo google/protobuf/compiler/java/java_message_builder_lite.cc
libtool: compile:  clang++ -DHAVE_CONFIG_H -I. -I.. -pthread -DHAVE_PTHREAD=1 -DHAVE_ZLIB=1 -Wall -Wno-sign-compare -O2 -g -std=c++11 -DNDEBUG -c google/protobuf/compiler/java/java_message_builder_lite.cc  -fno-common -DPIC -o google/protobuf/compiler/java/.libs/java_message_builder_lite.o
libtool: compile:  clang++ -DHAVE_CONFIG_H -I. -I.. -pthread -DHAVE_PTHREAD=1 -DHAVE_ZLIB=1 -Wall -Wno-sign-compare -O2 -g -std=c++11 -DNDEBUG -c google/protobuf/compiler/java/java_message_builder_lite.cc -o google/protobuf/compiler/java/java_message_builder_lite.o >/dev/null 2>&1
building of '/nix/store/w783xj47dpgvc1gaqj22izfh5cmka8gs-protobuf-3.6.1.drv' timed out after 1800 seconds
cannot build derivation '/nix/store/4jb0vlx0pkqzpbivc90avbssipprza3p-opencv-3.4.3.drv': 3 dependencies couldn't be built
building of '/nix/store/jl77c90fs3jraylwdcafn3g5703rlgvp-python2.7-pyres-1.5.drv' timed out after 1800 seconds
cannot build derivation '/nix/store/ilvjzrj8xswgkc4ipr5zqam32xhs904k-python2.7-remotecv-2.2.2.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/cndjzfd5rrrp3z0nnrhbzl3ib8hsfx5k-python2.7-thumbor-6.5.2.drv': 4 dependencies couldn't be built
error: build of '/nix/store/cndjzfd5rrrp3z0nnrhbzl3ib8hsfx5k-python2.7-thumbor-6.5.2.drv' failed

Ma27

The change itself seems fine now, thanks a lot! 👍

I've observed several Darwin issues in the last days on Borg, it's possible that the Darwin package set isn't completely built, thus the timeout.

I'll wait with merging until @jluttine had a look at the new diff (after rebasing onto master and resolving the fairly big conflict in pkgs/top-level/python-packages.nix and fixing pyres to build on current master)

jluttine · 2018-11-29T07:20:25Z

@Ma27 Looks like all my review fixes are gone.

Upstream's requirements.txt wrongly suggests that redis==3.x is support (missing upper-bound constraint), but the tests break due to API incompatibilities.

Ma27 · 2018-11-29T11:39:28Z

reapplied review comments and rebuilt packages, thanks! :)

jluttine · 2018-11-29T11:56:29Z

Looks good. I ran nix-shell -p pythonPackages.thumbor and it successfully opened the shell. So I guess it's ok then. 🙂 I didn't run nox-review now.

Ma27 · 2018-11-29T13:20:45Z

The only failing package when running nix-review pr 49409 is python37Packages.Nikola, but this is failing on master as well: https://hydra.nixos.org/job/nixos/trunk-combined/nixpkgs.python37Packages.Nikola.x86_64-linux/all

I checked it locally and it doesn't seem to be related to your change (it wants piexif>=1.0.3, so your piexif bump won't break the installation)

Ma27 · 2018-11-29T13:21:30Z

@jluttine thanks!

jluttine requested a review from FRidh as a code owner October 29, 2018 16:40

GrahamcOfBorg added 6.topic: python 8.has: package (new) 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Oct 29, 2018

jluttine force-pushed the enable-thumbor-tests branch from 49b397f to 16c769e Compare October 30, 2018 17:41

jluttine mentioned this pull request Nov 9, 2018

python36Packages.piexif: 1.0.13 -> 1.1.1 #49983

Closed

jluttine force-pushed the enable-thumbor-tests branch from 16c769e to 71877f3 Compare November 18, 2018 15:29

jluttine changed the title ~~piexif: 1.0.13 -> 1.1.1 (and bunch of other packages)~~ piexif: 1.0.13 -> 1.1.2 (and bunch of other packages) Nov 18, 2018

jluttine mentioned this pull request Nov 18, 2018

python36Packages.piexif: 1.0.13 -> 1.1.2 #50599

Closed

jluttine force-pushed the enable-thumbor-tests branch from 71877f3 to 213b5d6 Compare November 19, 2018 07:18

veprbl approved these changes Nov 27, 2018

View reviewed changes

worldofpeace reviewed Nov 27, 2018

View reviewed changes

pkgs/development/python-modules/nose-focus/default.nix Outdated Show resolved Hide resolved

worldofpeace reviewed Nov 27, 2018

View reviewed changes

pkgs/development/python-modules/nose-of-yeti/default.nix Outdated Show resolved Hide resolved

worldofpeace reviewed Nov 27, 2018

View reviewed changes

pkgs/development/python-modules/should-dsl/default.nix Outdated Show resolved Hide resolved

Ma27 requested changes Nov 27, 2018

View reviewed changes

jluttine force-pushed the enable-thumbor-tests branch from 213b5d6 to cc4444a Compare November 28, 2018 07:20

GrahamcOfBorg added the 2.status: merge conflict label Nov 28, 2018

jluttine force-pushed the enable-thumbor-tests branch from cc4444a to 3e69c3c Compare November 28, 2018 07:26

Ma27 force-pushed the enable-thumbor-tests branch from 3e69c3c to bcb5eaf Compare November 28, 2018 15:31

Ma27 force-pushed the enable-thumbor-tests branch from bcb5eaf to 0bb5775 Compare November 28, 2018 15:35

GrahamcOfBorg removed the 2.status: merge conflict label Nov 28, 2018

Ma27 approved these changes Nov 28, 2018

View reviewed changes

Ma27 force-pushed the enable-thumbor-tests branch from 0bb5775 to d873a28 Compare November 28, 2018 20:44

jluttine and others added 13 commits November 29, 2018 12:38

pythonPackages.should-dsl: init at 2.1.2

2fe089c

pythonPackages.nose-pattern-exclude: init at 0.1.3

d91e618

pythonPackages.nose-of-yeti: init at 1.8

f5f7f15

pythonPackages.nose-focus: init at 0.1.3

0f81ac7

pythonPackages.yanc: init at 0.3.3

5d2ac2c

pythonPackages.pyres: init at 1.5

735abd8

pythonPackages.pyssim: init at 0.4

866be59

pythonPackages.preggy: init at 1.4.2

c6bd529

pythonPackages.remotecv: init at 2.2.2

fd6e6dc

pythonPackages.cairosvg1: init at 1.0.22

cfaf696

pythonPackages.thumbor: run tests

336f11c

pythonPackages.piexif: 1.0.13 -> 1.1.2

51f6d7f

pythonPackages.pyres: ensure that redis 2.10.6 is used

e2a6e8b

Upstream's requirements.txt wrongly suggests that redis==3.x is support (missing upper-bound constraint), but the tests break due to API incompatibilities.

Ma27 force-pushed the enable-thumbor-tests branch from d873a28 to e2a6e8b Compare November 29, 2018 11:39

Ma27 merged commit a9ffe7f into NixOS:master Nov 29, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

piexif: 1.0.13 -> 1.1.2 (and bunch of other packages) #49409

piexif: 1.0.13 -> 1.1.2 (and bunch of other packages) #49409

jluttine commented Oct 29, 2018

veprbl left a comment

veprbl commented Nov 27, 2018

Ma27 commented Nov 27, 2018

Ma27 left a comment

jluttine commented Nov 28, 2018

Ma27 commented Nov 28, 2018

Ma27 commented Nov 28, 2018

jluttine commented Nov 28, 2018

Ma27 commented Nov 28, 2018 •

edited

Ma27 commented Nov 28, 2018

Ma27 commented Nov 28, 2018

Ma27 commented Nov 28, 2018

GrahamcOfBorg commented Nov 28, 2018

GrahamcOfBorg commented Nov 28, 2018

GrahamcOfBorg commented Nov 28, 2018

Ma27 left a comment

jluttine commented Nov 29, 2018

Ma27 commented Nov 29, 2018

jluttine commented Nov 29, 2018

Ma27 commented Nov 29, 2018

Ma27 commented Nov 29, 2018

piexif: 1.0.13 -> 1.1.2 (and bunch of other packages) #49409

piexif: 1.0.13 -> 1.1.2 (and bunch of other packages) #49409

Conversation

jluttine commented Oct 29, 2018

Motivation for this change

Things done

veprbl left a comment

Choose a reason for hiding this comment

veprbl commented Nov 27, 2018

Ma27 commented Nov 27, 2018

Ma27 left a comment

Choose a reason for hiding this comment

jluttine commented Nov 28, 2018

Ma27 commented Nov 28, 2018

Ma27 commented Nov 28, 2018

jluttine commented Nov 28, 2018

Ma27 commented Nov 28, 2018 • edited

Ma27 commented Nov 28, 2018

Ma27 commented Nov 28, 2018

Ma27 commented Nov 28, 2018

GrahamcOfBorg commented Nov 28, 2018

GrahamcOfBorg commented Nov 28, 2018

GrahamcOfBorg commented Nov 28, 2018

Ma27 left a comment

Choose a reason for hiding this comment

jluttine commented Nov 29, 2018

Ma27 commented Nov 29, 2018

jluttine commented Nov 29, 2018

Ma27 commented Nov 29, 2018

Ma27 commented Nov 29, 2018

Ma27 commented Nov 28, 2018 •

edited