couchdb: add support for version 2.0.0 #22897

timor · 2017-02-17T11:16:18Z

Motivation for this change

Support 2.0.0 CouchDB release

Version 2.0.0 is installed as a separate package called "couchdb2".
When setting the config option "package" attribute to pkgs.couchdb2, a
corresponding service configuration will be generated. If a previous
1.6 installation exists, the databases can still be found on the local
port (default: 5986) and can be replicated from there.

Note that single-node or cluster setup still needs to be configured
manually, as described in
http://docs.couchdb.org/en/2.0.0/install/index.html.

Things done

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

mention-bot · 2017-02-17T11:16:20Z

@timor, thanks for your PR! By analyzing the history of the files in this pull request, we identified @garbas, @domenkozar and @offlinehacker to be potential reviewers.

timor · 2017-02-17T11:21:19Z

@LnL7 ping

globin · 2017-02-17T11:26:54Z

nixos/modules/services/databases/couchdb.nix

@@ -4,20 +4,29 @@ with lib;

 let
  cfg = config.services.couchdb;
-  configFile = pkgs.writeText "couchdb.ini"
+  useVersion2 = strings.hasPrefix "2" (builtins.parseDrvName cfg.package.name).version;


There is versionOlder and versionAtLeast in lib. (https://github.com/NixOS/nixpkgs/blob/master/lib/strings.nix#L343-L363)

Using versionAtLeast would include any later versions, not only 2.x builds. Is that a problem?

globin · 2017-02-17T11:27:30Z

nixos/modules/services/databases/couchdb.nix

@@ -151,6 +160,9 @@ in {
      description = "CouchDB Server";
      wantedBy = [ "multi-user.target" ];

+      # erlang crashes without sh in path
+      path = mkIf useVersion2 [ pkgs.bash ];


Is it possible to wrap the binary to have it in path?

globin · 2017-02-17T11:27:55Z

nixos/modules/services/databases/couchdb.nix

@@ -170,11 +182,20 @@ in {
        fi
        '';

+      environment = mkIf useVersion2 {
+	# we are actually specifying 4 configuration files:


indentation is off

globin · 2017-02-17T11:29:14Z

pkgs/servers/http/couchdb/2.0.0.nix

+
+  '';
+
+  # HACK: this should probably not be hardcoded...


Why is this needed?

globin · 2017-02-17T11:56:39Z

pkgs/servers/http/couchdb/2.0.0.nix

+
+  buildInputs = [ erlang icu openssl spidermonkey ];
+
+  phases = ["unpackPhase" "patchPhase" "configurePhase" "buildPhase" "installPhase"];


You don't need to specify the phases. Why isn't fixupPhase being run?

globin · 2017-02-17T11:57:11Z

Some minor remarks but looks good otherwise

LnL7 · 2017-02-17T19:39:12Z

pkgs/servers/http/couchdb/2.0.0.nix

+    >                 {"-DHAVE_CURL -I/usr/local/include ''$NIX_CFLAGS_COMPILE", "-DHAVE_CURL -lmozjs185 -lcurl"};
+    EOF
+
+    patch bin/rebar <<EOF


can you use substituteInPlace for this?

Can I use that to only patch that exact line?

used substituteInPlace for the CFLAGs in the rebar script

LnL7 · 2017-02-17T19:43:46Z

nixos/modules/services/databases/couchdb.nix

@@ -130,7 +139,7 @@ in {

      configFile = mkOption {
        type = types.string;
-        default = "/var/lib/couchdb/couchdb.ini";
+        default = if useVersion2 then "/var/lib/couchdb/local.ini" else "/var/lib/couchdb/couchdb.ini";


I think it's preferred to use mkDefault in the config section for dynamic defaults like this.

I wanted to use that, but in this case the default depends on another config option. Is that possible?

ahhh nevermind, wrong option.

I mean something like this

config = { services.couchdb.configFile = mkDefault (if useVersion2 then "/var/lib/couchdb/local.ini" else "/var/lib/couchdb/couchdb.ini"); };

LnL7 · 2017-02-17T19:48:29Z

pkgs/top-level/all-packages.nix

+  couchdb2 = callPackage ../servers/http/couchdb/2.0.0.nix {
+    spidermonkey = spidermonkey_185;
+    python = python27;
+    erlang = erlangR16;


This seems rather old, it only works with R16?

The documentation mentions Erlang OTP (>=R61B03, =<19.x)

timor · 2017-02-23T13:37:59Z

@globin should I squash all the commits into one and force-push them to this PR?

offlinehacker · 2017-03-05T00:46:58Z

Yes, please do that.

Version 2.0.0 is installed as a separate package called "couchdb2". When setting the config option "package" attribute to pkgs.couchdb2, a corresponding service configuration will be generated. If a previous 1.6 installation exists, the databases can still be found on the local port (default: 5986) and can be replicated from there. Note that single-node or cluster setup still needs to be configured manually, as described in http://docs.couchdb.org/en/2.0.0/install/index.html.

timor · 2017-03-06T10:46:04Z

I think I addressed everything.

timor · 2017-03-14T10:44:35Z

@globin, @LnL7, @offlinehacker:

Is there anything left for me to change on this before it can be merged?

LnL7

Is there a test for couchdb, if so could you add one for couchdb2?

LnL7 · 2017-03-14T20:35:35Z

pkgs/servers/http/couchdb/2.0.0.nix

+  postPatch = ''
+    substituteInPlace src/couch/rebar.config.script --replace '-DHAVE_CURL -I/usr/local/include' "-DHAVE_CURL -I/usr/local/include $NIX_CFLAGS_COMPILE"
+
+    patch bin/rebar <<EOF


I think this also works

substituteInPlace bin/rebase \ --replace '#!/usr/bin/env' '#!${coreutils}/bin/env'

It does not work. "bin/rebar" is basically a bytecode archive file with a shebang in front to call the interpreter. For some reason that I could not debug, using substituteInPlace does not work in the same way as using patch, and causes the resulting file to be not executable correctly. For that reason, I kept it that way.

Ok. I'm pretty sure I asked that before 😄 .

timor · 2017-03-15T11:55:34Z

@LnL7 Sadly, I am not familiar with tests. I did not see one for couchdb in nixos/tests, and there is no checkPhase in the original package either.

globin requested changes Feb 17, 2017

View reviewed changes

LnL7 reviewed Feb 17, 2017

View reviewed changes

LnL7 added the 8.has: module (update) label Feb 17, 2017

timor force-pushed the couchdb-2.0.0 branch from 579a088 to f40b961 Compare March 6, 2017 10:44

LnL7 reviewed Mar 14, 2017

View reviewed changes

LnL7 approved these changes Mar 16, 2017

View reviewed changes

LnL7 merged commit 00ed0f7 into NixOS:master Mar 16, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

couchdb: add support for version 2.0.0 #22897

couchdb: add support for version 2.0.0 #22897

timor commented Feb 17, 2017

mention-bot commented Feb 17, 2017

timor commented Feb 17, 2017

globin Feb 17, 2017

timor Feb 20, 2017 •

edited

globin Feb 17, 2017

globin Feb 17, 2017

globin Feb 17, 2017

globin Feb 17, 2017

globin commented Feb 17, 2017

LnL7 Feb 17, 2017

timor Feb 20, 2017

timor Feb 23, 2017

LnL7 Feb 17, 2017

timor Feb 17, 2017

timor Feb 17, 2017

LnL7 Feb 17, 2017

LnL7 Feb 17, 2017

LnL7 Feb 17, 2017

timor commented Feb 23, 2017

offlinehacker commented Mar 5, 2017

timor commented Mar 6, 2017

timor commented Mar 14, 2017

LnL7 left a comment

LnL7 Mar 14, 2017

timor Mar 15, 2017

LnL7 Mar 16, 2017

timor commented Mar 15, 2017


		buildInputs = [ erlang icu openssl spidermonkey ];

		phases = ["unpackPhase" "patchPhase" "configurePhase" "buildPhase" "installPhase"];

couchdb: add support for version 2.0.0 #22897

couchdb: add support for version 2.0.0 #22897

Conversation

timor commented Feb 17, 2017

Motivation for this change

Things done

mention-bot commented Feb 17, 2017

timor commented Feb 17, 2017

Choose a reason for hiding this comment

timor Feb 20, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

globin commented Feb 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timor commented Feb 23, 2017

offlinehacker commented Mar 5, 2017

timor commented Mar 6, 2017

timor commented Mar 14, 2017

LnL7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timor commented Mar 15, 2017

timor Feb 20, 2017 •

edited