meguca: init at 2018-05-17 #39482

Chiiruno · 2018-04-25T13:18:36Z

Motivation for this change

meguca is a great piece of software, and I thought I'd make a package for it.

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
- 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 nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

Chiiruno · 2018-04-25T13:20:18Z

That said, putting together this package I'm pretty sure there are things in it that can be done better.
If you know any ways to improve it and make it "safer", please do tell.

If there's a way to include cmake in the buildinputs without it autofiring cmake, I'd like to know so that I can add an option to compile the WASM (optional) client, that uses emscripten.

I also can't do the checkPhase because the tests require a postgresql connection and temporary database, in which I don't have the permissions. Is there a way to be able to do the tests?

Chiiruno · 2018-04-25T13:34:29Z

pkgs/servers/meguca/default.nix

+    export HOME=$PWD/.home
+    export GOPATH=$HOME/go
+    export GIT_SSL_CAINFO=/etc/ssl/certs/ca-certificates.crt
+    export SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt


I'd like especially if someone could tell me a better way to do this.
I'm not familiar with buildGoPackage and couldn't find a way to make it work with the way meguca's build flow goes.

Mic92 · 2018-04-28T10:58:26Z

pkgs/servers/meguca/default.nix

+    url = "https://github.com/bakape/meguca.git";
+    rev = "4961e76743fc22c002fcf8f1bc8f77bbe73b2623";
+    sha256 = "055m323s5vki6dg77mm8sfj88ymh46bjckx8w474gyx5q59jprgp";
+    fetchSubmodules = false;


If you are not using fetchSubmodules, you can use fetchFromGitHub as well.

Mic92 · 2018-04-28T11:01:09Z

dontUseCmakeConfigure can be set to avoid the cmake hook: https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/tools/build-managers/cmake/setup-hook.sh#L92

Mic92 · 2018-04-28T11:03:41Z

@GrahamcOfBorg build meguca

GrahamcOfBorg · 2018-04-28T11:04:38Z

Failure on x86_64-linux (full log)

Attempted: meguca

Partial log (click to expand)

fatal: unable to access 'https://github.com/jteeuwen/go-bindata/': Could not resolve host: github.com
package github.com/jteeuwen/go-bindata/...: exit status 128
github.com/mailru/easyjson (download)
# cd .; git clone https://github.com/mailru/easyjson /build/meguca-4961e76/.home/go/src/github.com/mailru/easyjson
Cloning into '/build/meguca-4961e76/.home/go/src/github.com/mailru/easyjson'...
fatal: unable to access 'https://github.com/mailru/easyjson/': Could not resolve host: github.com
package github.com/mailru/easyjson/...: exit status 128
make: *** [Makefile:61: generate] Error 1
builder for '/nix/store/9yb0qwrknqx4ry1sqm9clplwbnwbdip2-meguca-unstable-2018-04-25.drv' failed with exit code 2
error: build of '/nix/store/9yb0qwrknqx4ry1sqm9clplwbnwbdip2-meguca-unstable-2018-04-25.drv' failed

GrahamcOfBorg · 2018-04-28T11:06:22Z

Failure on aarch64-linux (full log)

Attempted: meguca

Partial log (click to expand)

npm ERR! network In most cases you are behind a proxy or have bad network settings.
npm ERR! network
npm ERR! network If you are behind a proxy, please make sure that the
npm ERR! network 'proxy' config is set properly.  See: 'npm help config'

