use latest protobuf (3.12), remove overrides where possible #91446

prusnak · 2020-06-24T22:20:03Z

Motivation for this change

The goal of this PR is to set protobuf to the latest available version (currently 3.12) and remove all protobuf overrides where possible. Some of the changes are possible when a package is updated to more recent version, so this PR contains a couple of version updates (in separate commits) too.

Things done

andersk

or-tools looks good. Upstream wants 3.12.2, I’m sure 3.12.3 is fine.

I’m somewhat worried that future or-tools versions may require newer protobuf than our default protobuf again, but if we’re planning to keep the default protobuf up to date now, that shouldn’t be an issue.

marsam · 2020-06-25T00:41:23Z

IIRC tensorflow requires 3.8 #87014 (comment) cc: @bhipple

rnhmjoj

It seems the override is not needed anymore for monero. Just tested and monero{,-gui} builds and runs fine.

pkgs/servers/home-assistant/default.nix

pkgs/top-level/all-packages.nix

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

prusnak · 2020-06-25T08:53:59Z

@dotlambda If you don't have to experiment with home-assistant/esphome/aioesphomeapi changes now, I can remove changes to these packages from this PR and will extract them to a separate PR. Just let me know.

dotlambda · 2020-06-25T09:23:44Z

I can remove changes to these packages from this PR and will extract them to a separate PR.

Please do so. Home Assistant is quite fragile when it comes to its dependencies' versions.

prusnak · 2020-06-25T10:18:33Z

Please do so. Home Assistant is quite fragile when it comes to its dependencies' versions.

OK, removed changes to Home Assistant and its dependencies from this PR. We can discuss this in one of the subsequent PRs.

drewrisinger · 2020-06-25T13:02:04Z

@GrahamcOfBorg build python38Packages.cirq python37Packages.cirq

drewrisinger

Diff on python-packages.nix & cirq.nix LGTM as maintainer. Builds clean via OfBorg

prusnak · 2020-06-27T19:12:04Z

@FRidh or should this go to staging first?

marsam · 2020-07-02T13:30:56Z

pkgs/top-level/python-packages.nix

