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

python3Packages.cassandra-driver: fix build by packaging new dependencies #100989

Merged
merged 3 commits into from Jan 31, 2021

Conversation

@risicle
Copy link
Contributor

@risicle risicle commented Oct 18, 2020

Motivation for this change

ZHF: #97479

This is heavily based on @turion 's #91662, but I ended up modifying it so much I thought I should just post it separately rather than give him a long list of things to fix. Though I've still left @turion as the maintainer of these - I hope that's ok @turion?

To get NIX_REDIRECTS working I had to add libredirect.so to LD_PRELOAD, and also add a dummy /etc/resolv.conf for some later tests. Also converted to pytestCheckHook and disabled some tests causing trouble for various reasons.

All of the packages had to be switched to the github source because the tests weren't included in any of the pypi tarballs.

gremlinpython's tests do actually run if we override some version constraints and disable some sets of tests which want to connect to a running tinkerpop server.

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.
@turion
Copy link
Contributor

@turion turion commented Oct 19, 2020

@risicle Thanks a lot for picking this up!

@turion turion mentioned this pull request Oct 19, 2020
10 tasks
@risicle risicle force-pushed the ris-turion-cassandra-driver branch 2 times, most recently from 8ef3d69 to 0a21340 Oct 19, 2020
@risicle
Copy link
Contributor Author

@risicle risicle commented Oct 19, 2020

@ofborg build python37Packages.cassandra-driver python38Packages.cassandra-driver

@jonringer
Copy link
Contributor

@jonringer jonringer commented Oct 24, 2020

getting a failure from geomet, there is a 0.2.1.post on pypi

  Processing ./geomet-0.2.1-py3-none-any.whl
  ERROR: Package 'geomet' requires a different Python: 3.8.6 not in '>2.6, !=3.3.*, <3.8'
cannot build derivation '/nix/store/wi330bni0g0ybm6pp0fa8ngvwd2fy78p-python3.8-cassandra-driver-3.24.0.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/zbppwrvd6ynb66c8dx8vm2kq9d6j0wnx-env.drv': 2 dependencies couldn't be built
[8 built (1 failed), 7 copied (1.9 MiB), 0.4 MiB DL]
error: build of '/nix/store/zbppwrvd6ynb66c8dx8vm2kq9d6j0wnx-env.drv' failed
https://github.com/NixOS/nixpkgs/pull/100989
2 packages failed to build:
python38Packages.cassandra-driver python38Packages.geomet

5 packages built:
python27Packages.geomet python37Packages.cassandra-driver python37Packages.geomet python37Packages.gremlinpython python38Packages.gremlinpython

@risicle
Copy link
Contributor Author

@risicle risicle commented Oct 24, 2020

Hmmyes. There's no tagged release for the 3.8 fix, but I might just pull it in as a patch...

@risicle risicle force-pushed the ris-turion-cassandra-driver branch from 0a21340 to 96a82da Oct 24, 2020
@risicle
Copy link
Contributor Author

@risicle risicle commented Oct 24, 2020

@ofborg build python38Packages.cassandra-driver

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Nov 26, 2020

@risicle please solve the merge conflict.

@turion
Copy link
Contributor

@turion turion commented Jan 20, 2021

Ping @risicle. Do you want to continue on this?

@risicle
Copy link
Contributor Author

@risicle risicle commented Jan 20, 2021

Sure, I just have a few things on my TODO list - currently working on a second attempt of #93083

@risicle risicle force-pushed the ris-turion-cassandra-driver branch 2 times, most recently from b3d7e66 to 51147a1 Jan 27, 2021
@ofborg ofborg bot requested a review from turion Jan 27, 2021
@risicle
Copy link
Contributor Author

@risicle risicle commented Jan 27, 2021

Updated, retested macos 10.14, non-nixos linux.

@ofborg build python37Packages.cassandra-driver python38Packages.cassandra-driver python39Packages.cassandra-driver

@risicle
Copy link
Contributor Author

@risicle risicle commented Jan 28, 2021

I don't get it - why didn't you request these changes when you made your first pass? More to the point, how many times do we have to iterate this process until I've satisfied every last one of your personal style preferences? From the fact that I've had this on my TODO list since October, you should see I don't have a lot of time for this stuff, but you'd prefer to keep it bouncing.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Jan 28, 2021

I don't get it - why didn't you request these changes when you made your first pass? More to the point, how many times do we have to iterate this process until I've satisfied every last one of your personal style preferences?

I think we should be good to go after this. I am human and I can always miss something and notice it later. I think that's better than not noticing it at all. Also in the last two months I learned a lot of things and nixpkgs evolved. For example stdenv.lib got deprecated and is no longer allowed to get re-introduced.

From the fact that I've had this on my TODO list since October, you should see I don't have a lot of time for this stuff, but you'd prefer to keep it bouncing.

I can't know if you don't have enough time ,you forgot it or just missed the notification. I let it bounce once to you which I think is reasonable to do especially after two months.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Jan 28, 2021

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 100989 run on x86_64-linux 1