npm ERR! Please include the following file with any support request:
npm ERR!     /build/meguca-4961e76/npm-debug.log
make: *** [Makefile:31: client_deps] Error 1
builder for '/nix/store/8ql02pxvrb20jw6h3qhqz4zn88aj4kjk-meguca-unstable-2018-04-25.drv' failed with exit code 2
�[31;1merror:�[0m build of '/nix/store/8ql02pxvrb20jw6h3qhqz4zn88aj4kjk-meguca-unstable-2018-04-25.drv' failed

GrahamcOfBorg · 2018-04-28T11:11:23Z

Failure on x86_64-darwin (full log)

Attempted: meguca

Partial log (click to expand)

+-- gulp-typescript@3.2.4
+-- gulp-uglify@3.0.0
+-- gulp-util@3.0.8
+-- proxy-polyfill@0.1.7
+-- typescript@2.7.2
+-- uglify-es@3.3.10
`-- whatwg-fetch@2.0.4

builder for '/nix/store/df0s5qwj3gfhnaajbfmndl9scqjlfjlr-meguca-unstable-2018-04-25.drv' failed with exit code 2
error: build of '/nix/store/df0s5qwj3gfhnaajbfmndl9scqjlfjlr-meguca-unstable-2018-04-25.drv' failed

Chiiruno · 2018-04-29T02:11:22Z

Pending a review and a hydra build, I believe this is ready to be pulled.

Chiiruno · 2018-05-03T23:30:16Z

This (may) not build until #39955 is fixed.

Chiiruno · 2018-05-05T16:11:03Z

I just realized that I've been using the sandboxing wrong, I'll commit a fix and squash the merge soon.

Chiiruno · 2018-05-18T00:50:07Z

@Mic92 Hopefully this is the last time I ping you on this pull request.
It seems to be all good.

Mic92 · 2018-05-20T14:27:51Z

nixos/modules/services/web-servers/meguca.nix

+        WorkingDirectory = "${cfg.baseDir}";
+        ExecStart = ''${pkgs.meguca}/bin/meguca${if cfg.reverseProxy != null then " -R ${cfg.reverseProxy}" else ""}${if cfg.sslCertificate != null then " -S ${cfg.sslCertificate}" else ""}${if cfg.listenAddress != null then " -a ${cfg.listenAddress}" else ""}${if cfg.cacheSize != null then " -c ${cfg.cacheSize}" else ""}${if cfg.postgresArgs != null then " -d  ${cfg.postgresArgs}" else ""}${if cfg.compressTraffic then " -g" else ""}${if cfg.assumeReverseProxy then " -r" else ""}${if cfg.httpsOnly then " -s" else ""} start'';
+        ExecStop = "${pkgs.meguca}/bin/meguca stop";
+        ExecRestart = "${pkgs.meguca}/bin/meguca restart";


There is no explicit restart in systemd.

Mic92 · 2018-05-20T14:29:32Z

nixos/modules/services/web-servers/meguca.nix

+        User = "meguca";
+        Group = "meguca";
+        WorkingDirectory = "${cfg.baseDir}";
+        ExecStart = ''${pkgs.meguca}/bin/meguca${if cfg.reverseProxy != null then " -R ${cfg.reverseProxy}" else ""}${if cfg.sslCertificate != null then " -S ${cfg.sslCertificate}" else ""}${if cfg.listenAddress != null then " -a ${cfg.listenAddress}" else ""}${if cfg.cacheSize != null then " -c ${cfg.cacheSize}" else ""}${if cfg.postgresArgs != null then " -d  ${cfg.postgresArgs}" else ""}${if cfg.compressTraffic then " -g" else ""}${if cfg.assumeReverseProxy then " -r" else ""}${if cfg.httpsOnly then " -s" else ""} start'';


Can you break this into multiple lines for readability? You could make it a list that is joined by a spaces.
Also use lib.optionalString to add optional parameter.

Mic92 · 2018-05-20T14:39:05Z

nixos/modules/services/web-servers/meguca.nix

+    password = mkOption {
+      type = types.str;
+      default = "meguca";
+      description = "Password for the meguca database.";


The password is stored in clear text in nix readable for all users. We usually have at least an alternative option that stored the password in a file outside the nix store: #24288
Btw where is this password used in the service?

If meguca also accepts unix sockets for database connections, authentication could be also done via user without any password.
I would not hardcode meguca as default password and rather let the user choose.

meguca is hard coded into upstream itself as the default, however with postgresArgs you can define the password to access the database with, in which is set each service start with password field.

Mic92

The packaging side seems fine. I was not able to the test the service fully yet since the database password is not set in meguca.

Chiiruno · 2018-05-21T00:59:28Z

@Mic92 How is this?

Mic92 · 2018-05-21T10:26:52Z

nixos/modules/services/web-servers/meguca.nix

+${optionalString (cfg.assumeReverseProxy) " -r"}\
+${optionalString (cfg.httpsOnly) " -s"} start
+rm -f /run/meguca/start
+EOF


I think you accidentally dropped ExecStart from the service.
You can also use the script property instead of writing your own shell script.

script = '' exec ${pkgs.meguca}/bin/meguca -d "$(cat ${cfg.postgresArgsFile})"\ ${optionalString (cfg.reverseProxy != null) " -R ${cfg.reverseProxy}"}\ ${optionalString (cfg.sslCertificate != null) " -S ${cfg.sslCertificate}"}\ ${optionalString (cfg.listenAddress != null) " -a ${cfg.listenAddress}"}\ ${optionalString (cfg.cacheSize != null) " -c ${toString cfg.cacheSize}"}\ ${optionalString (cfg.compressTraffic) " -g"}\ ${optionalString (cfg.assumeReverseProxy) " -r"}\ ${optionalString (cfg.httpsOnly) " -s"} start '';

It works the same way preStart works.

Update just saw the script reference above. This can be just inlined, no need to create a separate script.

Mic92 · 2018-05-21T10:30:45Z

nixos/modules/services/web-servers/meguca.nix

        Type = "forking";
-        User = "meguca";
-        Group = "meguca";


Why did you removed the meguca user here?

The meguca user is no longer necessary, since it's running in /run/.
Actually, it was never really necessary, I don't know why I added it to begin with.
If it's part of some standard, I'll re-add.

Actually never mind, I'll re-add it, since being able to configure the directory in which meguca starts in would be useful.

But then the server would run as root, right? If it can be avoided network-facing services should not run as root.

Chiiruno · 2018-05-21T12:39:06Z

@Mic92 Ok, that should do it.
Diff for convenience.

Mic92 · 2018-05-27T22:06:25Z

nixos/modules/services/web-servers/meguca.nix

+    script = ''
+      cd ${cfg.baseDir}
+
+      ${pkgs.meguca}/bin/meguca -d "$(cat ${cfg.postgresArgsFile})"\


Too bad the password is not cleared in meguca and visible to any other process such as top:

/nix/store/7l3naj8wdpg9akp8bjps4nwl863nb216-meguca-unstable-2018-05-26-bin/bin/meguca -d user=meguca password=meguca dbname=meguca sslmode=disable start

This is an upstream issue however and should not be fixed in this pull request.

tracked here: bakape/shamichan#729

GrahamcOfBorg added 6.topic: nixos 8.has: module (update) 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Apr 25, 2018

Chiiruno force-pushed the init/meguca branch from 42b4295 to 0d5a347 Compare April 25, 2018 13:33

Chiiruno commented Apr 25, 2018

View reviewed changes

Chiiruno force-pushed the init/meguca branch from bc452e5 to d68b1b0 Compare April 26, 2018 12:32

Mic92 reviewed Apr 28, 2018

View reviewed changes

Chiiruno force-pushed the init/meguca branch from d68b1b0 to 7eb466c Compare April 29, 2018 01:49

Chiiruno changed the title ~~meguca: init at git-2018-04-25~~ meguca: init at git-2018-04-28 Apr 29, 2018

Chiiruno force-pushed the init/meguca branch 3 times, most recently from 87bd352 to 1316d44 Compare May 1, 2018 20:49

Chiiruno changed the title ~~meguca: init at git-2018-04-28~~ meguca: init at git-2018-05-01 May 1, 2018

GrahamcOfBorg added 2.status: merge conflict and removed 2.status: merge conflict labels May 1, 2018

Chiiruno force-pushed the init/meguca branch 2 times, most recently from 9c9a232 to 64a7f95 Compare May 1, 2018 21:19

Chiiruno force-pushed the init/meguca branch from 64a7f95 to d5cd665 Compare May 4, 2018 01:29

Chiiruno changed the title ~~meguca: init at git-2018-05-01~~ meguca: init at git-2018-05-03 May 4, 2018

Chiiruno force-pushed the init/meguca branch from 168a363 to 614b7d5 Compare May 18, 2018 00:36

Chiiruno changed the title ~~meguca: init at git-2018-05-05~~ meguca: init at git-2018-05-17 May 18, 2018

GrahamcOfBorg added 10.rebuild-linux: 11-100 and removed 10.rebuild-linux: 1-10 labels May 18, 2018

Mic92 changed the title ~~meguca: init at git-2018-05-17~~ meguca: init at 2018-05-17 May 20, 2018

Mic92 reviewed May 20, 2018

View reviewed changes

Mic92 requested changes May 20, 2018

View reviewed changes

Chiiruno force-pushed the init/meguca branch from 0ebb093 to 8c2932f Compare May 21, 2018 00:58

Mic92 reviewed May 21, 2018

View reviewed changes

Chiiruno force-pushed the init/meguca branch 2 times, most recently from 436678d to 2d0e613 Compare May 21, 2018 12:35

Madouura added 6 commits May 26, 2018 07:02

easyjson: init at git-2018-03-23

6ed5195

quicktemplate: init at git-2018-04-30

c989dd5

nodePackages: meguca: init at git-2018-05-17

934db65

meguca: init at git-2018-05-17

14a26f0

meguca: git-2018-05-17 -> git-2018-05-20

e2f1a05

meguca: 2018-05-20 -> 2018-05-25

79beebe

Chiiruno force-pushed the init/meguca branch from 67609ad to 79beebe Compare May 26, 2018 12:07

meguca: 2018-05-25 -> 2018-05-26

6ba3b2f

Chiiruno force-pushed the init/meguca branch from a31f752 to 6ba3b2f Compare May 26, 2018 13:16

Mic92 reviewed May 27, 2018

View reviewed changes

Mic92 merged commit 06fe322 into NixOS:master May 27, 2018

Mic92 mentioned this pull request May 27, 2018

Database password should be not visible to other processes bakape/shamichan#729

Closed

Chiiruno deleted the init/meguca branch May 27, 2018 22:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

meguca: init at 2018-05-17 #39482

meguca: init at 2018-05-17 #39482

Chiiruno commented Apr 25, 2018 •

edited

Chiiruno commented Apr 25, 2018 •

edited

Chiiruno Apr 25, 2018 •

edited

Mic92 Apr 28, 2018

Mic92 commented Apr 28, 2018

Mic92 commented Apr 28, 2018

GrahamcOfBorg commented Apr 28, 2018

GrahamcOfBorg commented Apr 28, 2018

GrahamcOfBorg commented Apr 28, 2018

Chiiruno commented Apr 29, 2018 •

edited

Chiiruno commented May 3, 2018

Chiiruno commented May 5, 2018

Chiiruno commented May 18, 2018

Mic92 May 20, 2018

Mic92 May 20, 2018 •

edited

Mic92 May 20, 2018

Mic92 May 20, 2018

Chiiruno May 20, 2018 •

edited

Mic92 left a comment

Chiiruno commented May 21, 2018

Mic92 May 21, 2018 •

edited

Mic92 May 21, 2018 •

edited

Mic92 May 21, 2018

Chiiruno May 21, 2018

Chiiruno May 21, 2018

Mic92 May 21, 2018

Chiiruno commented May 21, 2018

Mic92 May 27, 2018

Mic92 May 27, 2018

meguca: init at 2018-05-17 #39482

meguca: init at 2018-05-17 #39482

Conversation

Chiiruno commented Apr 25, 2018 • edited

Motivation for this change

Things done

Chiiruno commented Apr 25, 2018 • edited

Chiiruno Apr 25, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mic92 commented Apr 28, 2018

Mic92 commented Apr 28, 2018

GrahamcOfBorg commented Apr 28, 2018

GrahamcOfBorg commented Apr 28, 2018

GrahamcOfBorg commented Apr 28, 2018

Chiiruno commented Apr 29, 2018 • edited

Chiiruno commented May 3, 2018

Chiiruno commented May 5, 2018

Chiiruno commented May 18, 2018

Choose a reason for hiding this comment

Mic92 May 20, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chiiruno May 20, 2018 • edited

Choose a reason for hiding this comment

Mic92 left a comment

Choose a reason for hiding this comment

Chiiruno commented May 21, 2018

Mic92 May 21, 2018 • edited

Choose a reason for hiding this comment

Mic92 May 21, 2018 • 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

Chiiruno commented May 21, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chiiruno commented Apr 25, 2018 •

edited

Chiiruno commented Apr 25, 2018 •

edited

Chiiruno Apr 25, 2018 •

edited

Chiiruno commented Apr 29, 2018 •

edited

Mic92 May 20, 2018 •

edited

Chiiruno May 20, 2018 •

edited

Mic92 May 21, 2018 •

edited

Mic92 May 21, 2018 •

edited