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

rethinkdb: 2.3.6 -> 2.4.0, fix #87828

Open
wants to merge 3 commits into
base: master
from
Open

Conversation

@matthias-t
Copy link
Contributor

matthias-t commented May 14, 2020

Motivation for this change

The rethinkdb package for the RethinkDB database server fails to build and is marked as broken. The python2.pkgs.rethinkdb package for the python client errors because of a missing dependency. This fixes the two packages. Please see the commit messages for details.

Also, I changed the license meta fields as the codebase was relicensed to Apache 2.0 after ownership was transferred to the Linux foundation.

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.
@matthias-t matthias-t requested review from FRidh and jonringer as code owners May 14, 2020
@ofborg ofborg bot added the 6.topic: python label May 14, 2020
@ofborg ofborg bot requested review from bluescreen303 and thoughtpolice May 14, 2020
@matthias-t matthias-t changed the title Fix RethinkDB build rethinkdb: 2.3.6 -> 2.4.0, fix May 21, 2020
@nixos-discourse
Copy link

nixos-discourse commented May 21, 2020

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

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/164

@matthias-t matthias-t mentioned this pull request May 26, 2020
2 of 10 tasks complete
pkgs/development/python-modules/rethinkdb/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/rethinkdb/default.nix Outdated Show resolved Hide resolved
sha256 = "05nagixlwnq3x7441fhll5vs70pxppbsciw8qjqp660bdb5m4jm1";
})
];
patches = [ ./v8-no-snapshot.patch ];

This comment has been minimized.

Copy link
@jonringer

jonringer May 29, 2020

Contributor

do you mind adding a comment about why this patch is necessary?

This comment has been minimized.

Copy link
@matthias-t

matthias-t May 29, 2020

Author Contributor

Will that suffice? I'm not certain how exactly RethinkDB uses V8 or why the snapshots segfault. When I removed the failing patch, which originally disabled V8 snapshots, I noticed that making V8 snapshots would still segfault the build, so I adapted it for the most recent version of RethinkDB, upon which the build succeeded. But I'm unsure I understand what's going on. The commit message of the original patch is "Workaround for building V8 with GCC 6.2".

This comment has been minimized.

Copy link
@matthias-t

matthias-t May 29, 2020

Author Contributor

Maybe we could ask @pbogdan? 26ebbac

This comment has been minimized.

Copy link
@matthias-t

matthias-t May 29, 2020

Author Contributor

The associated PR mentions this issue upstream rethinkdb/rethinkdb#5757, which was closed with mandating CXX=clang++.

This comment has been minimized.

Copy link
@matthias-t

matthias-t May 29, 2020

Author Contributor

OK, this builds perfectly using clangStdenv, so the latest commit switches to that. Let me know if for some reason you prefer using GCC with the patch.

@matthias-t matthias-t force-pushed the matthias-t:rethinkdb branch 2 times, most recently from 4066837 to ecf94fc May 29, 2020
@@ -36,6 +31,8 @@ stdenv.mkDerivation rec {
"--lib-path=${jemalloc}/lib"
];

makeFlags = [ "rethinkdb" ];

This comment has been minimized.

Copy link
@matthias-t

matthias-t May 29, 2020

Author Contributor

Fix build by building only the database server. Other make targets fetch
dependencies at build time and this behaviour cannot be overriden.
Therefore, the clients and web interface are no longer built. See
rethinkdb/rethinkdb#6867.

Is this ok?

matthias-t added 3 commits May 14, 2020
Add missing dependencies on the `six` and `setuptools` python packages.
Update patch that prevents making V8 snapshots, as those segfault.

Fix build by building only the database server. Other make targets fetch
dependencies at build time and this behaviour cannot be overriden.
Therefore, the clients and web interface are no longer built. See
rethinkdb/rethinkdb#6867.
And remove patch working around a GCC bug.
@matthias-t matthias-t force-pushed the matthias-t:rethinkdb branch from ecf94fc to 6fd1d59 May 31, 2020
@matthias-t matthias-t requested a review from jonringer May 31, 2020
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

3 participants
You can’t perform that action at this time.