9 packages built:
  • python37Packages.cassandra-driver
  • python37Packages.geomet
  • python37Packages.gremlinpython
  • python38Packages.cassandra-driver
  • python38Packages.geomet
  • python38Packages.gremlinpython
  • python39Packages.cassandra-driver
  • python39Packages.geomet
  • python39Packages.gremlinpython

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Jan 28, 2021

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 100989 run on x86_64-darwin 1

9 packages built:
  • python37Packages.cassandra-driver
  • python37Packages.geomet
  • python37Packages.gremlinpython
  • python38Packages.cassandra-driver
  • python38Packages.geomet
  • python38Packages.gremlinpython
  • python39Packages.cassandra-driver
  • python39Packages.geomet
  • python39Packages.gremlinpython

@risicle risicle force-pushed the ris-turion-cassandra-driver branch from 51147a1 to 04a96e3 Jan 30, 2021
@risicle
Copy link
Contributor Author

@risicle risicle commented Jan 30, 2021

My point is that being overly picky about stylistic things really just puts off contributors. If something straightforward (like perhaps deciding to upstream one of your team's fixes after work for the good of the community) is going to become a multiple week saga bouncing back and forth talking about newlines and semi-colons, people just aren't going to bother because they have other things to do that feel a lot more productive.

@turion
Copy link
Contributor

@turion turion commented Jan 30, 2021

My point is that being overly picky about stylistic things really just puts off contributors. If something straightforward (like perhaps deciding to upstream one of your team's fixes after work for the good of the community) is going to become a multiple week saga bouncing back and forth talking about newlines and semi-colons, people just aren't going to bother because they have other things to do that feel a lot more productive.

I agree with that. Getting a PR into nixpkgs is so much work already, even for smaller contributions. Formatting issues should be addressed by an automatic formatter. They shouldn't cost our precious human's time.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Jan 31, 2021

My point is that being overly picky about stylistic things really just puts off contributors.

I am not overly picky. Most of the requests are to have a somewhat uniform style to make treewide changes in the future way easier by not having to many variants of essentially the same thing.

I did not order your inputs or sort your phases. Run nixpkgs-hammer on your package and you should see what overly picky looks like.

Getting a PR into nixpkgs is so much work already

It isn't if you follow the contributing guide. Take a look at fabaff PRs since December and a lot of them got merged without any review comments or only very small changes.

Formatting issues should be addressed by an automatic formatter.

There is no formatter that is one stop and does everything. nixpkgs-hammering and nix-fmt should be a good option but they do not cover everything. I am not familiar with programming formatters so I can't improve the situation in a meaningful amount of time.

@turion
Copy link
Contributor

@turion turion commented Jan 31, 2021

It isn't if you follow the contributing guide.

@SuperSandro2000 What exactly is the contributing guide? https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md is the only thing I know in that direction, and it doesn't give me enough information to submit PRs that can just be merged as-is. I'd like to learn more.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Jan 31, 2021

There is also https://github.com/NixOS/nixpkgs/tree/master/doc/contributing and the language specific doc.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Jan 31, 2021

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 100989 run on x86_64-linux 1

9 packages built:
  • python37Packages.cassandra-driver
  • python37Packages.geomet
  • python37Packages.gremlinpython
  • python38Packages.cassandra-driver
  • python38Packages.geomet
  • python38Packages.gremlinpython
  • python39Packages.cassandra-driver
  • python39Packages.geomet
  • python39Packages.gremlinpython

@SuperSandro2000 SuperSandro2000 merged commit ba64d09 into NixOS:master Jan 31, 2021
18 checks passed
18 checks passed
@github-actions[bot]
tests
Details
@github-actions[bot]
action
Details
@ofborg[bot]
Evaluation Performance Report Evaluator Performance Report
Details
@github-actions[bot]
Wait for ofborg
Details
@ofborg[bot]
grahamcofborg-eval ^.^!
Details
@ofborg[bot]
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
@ofborg[bot]
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
@ofborg[bot]
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="04a96e3"; rev="04a96e3c8b92a7beb488fb39b0ac830d7523679f"; } ./pkgs/t
Details
@ofborg[bot]
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
@ofborg[bot]
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="04a96e3"; rev="04a96e3c8b92a7beb488fb39b0ac830d7523679f"; } ./nixos/
Details
@ofborg[bot]
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="04a96e3"; rev="04a96e3c8b92a7beb488fb39b0ac830d7523679f"; } ./nixos/
Details
@ofborg[bot]
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="04a96e3"; rev="04a96e3c8b92a7beb488fb39b0ac830d7523679f"; } ./nixos/
Details
@ofborg[bot]
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="04a96e3"; rev="04a96e3c8b92a7beb488fb39b0ac830d7523679f"; } ./pkgs/t
Details
@ofborg[bot]
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="04a96e3"; rev="04a96e3c8b92a7beb488fb39b0ac830d7523679f"; } ./pkgs/t
Details
@ofborg[bot]
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="04a96e3"; rev="04a96e3c8b92a7beb488fb39b0ac830d7523679f"; } ./pkgs/t
Details
@ofborg[bot]
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
@ofborg[bot]
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@ofborg[bot]
ofborg-eval-started Evaluation started
Details
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

4 participants