{"thread":{"id":"38996","subject":"[v3RFC] systemd socket activation support","startedAt":"2015-04-03T22:30:24Z","lastAt":"2015-04-03T23:06:14Z","messageCount":2,"participants":["Shawn Landden","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"258959","messageId":"1428100224-29441-1-git-send-email-shawn@churchofgit.com","threadId":"38996","inReplyTo":null,"subject":"[v3RFC] systemd socket activation support","fromName":"Shawn Landden","fromEmail":"shawn@churchofgit.com","sentAt":"2015-04-03T22:30:24Z","receivedAt":"2015-04-03T22:30:24Z","isPatch":false,"sender":{"key":"shawn@churchofgit.com","avatar":"https://gravatar.com/avatar/66fa92115ac78085af2524ca39ccca362bd1ca7571a4b0b843a8a7017b210df2?d=mp&s=160"},"body":"systemd supports git-daemon's existing --inetd mode as well.\n\nv2: actually test...\nv3: make optional, switch to libsystemd\n\nshawn@zephyr:~/git/git$ ldd /lib/x86_64-linux-gnu/libsystemd.so.0\n\tlinux-vdso.so.1 (0x00007ffeba7ec000)\n\tlibcap.so.2 => /lib/x86_64-linux-gnu/libcap.so.2 (0x00007fea158fe000)\n\tlibm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fea155f9000)\n\tlibrt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007fea153f0000)\n\tlibselinux.so.1 => /lib/x86_64-linux-gnu/libselinux.so.1 (0x00007fea151cb000)\n\tliblzma.so.5 => /lib/x86_64-linux-gnu/liblzma.so.5 (0x00007fea14fa8000)\n\tlibgcrypt.so.20 => /lib/x86_64-linux-gnu/libgcrypt.so.20 (0x00007fea14cc5000)\n\tlibresolv.so.2 => /lib/x86_64-linux-gnu/libresolv.so.2 (0x00007fea14aae000)\n\tlibdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fea148aa000)\n\tlibpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fea1468b000)\n\tlibc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fea142e7000)\n\t/lib64/ld-linux-x86-64.so.2 (0x00007fea15d5b000)\n\tlibattr.so.1 => /lib/x86_64-linux-gnu/libattr.so.1 (0x00007fea140e2000)\n\tlibpcre.so.3 => /lib/x86_64-linux-gnu/libpcre.so.3 (0x00007fea13e73000)\n\tlibgpg-error.so.0 => /lib/x86_64-linux-gnu/libgpg-error.so.0 (0x00007fea13c61000)\n\newwwww...and only for two tiny functions.\n\nSigned-off-by: Shawn Landden <shawn@churchofgit.com>\n---\n Documentation/git-daemon.txt | 25 ++++++++++++++++++++++++\n Makefile                     | 14 ++++++++++++--\n daemon.c                     | 46 ++++++++++++++++++++++++++++++++++++++------\n 3 files changed, 77 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\nindex a69b361..898e01f 100644\n--- a/Documentation/git-daemon.txt\n+++ b/Documentation/git-daemon.txt\n@@ -20,6 +20,7 @@ SYNOPSIS\n \t     [--inetd |\n \t      [--listen=<host_or_ipaddr>] [--port=<n>]\n \t      [--user=<user> [--group=<group>]]]\n+\t      [--systemd]\n \t     [<directory>...]\n \n DESCRIPTION\n@@ -190,6 +191,12 @@ Git configuration files in that directory are readable by `<user>`.\n \texiting with a zero status).  It can also look at the $REMOTE_ADDR\n \tand $REMOTE_PORT environment variables to learn about the\n \trequestor when making this decision.\n+--systemd::\n+\tFor running git-daemon under systemd(1) which will pass\n+\tan open connection. This is similar to --inetd, except\n+\tthat more than one address/port can be listened to at once\n+\tboth through systemd and through --listen, and git-daemon doesn't get\n+\tinvoked for every connection. For more details see systemd.socket(5).\n +\n The external command can optionally write a single line to its\n standard output to be sent to the requestor as an error message when\n@@ -304,7 +311,25 @@ selectively enable/disable services per repository::\n \t\tuploadpack = false\n \t\tuploadarch = true\n ----------------------------------------------------------------\n++\n+systemd configuration example:\n+\n+# /etc/systemd/system/git-daemon.socket\n+[Unit]\n+Description=Git Daemon socket\n+\n+[Socket]\n+ListenStream=9418\n+\n+[Install]\n+WantedBy=sockets.target\n+# /etc/systemd/system/git-daemon.service\n+[Unit]\n+Description=Git Daemon\n \n+[Service]\n+ExecStart=/usr/lib/git-core/git-daemon --systemd --reuseaddr --base-path=/var/lib /var/lib/git\n+User=gitdaemon\n \n ENVIRONMENT\n -----------\ndiff --git a/Makefile b/Makefile\nindex 5f3987f..362af94 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -42,6 +42,9 @@ all::\n # Define NO_EXPAT if you do not have expat installed.  git-http-push is\n # not built, and you cannot push using http:// and https:// transports (dumb).\n #\n+# Define NO_SYSTEMD to prevent systemd socket activation support from being\n+# built into git-daemon.\n+#\n # Define EXPATDIR=/foo/bar if your expat header and library files are in\n # /foo/bar/include and /foo/bar/lib directories.\n #\n@@ -995,6 +998,13 @@ ifeq ($(uname_S),Darwin)\n \tPTHREAD_LIBS =\n endif\n \n+ifndef NO_SYSTEMD\n+\tifeq ($(shell echo \"\\#include <systemd/sd-daemon.h>\" | $(CC) -E - -o /dev/null && echo y),y)\n+\t\tBASIC_CFLAGS += -DHAVE_SYSTEMD\n+\t\tEXTLIBS += -lsystemd\n+\tendif\n+endif\n+\n ifndef CC_LD_DYNPATH\n \tifdef NO_R_TO_GCC_LINKER\n \t\t# Some gcc does not accept and pass -R to the linker to specify\n@@ -1403,8 +1413,8 @@ ifdef NATIVE_CRLF\n endif\n \n ifdef USE_NED_ALLOCATOR\n-       COMPAT_CFLAGS += -Icompat/nedmalloc\n-       COMPAT_OBJS += compat/nedmalloc/nedmalloc.o\n+\tCOMPAT_CFLAGS += -Icompat/nedmalloc\n+\tCOMPAT_OBJS += compat/nedmalloc/nedmalloc.o\n endif\n \n ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT\ndiff --git a/daemon.c b/daemon.c\nindex 9ee2187..16b9eda 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1,3 +1,7 @@\n+#ifdef HAVE_SYSTEMD\n+#  include <systemd/sd-daemon.h>\n+#endif\n+\n #include \"cache.h\"\n #include \"pkt-line.h\"\n #include \"exec_cmd.h\"\n@@ -29,6 +33,9 @@ static const char daemon_usage[] =\n \"           [--access-hook=<path>]\\n\"\n \"           [--inetd | [--listen=<host_or_ipaddr>] [--port=<n>]\\n\"\n \"                      [--detach] [--user=<user> [--group=<group>]]\\n\"\n+#ifdef HAVE_SYSTEMD\n+\"           [--systemd]\\n\"\n+#endif\n \"           [<directory>...]\";\n \n /* List of acceptable pathname prefixes */\n@@ -1176,11 +1183,24 @@ static void store_pid(const char *path)\n }\n \n static int serve(struct string_list *listen_addr, int listen_port,\n-    struct credentials *cred)\n+    struct credentials *cred, int systemd_mode)\n {\n \tstruct socketlist socklist = { NULL, 0, 0 };\n \n-\tsocksetup(listen_addr, listen_port, &socklist);\n+#ifdef HAVE_SYSTEMD\n+\tif (systemd_mode) {\n+\t\tint i;\n+\t\tint n;\n+\n+\t\tn = sd_listen_fds(0);\n+\t\tALLOC_GROW(socklist.list, socklist.nr + n, socklist.alloc);\n+\t\tfor (i = 0; i < n; i++)\n+\t\t\tsocklist.list[socklist.nr++] = SD_LISTEN_FDS_START + i;\n+\t}\n+\n+\tif (listen_addr->nr > 0 || !systemd_mode)\n+#endif\n+\t\tsocksetup(listen_addr, listen_port, &socklist);\n \tif (socklist.nr == 0)\n \t\tdie(\"unable to allocate any listen sockets on port %u\",\n \t\t    listen_port);\n@@ -1196,7 +1216,7 @@ int main(int argc, char **argv)\n {\n \tint listen_port = 0;\n \tstruct string_list listen_addr = STRING_LIST_INIT_NODUP;\n-\tint serve_mode = 0, inetd_mode = 0;\n+\tint serve_mode = 0, inetd_mode = 0, systemd_mode = 0;\n \tconst char *pid_file = NULL, *user_name = NULL, *group_name = NULL;\n \tint detach = 0;\n \tstruct credentials *cred = NULL;\n@@ -1331,6 +1351,12 @@ int main(int argc, char **argv)\n \t\t\tinformative_errors = 0;\n \t\t\tcontinue;\n \t\t}\n+#ifdef HAVE_SYSTEMD\n+\t\tif (!strcmp(arg, \"--systemd\")) {\n+\t\t\tsystemd_mode = 1;\n+\t\t\tcontinue;\n+\t\t}\n+#endif\n \t\tif (!strcmp(arg, \"--\")) {\n \t\t\tok_paths = &argv[i+1];\n \t\t\tbreak;\n@@ -1349,8 +1375,16 @@ int main(int argc, char **argv)\n \t\t/* avoid splitting a message in the middle */\n \t\tsetvbuf(stderr, NULL, _IOFBF, 4096);\n \n-\tif (inetd_mode && (detach || group_name || user_name))\n-\t\tdie(\"--detach, --user and --group are incompatible with --inetd\");\n+\tif ((inetd_mode || systemd_mode) && (detach || group_name || user_name))\n+\t\tdie(\"--detach, --user and --group are incompatible with --inetd and --systemd\");\n+\n+#ifdef HAVE_SYSTEMD\n+\tif (systemd_mode && inetd_mode)\n+\t\tdie(\"--inetd is incompatible with --systemd\");\n+\n+\tif (systemd_mode && !sd_booted())\n+\t\tdie(\"--systemd passed and not invoked from systemd\");\n+#endif\n \n \tif (inetd_mode && (listen_port || (listen_addr.nr > 0)))\n \t\tdie(\"--listen= and --port= are incompatible with --inetd\");\n@@ -1395,5 +1429,5 @@ int main(int argc, char **argv)\n \t\tcld_argv[i+1] = argv[i];\n \tcld_argv[argc+1] = NULL;\n \n-\treturn serve(&listen_addr, listen_port, cred);\n+\treturn serve(&listen_addr, listen_port, cred, systemd_mode);\n }\n-- \n2.2.1.209.g41e5f3a\n"},{"id":"258964","messageId":"CAPig+cTFRz=b=hdsuP_v_1jzu9-utGEWQM_rkhpEBvq-chrZig@mail.gmail.com","threadId":"38996","inReplyTo":"1428100224-29441-1-git-send-email-shawn@churchofgit.com","subject":"Re: [v3RFC] systemd socket activation support","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-04-03T23:06:14Z","receivedAt":"2015-04-03T23:06:14Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Apr 3, 2015 at 6:30 PM, Shawn Landden <shawn@churchofgit.com> wrote:\n> systemd supports git-daemon's existing --inetd mode as well.\n>\n> v2: actually test...\n> v3: make optional, switch to libsystemd\n\nEvery issue raised by my review[1] of v2 still applies to v3, so I\nwon't bother repeating them here, however, there is one comment\n(below) new to this version of the patch.\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/266650\n\n> shawn@zephyr:~/git/git$ ldd /lib/x86_64-linux-gnu/libsystemd.so.0\n>         linux-vdso.so.1 (0x00007ffeba7ec000)\n>         libcap.so.2 => /lib/x86_64-linux-gnu/libcap.so.2 (0x00007fea158fe000)\n>         libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fea155f9000)\n>         librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007fea153f0000)\n>         libselinux.so.1 => /lib/x86_64-linux-gnu/libselinux.so.1 (0x00007fea151cb000)\n>         liblzma.so.5 => /lib/x86_64-linux-gnu/liblzma.so.5 (0x00007fea14fa8000)\n>         libgcrypt.so.20 => /lib/x86_64-linux-gnu/libgcrypt.so.20 (0x00007fea14cc5000)\n>         libresolv.so.2 => /lib/x86_64-linux-gnu/libresolv.so.2 (0x00007fea14aae000)\n>         libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fea148aa000)\n>         libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fea1468b000)\n>         libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fea142e7000)\n>         /lib64/ld-linux-x86-64.so.2 (0x00007fea15d5b000)\n>         libattr.so.1 => /lib/x86_64-linux-gnu/libattr.so.1 (0x00007fea140e2000)\n>         libpcre.so.3 => /lib/x86_64-linux-gnu/libpcre.so.3 (0x00007fea13e73000)\n>         libgpg-error.so.0 => /lib/x86_64-linux-gnu/libgpg-error.so.0 (0x00007fea13c61000)\n>\n> ewwwww...and only for two tiny functions.\n>\n> Signed-off-by: Shawn Landden <shawn@churchofgit.com>\n> ---\n> diff --git a/Makefile b/Makefile\n> index 5f3987f..362af94 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1403,8 +1413,8 @@ ifdef NATIVE_CRLF\n>  endif\n>\n>  ifdef USE_NED_ALLOCATOR\n> -       COMPAT_CFLAGS += -Icompat/nedmalloc\n> -       COMPAT_OBJS += compat/nedmalloc/nedmalloc.o\n> +       COMPAT_CFLAGS += -Icompat/nedmalloc\n> +       COMPAT_OBJS += compat/nedmalloc/nedmalloc.o\n\nYou're sneaking in (whitespace) changes unrelated to the rest of the\npatch. Typically, such unrelated cleanups should be relegated to\nseparate preparatory patches. (In this case, however, the change is\nprobably unwarranted and likely accidental, but serves as a reminder\nto review patches before sending them out.)\n\n>  endif\n>\n>  ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT\n> diff --git a/daemon.c b/daemon.c\n> index 9ee2187..16b9eda 100644\n> --- a/daemon.c\n> +++ b/daemon.c\n"}]}