google-cloud-sdk: disable gce as a dependency by default (fixes #31369) #31709
Conversation
| let | ||
| pythonInputs = [ cffi cryptography pyopenssl crcmod google-compute-engine ]; | ||
| gcePackages = if forGceInstance then [ google-compute-engine ] else []; | ||
| pythonInputs = [ cffi cryptography pyopenssl crcmod ] ++ gcePackages; |
There was a problem hiding this comment.
I'd rather see [ cffi cryptography pyopenssl crcmod ] ++ lib.optional (!stdenv.isDarwin) google-compute-engine than introduce overrides and overlays. I don't think that the closure size matters that much in this context.
There was a problem hiding this comment.
Well, in this case installing google-cloud-sdk from nixpkgs would install systemd libs as it is dependency to google-compute-engine. IMO, it is hardly a sane choice, especially for non-NixOS users.
There was a problem hiding this comment.
I reiterate that the package google-compute-engine (coming from compute-image-packages repo) is only meant for GCE guests. Installing it everywhere just in case seems too much to me.
There was a problem hiding this comment.
In that case let's make two top-level packages, google-cloud-sdk and google-cloud-sdk-gce. Changing the overlay of the GCE image profile seems a bit too much.
There was a problem hiding this comment.
Can't we just add (google-cloud-sdk.override { gceClient = true; } into environment.systemPackages? I think that covers most of SDK usage without much complexity, and we can skip adding an additional package too (don't have a strong opinion here).
There are also potential users of SDK as a library like TensorFlow -- I think it supports transparently using Google Cloud Storage when it can find both google-cloud-sdk and google-cloud-engine but I didn't try that; let's leave this usecase be for a time.
There was a problem hiding this comment.
@abbradar If one puts overriden google-cloud-sdk in systemPackages it won't change the fact that all packages using google-cloud-sdk (e.g. TensorFlow) will use the default version (without gce).
An overlay for GCE would override the default version of google-cloud-sdk and this is exactly why I added it. The packages built against pkgs with this overlay (e.g. on a default NixOS GCE image) would then use gce-enabled google-cloud-sdk. The problem is that all packages that use google-cloud-sdk need to be built with two different versions of it if one want the binary cache to be available both on usual and GCE installations.
|
Agreed.
|
1pakch
force-pushed
the
google-cloud-sdk-fix
branch
from
258620c
to
f4c1bcd
Compare
|
I dropped the overlay and added a top level package with gce-enabled as per @zimbatm suggestion. The last commit adds the |
|
BTW, I rewrote the history in my private branch so the initial PR is not reachable. Is that OK, or it would have been better to revert? |
|
Thanks @ilya-kolpakov ! |
Motivation for this change
The commit b241bcf made
google-compute-enginea dependency forgoogle-cloud-sdkwhich make it impossible to buildgoogle-cloud-sdkon MacOS (#31679). In general,google-compute-engineis a package intended for GCE guest images andgoogle-cloud-sdkneedsgoogle-compute-engineonly when running on GCE.This commit introduces a new attribute
forGceInstancewhich controls whethergoogle-compute-engineis included as a dependency and isfalseby default. Moreover, this commit includes the override for this package withforGceInstanceset to true in the default NixOS configuration for GCE at nixos/modules/virtualisation/google-compute-image.nix.Maintainers @stephenmw @zimbatm
Things done
nix-shell -p nox --run "nox-review wip"./result/bin/)