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

python.pkgs.google_cloud_storage: propagate setuptools #70900

Merged
merged 6 commits into from Oct 16, 2019

Conversation

flokli
Copy link
Contributor

@flokli flokli commented Oct 10, 2019

This is used during runtime.

cc #68314

Motivation for this change

python.pkgs.google_cloud_storage requires setuptools at runtime.
This looks like a pattern used in other parts of the google bindings aswell, so we might want to fix those too / cc @costrouc.

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 nix-review --run "nix-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.
Notify maintainers

cc @costrouc

@flokli flokli requested a review from costrouc October 10, 2019 09:06
@flokli flokli requested a review from FRidh as a code owner October 10, 2019 09:06
@worldofpeace
Copy link
Contributor

python.pkgs.google_cloud_storage requires setuptools at runtime.
This looks like a pattern used in other parts of the google bindings aswell, so we might want to fix those too

@flokli Whatever depends on google_cloud_storage that also needs setuptools will have this requirement satiated transitively because it's propagated here. So likely we won't notice it being an issue. Can we fix them all here?

@flokli flokli force-pushed the google_cloud_storage-setuptools branch from 04f48c9 to 789565d Compare October 15, 2019 22:07
@flokli
Copy link
Contributor Author

flokli commented Oct 15, 2019

hrm, it's not really inspectable.

@FRidh I wonder if we could add some script in fixup phase warning/failing on non-progated setuptools, but code like pkg_resources.declare_namespace(__name__) present.

But out of the scope for this PR.

I looked at the python google package dependencies of google_cloud_storage (only those, not all, as it's a lot), and added setuptools manually where used (it was needed everywhere).

@FRidh
Copy link
Member

FRidh commented Oct 16, 2019

We could create a hook that greps on pkg_resources and verifies if setuptools is in $out/nix-support/propagated-build-inputs. Then we could run a single job with this hook added to buildPython*.

@worldofpeace
Copy link
Contributor

@FRidh Thinking the same. Should be good and we could probably get them all in one go.

@FRidh
Copy link
Member

FRidh commented Oct 16, 2019

It's a pity there are so more many packages that use pkg_resources than I initially expected. At this point I am actually considering just adding setuptools again (on stable).

Next couple of days I'll be occupied with #71222. After that, could run a run with the proposed hook.

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

4 participants