@@ -5212,7 +5210,7 @@ in {
  protobuf = callPackage ../development/python-modules/protobuf {
    disabled = isPyPy;
    doCheck = !isPy3k;
-    protobuf = pkgs.protobuf3_8;
+    protobuf = pkgs.protobuf3_12;


I think this was added for tensorflow, because it requires protobuf==3.8 #81278 tensorflow/tensorflow#35573
Could you please add an override for tensorflow to use protobuf3_8?

That's not ok because then one can get a closure with multiple versions of the protobuf package. This is unsolvable with Python and a reason why we do not always use the latest version of libraries but instead versions that keep most things working.

I understand the reason, but this makes it impossible to package software that requires newer python-protobuf into nixpkgs.

Yes, in case of libraries, you are correct. We need to balance this. Thus, that also means, if one or a few packages are extremely slow updating, we may need to reconsider whether they should be part of Nixpkgs.

TensorFlow requires protobuf >= 3.8.0, not ==.

#81278 bumped this line from 3.7 to 3.8, but doesn’t seem to indicate that a later version wouldn’t have worked. tensorflow/tensorflow#35573 appears to be an issue with a specific binary distribution.

I see no reason to think it wouldn’t work if everything consistently used the current stable version. Why not protobuf = pkgs.protobuf?

I see no reason to think it wouldn’t work if everything consistently used the current stable version. Why not protobuf = pkgs.protobuf?

That sounds good to me. Care to send a PR trying that for tensorflow?

@bhipple It would have no effect without this PR, because we still have pkgs.protobuf = pkgs.protobuf3_8 in master.

My take is: let's get this into master/staging and fix breakages that might happen. If there are breakages, these should be fixed in upstream, but we'll never discover them if we stick to the old version of protobuf.

1000101 · 2020-07-13T16:09:42Z

Is there any update on this PR? I've just stumbled upon another case where a package can't be upgraded because of old protobuf version.

prusnak · 2020-07-13T16:16:40Z

I did my best here, but the discussion has stalled. I am still in favor of merging this and fixing all potential breakages. I will happily rebase this PR to staging if that's preferred.

prusnak · 2020-07-16T13:21:59Z

Another PR which needs the latest protobuf: #93231

drewrisinger · 2020-07-16T16:48:10Z

I don't know how the staging pipeline works, but that sounds like a decent idea to me. Some things will probably break, but protobuf needs updated.

bhipple · 2020-07-16T23:54:46Z

I'm in favor of merging as long as we can unpin tensorflow to use the latest protobuf consistently with the python libs. It looks like the bound on 3.8 is just a lower bound, so let's go for it?

nixos-discourse · 2020-07-25T10:41:03Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/229

prusnak · 2020-07-26T13:12:05Z

Changed the target branch to staging.

prusnak requested review from FRidh, jonringer and mmahut as code owners June 24, 2020 22:20

prusnak force-pushed the protobuf-latest branch from c8df323 to 4d9c78d Compare June 24, 2020 22:23

ofborg bot added the 6.topic: python label Jun 24, 2020

ofborg bot requested review from dotlambda, rnhmjoj, ehmry, drewrisinger, globin and andersk June 24, 2020 22:31

ofborg bot added 10.rebuild-darwin: 101-500 10.rebuild-linux: 501+ 10.rebuild-linux: 501-1000 labels Jun 24, 2020

andersk approved these changes Jun 24, 2020

View reviewed changes

rnhmjoj approved these changes Jun 25, 2020

View reviewed changes

dotlambda reviewed Jun 25, 2020

View reviewed changes

pkgs/servers/home-assistant/default.nix Outdated Show resolved Hide resolved

dotlambda requested changes Jun 25, 2020

View reviewed changes

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved

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

prusnak force-pushed the protobuf-latest branch from 4d9c78d to aac9145 Compare June 25, 2020 10:18

ofborg bot requested review from andersk and rnhmjoj June 25, 2020 10:26

drewrisinger approved these changes Jun 25, 2020

View reviewed changes

prusnak requested a review from marsam July 1, 2020 14:12

marsam reviewed Jul 2, 2020

View reviewed changes

prusnak mentioned this pull request Jul 16, 2020

Update google-api-core #93231

Closed

10 tasks

prusnak mentioned this pull request Jul 20, 2020

Update default attributes to latest version available #40015

Open

23 tasks

prusnak changed the base branch from master to staging July 26, 2020 13:11

protobuf: make 3.12.x default

5aa7c0c

prusnak force-pushed the protobuf-latest branch from aac9145 to 9314840 Compare July 26, 2020 13:26

prusnak added 3 commits July 26, 2020 15:34

monero: use latest protobuf

c09f880

or-tools: 7.6 -> 7.7

7839270

ola: 0.10.7 -> unstable-2020-07-17

17cac3b

ofborg bot requested a review from drewrisinger July 26, 2020 13:45

python3Packages.cirq: 0.8.0 -> 0.8.2

331b642

prusnak force-pushed the protobuf-latest branch from 9314840 to 331b642 Compare July 26, 2020 13:46

FRidh merged commit d9da3e3 into NixOS:staging Jul 26, 2020

prusnak deleted the protobuf-latest branch July 26, 2020 18:15

andersk mentioned this pull request Aug 8, 2020

pythonPackages.protobuf: Use default version of protobuf #94920

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use latest protobuf (3.12), remove overrides where possible #91446

use latest protobuf (3.12), remove overrides where possible #91446

prusnak commented Jun 24, 2020 •

edited

andersk left a comment

marsam commented Jun 25, 2020

rnhmjoj left a comment

prusnak commented Jun 25, 2020

dotlambda commented Jun 25, 2020

prusnak commented Jun 25, 2020 •

edited

drewrisinger commented Jun 25, 2020

drewrisinger left a comment

prusnak commented Jun 27, 2020

marsam Jul 2, 2020

FRidh Jul 2, 2020

prusnak Jul 2, 2020

FRidh Jul 2, 2020

andersk Jul 4, 2020

bhipple Jul 5, 2020

andersk Jul 5, 2020 •

edited

prusnak Jul 5, 2020 •

edited

1000101 commented Jul 13, 2020

prusnak commented Jul 13, 2020

prusnak commented Jul 16, 2020 •

edited

drewrisinger commented Jul 16, 2020

bhipple commented Jul 16, 2020

nixos-discourse commented Jul 25, 2020

prusnak commented Jul 26, 2020

use latest protobuf (3.12), remove overrides where possible #91446

use latest protobuf (3.12), remove overrides where possible #91446

Conversation

prusnak commented Jun 24, 2020 • edited

Motivation for this change

Things done

andersk left a comment

Choose a reason for hiding this comment

marsam commented Jun 25, 2020

rnhmjoj left a comment

Choose a reason for hiding this comment

prusnak commented Jun 25, 2020

dotlambda commented Jun 25, 2020

prusnak commented Jun 25, 2020 • edited

drewrisinger commented Jun 25, 2020

drewrisinger left a comment

Choose a reason for hiding this comment

prusnak commented Jun 27, 2020

marsam Jul 2, 2020

Choose a reason for hiding this comment

FRidh Jul 2, 2020

Choose a reason for hiding this comment

prusnak Jul 2, 2020

Choose a reason for hiding this comment

FRidh Jul 2, 2020

Choose a reason for hiding this comment

andersk Jul 4, 2020

Choose a reason for hiding this comment

bhipple Jul 5, 2020

Choose a reason for hiding this comment

andersk Jul 5, 2020 • edited

Choose a reason for hiding this comment

prusnak Jul 5, 2020 • edited

Choose a reason for hiding this comment

1000101 commented Jul 13, 2020

prusnak commented Jul 13, 2020

prusnak commented Jul 16, 2020 • edited

drewrisinger commented Jul 16, 2020

bhipple commented Jul 16, 2020

nixos-discourse commented Jul 25, 2020

prusnak commented Jul 26, 2020

prusnak commented Jun 24, 2020 •

edited

prusnak commented Jun 25, 2020 •

edited

andersk Jul 5, 2020 •

edited

prusnak Jul 5, 2020 •

edited

prusnak commented Jul 16, 2020 •

edited