{"thread":{"id":"39348","subject":"[PATCH] daemon: add systemd support","startedAt":"2015-05-17T02:44:10Z","lastAt":"2015-05-17T19:58:11Z","messageCount":3,"participants":["Shawn Landden","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"261399","messageId":"1431830650-111684-1-git-send-email-shawn@churchofgit.com","threadId":"39348","inReplyTo":null,"subject":"[PATCH] daemon: add systemd support","fromName":"Shawn Landden","fromEmail":"shawn@churchofgit.com","sentAt":"2015-05-17T02:44:10Z","receivedAt":"2015-05-17T02:44:10Z","isPatch":true,"sender":{"key":"shawn@churchofgit.com","avatar":"https://gravatar.com/avatar/66fa92115ac78085af2524ca39ccca362bd1ca7571a4b0b843a8a7017b210df2?d=mp&s=160"},"body":"git-daemon's --systemd mode allows git-daemon to be connect-activated\non one or more addresses or ports. Unlike --inetd[1], git-daemon is\nnot spawned for every connection.\n\n[1]which systemd is compatible with using its Accept=yes mode\n\nSigned-off-by: Shawn Landden <shawn@churchofgit.com>\n---\n Documentation/git-daemon.txt | 49 ++++++++++++++++++++++++++++++----\n Makefile                     | 10 +++++++\n daemon.c                     | 62 +++++++++++++++++++++++++++++++++++++++-----\n 3 files changed, 110 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\nindex a69b361..0eab51b 100644\n--- a/Documentation/git-daemon.txt\n+++ b/Documentation/git-daemon.txt\n@@ -19,7 +19,8 @@ SYNOPSIS\n \t     [--access-hook=<path>] [--[no-]informative-errors]\n \t     [--inetd |\n \t      [--listen=<host_or_ipaddr>] [--port=<n>]\n-\t      [--user=<user> [--group=<group>]]]\n+\t      [--systemd |\n+\t       [--user=<user> [--group=<group>]]]\n \t     [<directory>...]\n \n DESCRIPTION\n@@ -81,8 +82,8 @@ OPTIONS\n \n --inetd::\n \tHave the server run as an inetd service. Implies --syslog.\n-\tIncompatible with --detach, --port, --listen, --user and --group\n-\toptions.\n+\tIncompatible with --systemd, --detach, --port, --listen, --user and\n+\t--group options.\n \n --listen=<host_or_ipaddr>::\n \tListen on a specific IP address or hostname.  IP addresses can\n@@ -146,8 +147,8 @@ OPTIONS\n \tthe option are given to `getpwnam(3)` and `getgrnam(3)`\n \tand numeric IDs are not supported.\n +\n-Giving these options is an error when used with `--inetd`; use\n-the facility of inet daemon to achieve the same before spawning\n+Giving these options is an error when used with `--inetd` or `--systemd`; use\n+the facility of systemd or the inet daemon to achieve the same before spawning\n 'git daemon' if needed.\n +\n Like many programs that switch user id, the daemon does not reset\n@@ -180,6 +181,16 @@ Git configuration files in that directory are readable by `<user>`.\n \terrors are not enabled, all errors report \"access denied\" to the\n \tclient. The default is --no-informative-errors.\n \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/--port, and git-daemon\n+\tdoesn't get invoked for every connection, but only the first.\n+\tFor more details see systemd.socket(5). Incompatible with\n+\t--inetd, --detach, --user and --group options.\n+\tWorks with the session manager (systemd --user) too.\n+\n --access-hook=<path>::\n \tEvery time a client connects, first run an external command\n \tspecified by the <path> with service name (e.g. \"upload-pack\"),\n@@ -305,6 +316,34 @@ selectively enable/disable services per repository::\n \t\tuploadarch = true\n ----------------------------------------------------------------\n \n+systemd configuration example::\n+Example systemd configuration files, typically placed in `/etc/systemd/system`\n+or `$HOME/.config/systemd/user`.\n++\n+`git-daemon.socket`\n++\n+----------------------------------------------------------------\n+[Unit]\n+Description=Git Daemon socket\n+\n+[Socket]\n+ListenStream=9418\n+\n+[Install]\n+WantedBy=sockets.target\n+----------------------------------------------------------------\n++\n+`git-daemon.service`\n++\n+----------------------------------------------------------------\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=git-daemon\n+StandardError=null\n+----------------------------------------------------------------\n \n ENVIRONMENT\n -----------\ndiff --git a/Makefile b/Makefile\nindex 36655d5..54986a0 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@@ -997,6 +1000,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 2>/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\ndiff --git a/daemon.c b/daemon.c\nindex d3d3e43..42e1441 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@@ -28,7 +32,11 @@ static const char daemon_usage[] =\n \"           [--(enable|disable|allow-override|forbid-override)=<service>]\\n\"\n \"           [--access-hook=<path>]\\n\"\n \"           [--inetd | [--listen=<host_or_ipaddr>] [--port=<n>]\\n\"\n+#ifdef HAVE_SYSTEMD\n+\"                      [--systemd | [--detach] [--user=<user> [--group=<group>]]]\\n\" /* exactly 80 characters */\n+#else\n \"                      [--detach] [--user=<user> [--group=<group>]]\\n\"\n+#endif\n \"           [<directory>...]\";\n \n /* List of acceptable pathname prefixes */\n@@ -1166,12 +1174,40 @@ static struct credentials *prepare_credentials(const char *user_name,\n }\n #endif\n \n+#ifdef HAVE_SYSTEMD\n+static int enumerate_sockets(struct socketlist *socklist, struct string_list *listen_addr, int listen_port, int systemd_mode)\n+{\n+\tif (systemd_mode) {\n+\t\tint i, n;\n+\n+\t\tn = sd_listen_fds(0);\n+\t\tif (n <= 0)\n+\t\t\tdie(\"--systemd mode specified and no file descriptors recieved\");\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+\t\tsocksetup(listen_addr, listen_port, socklist);\n+\n+\treturn 0;\n+}\n+#else\n+static int enumerate_sockets(struct socketlist *socklist, struct string_list *listen_addr, int listen_port, int systemd_mode)\n+{\n+\tsocksetup(listen_addr, listen_port, socklist);\n+\n+\treturn 0;\n+}\n+#endif\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+\tenumerate_sockets(&socklist, listen_addr, listen_port, systemd_mode);\n \tif (socklist.nr == 0)\n \t\tdie(\"unable to allocate any listen sockets on port %u\",\n \t\t    listen_port);\n@@ -1187,7 +1223,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@@ -1322,6 +1358,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@@ -1340,8 +1382,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@@ -1386,5 +1436,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":"261413","messageId":"xmqq4mnbdqfi.fsf@gitster.dls.corp.google.com","threadId":"39348","inReplyTo":"1431830650-111684-1-git-send-email-shawn@churchofgit.com","subject":"Re: [PATCH] daemon: add systemd support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-05-17T18:12:01Z","receivedAt":"2015-05-17T18:12:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Landden <shawn@churchofgit.com> writes:\n\n> git-daemon's --systemd mode allows git-daemon to be connect-activated\n> on one or more addresses or ports. Unlike --inetd[1], git-daemon is\n> not spawned for every connection.\n>\n> [1]which systemd is compatible with using its Accept=yes mode\n\nI can barely parse but cannot comprehend this footnote and the body\ntext the footnote is attached to.\n\n> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt\n> index a69b361..0eab51b 100644\n> --- a/Documentation/git-daemon.txt\n> +++ b/Documentation/git-daemon.txt\n> @@ -19,7 +19,8 @@ SYNOPSIS\n>  \t     [--access-hook=<path>] [--[no-]informative-errors]\n>  \t     [--inetd |\n>  \t      [--listen=<host_or_ipaddr>] [--port=<n>]\n> -\t      [--user=<user> [--group=<group>]]]\n> +\t      [--systemd |\n> +\t       [--user=<user> [--group=<group>]]]\n>  \t     [<directory>...]\n>  DESCRIPTION\n> @@ -81,8 +82,8 @@ OPTIONS\n>  \n>  --inetd::\n>  \tHave the server run as an inetd service. Implies --syslog.\n> -\tIncompatible with --detach, --port, --listen, --user and --group\n> -\toptions.\n> +\tIncompatible with --systemd, --detach, --port, --listen, --user and\n> +\t--group options.\n\nWhen adding to a new thing to an existing list, we usually add to\nthe end (same comment applies to the addition to SYNOPSIS above).\nYou did that correctly to the other part (like the next hunk and\nalso the example), so let's be consistent.\n\n> @@ -146,8 +147,8 @@ OPTIONS\n>  \tthe option are given to `getpwnam(3)` and `getgrnam(3)`\n>  \tand numeric IDs are not supported.\n>  +\n> -Giving these options is an error when used with `--inetd`; use\n> -the facility of inet daemon to achieve the same before spawning\n> +Giving these options is an error when used with `--inetd` or `--systemd`; use\n> +the facility of systemd or the inet daemon to achieve the same before spawning\n>  'git daemon' if needed.\n>  +\n>  Like many programs that switch user id, the daemon does not reset\n\n> diff --git a/Makefile b/Makefile\n> index 36655d5..54986a0 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\nHmmm, instead of doing negative, make this an opt-in \"USE_SYSTEMD\"?\n\n> @@ -997,6 +1000,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 2>/dev/null && echo y),y)\n> +\t\tBASIC_CFLAGS += -DHAVE_SYSTEMD\n> +\t\tEXTLIBS += -lsystemd\n> +\tendif\n\nThis is bad, if we expect headers and libraries can be installed\noutside the usual search paths.  Perhaps imitate what we do for\nlibpcre library where we enable with $USE_LIBPCRE and allow the\nlocation specified with $LIBPCREDIR, or something like that?\n\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> diff --git a/daemon.c b/daemon.c\n> index d3d3e43..42e1441 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\nNever include system headers to our code before \"git-compat-util.h\"\nis included (either directly or via \"cache.h\" and friends).\n\n>  #include \"pkt-line.h\"\n>  #include \"exec_cmd.h\"\n> @@ -28,7 +32,11 @@ static const char daemon_usage[] =\n>  \"           [--(enable|disable|allow-override|forbid-override)=<service>]\\n\"\n>  \"           [--access-hook=<path>]\\n\"\n>  \"           [--inetd | [--listen=<host_or_ipaddr>] [--port=<n>]\\n\"\n> +#ifdef HAVE_SYSTEMD\n> +\"                      [--systemd | [--detach] [--user=<user> [--group=<group>]]]\\n\" /* exactly 80 characters */\n\nAn overlong source line.\n\nI am not sure if we want to hide the \"--systemd\" option from this\nlist with #ifdef ugliness.  \"git daemon --systemd\" can error out with\na build without USE_SYSTEMD with \"fatal: systemd not supported\".\n\n> +#else\n>  \"                      [--detach] [--user=<user> [--group=<group>]]\\n\"\n> +#endif\n>  \"           [<directory>...]\";\n>  \n>  /* List of acceptable pathname prefixes */\n> @@ -1166,12 +1174,40 @@ static struct credentials *prepare_credentials(const char *user_name,\n>  }\n>  #endif\n>  \n> +#ifdef HAVE_SYSTEMD\n> +static int enumerate_sockets(struct socketlist *socklist, struct string_list *listen_addr, int listen_port, int systemd_mode)\n> +{\n> +\tif (systemd_mode) {\n> +\t\tint i, n;\n> +\n> +\t\tn = sd_listen_fds(0);\n> +\t\tif (n <= 0)\n> +\t\t\tdie(\"--systemd mode specified and no file descriptors recieved\");\n\n\"received\"\n\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> +\t\tsocksetup(listen_addr, listen_port, socklist);\n> +\n> +\treturn 0;\n> +}\n> +#else\n> +static int enumerate_sockets(struct socketlist *socklist, struct string_list *listen_addr, int listen_port, int systemd_mode)\n> +{\n> +\tsocksetup(listen_addr, listen_port, socklist);\n> +\n> +\treturn 0;\n> +}\n> +#endif\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\nA full int for \"systemd_mode\" feels a poor taste that invites more\nugliness in the future---the next person will be tempted to add yet\nanother variable to add foobar_mode.  How about adding \"unsigned flags\"\nand check\n\n\t#define SYSTEMD_MODE 1\n\n\tif (flags & SYSTEMD_MODE)\n        \t... do the systemd thing ...\n\telse\n        \t... do other thing ...\n\nActually, I think we should do without this systemd_mode (or flags\nfor that matter) passed to this function via a parameter.\n\nCan't we consolidate inetd_mode and serve_mode into a single \"enum\nservice_mode { SERVE, INETD, SYSTEMD }\" variable as a preparatory\nstep?  I think it is perfectly fine to make it a global variable\nwithout passing it via a parameter, as the choice among serve, inetd\nand systemd is done once for the process and will not change\nthroughout the life of it.\n\nWith those suggested changes, there would be fewer conditionally-\ncompiled stuff and I suspect the result would be cleaner.\n\nAlso, I wonder if we may even want do the attached patch for the\n\"enumerate\" stuff (of course, \"if (systemd)\" would further be\nchanged to \"if (sevice_mode == SYSTEMD)\" etc.).\n\nThanks.\n\n\n daemon.c | 26 +++++++++++---------------\n 1 file changed, 11 insertions(+), 15 deletions(-)\n\ndiff --git a/daemon.c b/daemon.c\nindex 42e1441..c8d529b 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1,7 +1,3 @@\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@@ -9,6 +5,13 @@\n #include \"strbuf.h\"\n #include \"string-list.h\"\n \n+#ifdef HAVE_SYSTEMD\n+#  include <systemd/sd-daemon.h>\n+#else\n+#define SD_LISTEN_FDS_START 0 /* not used */\n+#define sd_listen_fds(n) (n) /* not used */\n+#endif\n+\n #ifndef HOST_NAME_MAX\n #define HOST_NAME_MAX 256\n #endif\n@@ -1174,15 +1177,16 @@ static struct credentials *prepare_credentials(const char *user_name,\n }\n #endif\n \n-#ifdef HAVE_SYSTEMD\n-static int enumerate_sockets(struct socketlist *socklist, struct string_list *listen_addr, int listen_port, int systemd_mode)\n+static int enumerate_sockets(struct socketlist *socklist,\n+\t\t\t     struct string_list *listen_addr,\n+\t\t\t     int listen_port, int systemd_mode)\n {\n \tif (systemd_mode) {\n \t\tint i, n;\n \n \t\tn = sd_listen_fds(0);\n \t\tif (n <= 0)\n-\t\t\tdie(\"--systemd mode specified and no file descriptors recieved\");\n+\t\t\tdie(\"--systemd mode specified and no file descriptors received\");\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@@ -1193,14 +1197,6 @@ static int enumerate_sockets(struct socketlist *socklist, struct string_list *li\n \n \treturn 0;\n }\n-#else\n-static int enumerate_sockets(struct socketlist *socklist, struct string_list *listen_addr, int listen_port, int systemd_mode)\n-{\n-\tsocksetup(listen_addr, listen_port, socklist);\n-\n-\treturn 0;\n-}\n-#endif\n \n static int serve(struct string_list *listen_addr, int listen_port,\n     struct credentials *cred, int systemd_mode)\n"},{"id":"261416","messageId":"CAPig+cSOc8Qde1oxq2V+1m4gjnMHaa_mKXO4n21V6zQox9=6qg@mail.gmail.com","threadId":"39348","inReplyTo":"1431830650-111684-1-git-send-email-shawn@churchofgit.com","subject":"Re: [PATCH] daemon: add systemd support","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-05-17T19:58:11Z","receivedAt":"2015-05-17T19:58:11Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, May 16, 2015 at 10:44 PM, Shawn Landden <shawn@churchofgit.com> wrote:\n> daemon: add systemd support\n>\n> git-daemon's --systemd mode allows git-daemon to be connect-activated\n> on one or more addresses or ports. Unlike --inetd[1], git-daemon is\n> not spawned for every connection.\n>\n> [1]which systemd is compatible with using its Accept=yes mode\n>\n> Signed-off-by: Shawn Landden <shawn@churchofgit.com>\n> ---\n\nFor convenience of other reviewers, this is v8. Links to all versions:\n\nv8 (2015-05-17): http://thread.gmane.org/gmane.comp.version-control.git/269205\n\nv7.1 (2015-04-08):\nhttp://thread.gmane.org/gmane.comp.version-control.git/266632/focus=266969\n\nv7 (2015-04-07): http://thread.gmane.org/gmane.comp.version-control.git/266926\n\nv6 (2015-04-07): http://thread.gmane.org/gmane.comp.version-control.git/266895\n\nv5 (2015-04-04): http://thread.gmane.org/gmane.comp.version-control.git/266759\n\nv4 (2015-04-03):\nhttp://git.661346.n2.nabble.com/RFCv4-PATCH-daemon-add-systemd-support-td7628351.html\n\nv3 (2015-04-03):\nhttp://git.661346.n2.nabble.com/v3RFC-systemd-socket-activation-support-td7628336.html\n\nv2 (2015-04-02): http://thread.gmane.org/gmane.comp.version-control.git/266646\n\nv1.1 (2015-04-02): http://thread.gmane.org/gmane.comp.version-control.git/266632\n\nv1 (2015-04-02): http://thread.gmane.org/gmane.comp.version-control.git/266628\n\nBelow are some additional comments beyond what Junio already mentioned\nin his review...\n\n> diff --git a/Makefile b/Makefile\n> index 36655d5..54986a0 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -997,6 +1000,13 @@ ifeq ($(uname_S),Darwin)\n>         PTHREAD_LIBS =\n>  endif\n>\n> +ifndef NO_SYSTEMD\n> +       ifeq ($(shell echo \"\\#include <systemd/sd-daemon.h>\" | $(CC) -E - -o /dev/null 2>/dev/null && echo y),y)\n\nIt is highly unusual to place such an expensive check directly in\nMakefile (or even config.mak.uname) where it will penalize everyone\n(who hasn't disabled systemd) each time 'make' is invoked. This sort\nof expensive detection is typically only done by the configure script.\n\n> +               BASIC_CFLAGS += -DHAVE_SYSTEMD\n> +               EXTLIBS += -lsystemd\n> +       endif\n> +endif\n> diff --git a/daemon.c b/daemon.c\n> index d3d3e43..42e1441 100644\n> --- a/daemon.c\n> +++ b/daemon.c\n> @@ -1166,12 +1174,40 @@ static struct credentials *prepare_credentials(const char *user_name,\n>  }\n>  #endif\n>\n> +#ifdef HAVE_SYSTEMD\n> +static int enumerate_sockets(struct socketlist *socklist, struct string_list *listen_addr, int listen_port, int systemd_mode)\n> +{\n> +       if (systemd_mode) {\n> +               int i, n;\n> +\n> +               n = sd_listen_fds(0);\n> +               if (n <= 0)\n> +                       die(\"--systemd mode specified and no file descriptors recieved\");\n> +               ALLOC_GROW(socklist->list, socklist->nr + n, socklist->alloc);\n> +               for (i = 0; i < n; i++)\n> +                       socklist->list[socklist->nr++] = SD_LISTEN_FDS_START + i;\n> +       }\n> +\n> +       if (listen_addr->nr > 0 || !systemd_mode)\n> +               socksetup(listen_addr, listen_port, socklist);\n> +\n> +       return 0;\n\nWhat is the significance of the return value of enumerate_sockets()?\nIt's unconditionally 0, even if socksetup() was never invoked, and\nisn't checked by the caller.\n\n> +}\n> +#else\n> +static int enumerate_sockets(struct socketlist *socklist, struct string_list *listen_addr, int listen_port, int systemd_mode)\n> +{\n> +       socksetup(listen_addr, listen_port, socklist);\n> +\n> +       return 0;\n> +}\n> +#endif\n> @@ -1340,8 +1382,16 @@ int main(int argc, char **argv)\n>                 /* avoid splitting a message in the middle */\n>                 setvbuf(stderr, NULL, _IOFBF, 4096);\n>\n> -       if (inetd_mode && (detach || group_name || user_name))\n> -               die(\"--detach, --user and --group are incompatible with --inetd\");\n> +       if ((inetd_mode || systemd_mode) && (detach || group_name || user_name))\n> +               die(\"--detach, --user and --group are incompatible with --inetd and --systemd\");\n> +\n> +#ifdef HAVE_SYSTEMD\n\nThis #if is unnecessary since 'systemd_mode' will never become true\n(1) when HAVE_SYSTEMD is not defined, thus neither of the two\nfollowing 'if' conditionals will trigger anyhow.\n\n> +       if (systemd_mode && inetd_mode)\n> +               die(\"--inetd is incompatible with --systemd\");\n> +\n> +       if (systemd_mode && !sd_booted())\n> +               die(\"--systemd passed and not invoked from systemd\");\n> +#endif\n>         if (inetd_mode && (listen_port || (listen_addr.nr > 0)))\n>                 die(\"--listen= and --port= are incompatible with --inetd\");\n> --\n> 2.2.1.209.g41e5f3a\n"}]}