unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#19688: [patch] add support for emacs daemon on Windows
@ 2015-01-25 19:18 Mark Laws
  2015-01-25 20:34 ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-01-25 19:18 UTC (permalink / raw)
  To: 19688

[-- Attachment #1: Type: text/plain, Size: 1351 bytes --]

Hi,

I am not on this list so please CC me in any replies.

Attached is a patch which should apply cleanly on top of master
(a3689d3c661fe36df971c875760f8d500b5ae994 as of this email). It allows
Emacs to run as a daemon on Windows. Without daemon mode, emacsclient
-a "" does not work, which makes it impossible to pass elisp to Emacs
via emacsclient unless Emacs is already running. In other words, this
will now work correctly:

emacsclient -a "" -e "(ediff-merge-files-with-ancestor ...)"

Previously, -a "" produced an error message on Windows. There was no
workaround, because:

emacs -a emacs -e "(ediff-merge-files-with-ancestor ...)"

would start a new Emacs with a buffer called
"(ediff-merge-files-with-ancestor ...)".

The functionality is identical to the UNIX implementation with one
difference: I didn't allow for running multiple/named daemons. The
reason I didn't was because the code for that in emacsclient.c took
the daemon name from the socket name if one was provided, and because
filesystem sockets are not supported on Windows, I couldn't think of a
way to provide identical behavior across UNIX and Windows.

This patch has been heavily tested against emacs-24 without any
issues; it seems to work well against master as well.

Thanks to Eli Zaretskii for help with debugging.

Cheers,
Mark Laws

-- 
|v\ /\ |\ |< |_ /\ \^| //

[-- Attachment #2: emacs-windows-daemon.patch --]
[-- Type: application/octet-stream, Size: 14011 bytes --]

diff --git a/lib-src/Makefile.in b/lib-src/Makefile.in
index d2705e7..b34daa4 100644
--- a/lib-src/Makefile.in
+++ b/lib-src/Makefile.in
@@ -389,12 +389,16 @@ movemail${EXEEXT}: ${srcdir}/movemail.c pop.o $(NTLIB) $(config_h)
 pop.o: ${srcdir}/pop.c ${srcdir}/pop.h ${srcdir}/../lib/min-max.h $(config_h)
 	$(AM_V_CC)$(CC) -c ${CPP_CFLAGS} ${MOVE_FLAGS} $<
 
-emacsclient${EXEEXT}: ${srcdir}/emacsclient.c $(NTLIB) $(config_h)
+server-guid_h = ../src/server-guid.h
+
+emacsclient${EXEEXT}: ${srcdir}/emacsclient.c $(NTLIB) $(config_h) \
+			$(server-guid_h)
 	$(AM_V_CCLD)$(CC) ${ALL_CFLAGS} $< \
 	   -DVERSION="\"${version}\"" $(NTLIB) $(LOADLIBES) $(LIB_FDATASYNC) \
 	   $(LIB_WSOCK32) $(LIBS_ECLIENT) -o $@
 
-emacsclientw${EXEEXT}: ${srcdir}/emacsclient.c $(NTLIB) $(CLIENTRES) $(config_h)
+emacsclientw${EXEEXT}: ${srcdir}/emacsclient.c $(NTLIB) $(CLIENTRES) \
+			$(config_h) $(server-guid_h)
 	$(AM_V_CCLD)$(CC) ${ALL_CFLAGS} $(CLIENTRES) -mwindows $< \
 	   -DVERSION="\"${version}\"" $(LOADLIBES) $(LIB_FDATASYNC) \
 	   $(LIB_WSOCK32) $(LIBS_ECLIENT) -o $@
diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index a04dda6..9d4f3ce 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -33,6 +33,8 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 # include <io.h>
 # include <winsock2.h>
 
+# include "server-guid.h"
+
 # define NO_SOCKETS_IN_FILE_SYSTEM
 
 # define HSOCKET SOCKET
@@ -595,13 +597,6 @@ decode_options (int argc, char **argv)
       display = NULL;
       tty = 1;
     }
-
-  if (alternate_editor && alternate_editor[0] == '\0')
-    {
-      message (true, "--alternate-editor argument or ALTERNATE_EDITOR variable cannot be\n\
-an empty string");
-      exit (EXIT_FAILURE);
-    }
 #endif /* WINDOWSNT */
 }
 
@@ -642,10 +637,8 @@ The following OPTIONS are accepted:\n\
 			Set filename of the TCP authentication file\n\
 -a EDITOR, --alternate-editor=EDITOR\n\
 			Editor to fallback to if the server is not running\n"
-#ifndef WINDOWSNT
 "			If EDITOR is the empty string, start Emacs in daemon\n\
 			mode and try connecting again\n"
-#endif /* not WINDOWSNT */
 "\n\
 Report bugs with M-x report-emacs-bug.\n");
   exit (EXIT_SUCCESS);
@@ -1459,9 +1452,57 @@ w32_give_focus (void)
 /* Start the emacs daemon and try to connect to it.  */
 
 static void
+connect_to_emacs_socket (void)
+{
+#ifdef WINDOWSNT
+  /* It's just a progress message, so don't pop a dialog if this is
+     emacsclientw.  */
+  if (!w32_window_app ())
+#endif
+  message (true, "Emacs daemon should have started, trying to connect again\n");
+  if ((emacs_socket = set_socket (1)) == INVALID_SOCKET)
+    {
+      message (true, "Error: Cannot connect even after starting the Emacs daemon\n");
+      exit (EXIT_FAILURE);
+    }
+}
+
+static void
 start_daemon_and_retry_set_socket (void)
 {
-#ifndef WINDOWSNT
+#ifdef WINDOWSNT
+  DWORD wait_result;
+  HANDLE w32_daemon_event;
+  STARTUPINFO si;
+  PROCESS_INFORMATION pi;
+
+  ZeroMemory (&si, sizeof si);
+  si.cb = sizeof si;
+  ZeroMemory (&pi, sizeof pi);
+
+  if (!CreateProcess (NULL, "emacs --daemon", NULL, NULL, FALSE,
+                      CREATE_NO_WINDOW, NULL, NULL, &si, &pi))
+    {
+      message (true, "%s: error starting emacs daemon\n", progname);
+      exit (EXIT_FAILURE);
+    }
+
+  w32_daemon_event = CreateEvent (NULL, TRUE, FALSE, W32_EMACS_SERVER_GUID);
+  if (w32_daemon_event == NULL)
+    {
+      message (true, "Couldn't create Windows daemon event");
+      exit (EXIT_FAILURE);
+    }
+  if (WaitForSingleObject (w32_daemon_event, INFINITE) != WAIT_OBJECT_0)
+    {
+      message (true, "Error while waiting for Windows daemon event");
+      exit (EXIT_FAILURE);
+    }
+  CloseHandle (w32_daemon_event);
+
+  /* Try connecting, the daemon should have started by now.  */
+  connect_to_emacs_socket ();
+#elif !defined(WINDOWSNT)
   pid_t dpid;
   int status;
 
@@ -1479,12 +1520,7 @@ start_daemon_and_retry_set_socket (void)
 	}
 
       /* Try connecting, the daemon should have started by now.  */
-      message (true, "Emacs daemon should have started, trying to connect again\n");
-      if ((emacs_socket = set_socket (1)) == INVALID_SOCKET)
-	{
-	  message (true, "Error: Cannot connect even after starting the Emacs daemon\n");
-	  exit (EXIT_FAILURE);
-	}
+      connect_to_emacs_socket ();
     }
   else if (dpid < 0)
     {
@@ -1511,7 +1547,7 @@ start_daemon_and_retry_set_socket (void)
       execvp ("emacs", d_argv);
       message (true, "%s: error starting emacs daemon\n", progname);
     }
-#endif /* WINDOWSNT */
+#endif /* !WINDOWSNT */
 }
 
 int
diff --git a/lisp/frame.el b/lisp/frame.el
index 1d5bbf2..23bbc0d 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -536,7 +536,8 @@ is not considered (see `next-frame')."
 Return nil if we don't know how to interpret DISPLAY."
   ;; MS-Windows doesn't know how to create a GUI frame in a -nw session.
   (if (and (eq system-type 'windows-nt)
-	   (null (window-system)))
+	   (null (window-system))
+	   (not (daemonp)))
       nil
     (cl-loop for descriptor in display-format-alist
 	     for pattern = (car descriptor)
diff --git a/lisp/frameset.el b/lisp/frameset.el
index 4a06374..17fe39b 100644
--- a/lisp/frameset.el
+++ b/lisp/frameset.el
@@ -1022,8 +1022,8 @@ Internal use only."
 (defun frameset-keep-original-display-p (force-display)
   "True if saved frames' displays should be honored.
 For the meaning of FORCE-DISPLAY, see `frameset-restore'."
-  (cond ((daemonp) t)
-	((eq system-type 'windows-nt) nil) ;; Does ns support more than one display?
+  (cond ((eq system-type 'windows-nt) nil) ;; Does ns support more than one display?
+	((daemonp) t)
 	(t (not force-display))))
 
 (defun frameset-minibufferless-first-p (frame1 _frame2)
diff --git a/lisp/server.el b/lisp/server.el
index 166cd44..9585b17 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1139,9 +1139,12 @@ The following commands are accepted by the client:
                  ;; frame.  If running a GUI server, force the frame
                  ;; type to GUI.  (Cygwin is perfectly happy with
                  ;; multi-tty support, so don't override the user's
-                 ;; choice there.)
+                 ;; choice there.)  In daemon mode on Windows, we can't
+                 ;; make tty frames, so force the frame type to GUI
+                 ;; there too.
                  (when (and (eq system-type 'windows-nt)
-                            (eq window-system 'w32))
+                            (or (daemonp)
+                                (eq window-system 'w32)))
                    (push "-window-system" args-left)))
 
                 ;; -position LINE[:COLUMN]:  Set point to the given
@@ -1215,7 +1218,10 @@ The following commands are accepted by the client:
 					   terminal-frame)))))
 		    (setq tty-name nil tty-type nil)
 		    (if display (server-select-display display)))
-		   ((eq tty-name 'window-system)
+		   ((or (and (eq system-type 'windows-nt)
+			     (daemonp)
+			     (setq display "w32"))
+		        (eq tty-name 'window-system))
 		    (server-create-window-system-frame display nowait proc
 						       parent-id
 						       frame-parameters))
diff --git a/src/dispnew.c b/src/dispnew.c
index 3c8117e..e86fbb8 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -5974,9 +5974,12 @@ init_display (void)
     }
 #endif /* SIGWINCH */
 
-  /* If running as a daemon, no need to initialize any frames/terminal. */
+  /* If running as a daemon, no need to initialize any frames/terminal,
+     except on Windows, where we at least want to initialize it.  */
+#ifndef WINDOWSNT
   if (IS_DAEMON)
       return;
+#endif
 
   /* If the user wants to use a window system, we shouldn't bother
      initializing the terminal.  This is especially important when the
diff --git a/src/emacs.c b/src/emacs.c
index 345fe3e..8227eb0 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -39,6 +39,7 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <mbstring.h>
 #include "w32.h"
 #include "w32heap.h"
+#include "server-guid.h"
 #endif
 
 #if defined WINDOWSNT || defined HAVE_NTGUI
@@ -195,9 +196,15 @@ bool no_site_lisp;
 /* Name for the server started by the daemon.*/
 static char *daemon_name;
 
+#ifndef WINDOWSNT
 /* Pipe used to send exit notification to the daemon parent at
    startup.  */
 int daemon_pipe[2];
+#else
+bool w32_is_daemon;
+bool w32_daemon_is_initialized;
+static HANDLE w32_daemon_event;
+#endif
 
 /* Save argv and argc.  */
 char **initial_argv;
@@ -980,8 +987,10 @@ main (int argc, char **argv)
       exit (0);
     }
 
+#ifndef WINDOWSNT
   /* Make sure IS_DAEMON starts up as false.  */
   daemon_pipe[1] = 0;
+#endif
 
   if (argmatch (argv, argc, "-daemon", "--daemon", 5, NULL, &skip_args)
       || argmatch (argv, argc, "-daemon", "--daemon", 5, &dname_arg, &skip_args))
@@ -1111,10 +1120,12 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
       emacs_close (daemon_pipe[0]);
 
       setsid ();
-#else /* DOS_NT */
+#elif defined(WINDOWSNT)
+      w32_is_daemon = 1;
+#else /* MSDOS */
       fprintf (stderr, "This platform does not support the -daemon flag.\n");
       exit (1);
-#endif /* DOS_NT */
+#endif /* MSDOS */
     }
 
 #if defined HAVE_PTHREAD && !defined SYSTEM_MALLOC \
@@ -2310,23 +2321,45 @@ If the daemon was given a name argument, return that name. */)
     return Qnil;
 }
 
+static void
+daemon_check_preconditions (void)
+{
+  if (!IS_DAEMON)
+    error ("This function can only be called if emacs is run as a daemon");
+
+#ifdef WINDOWSNT
+  if (w32_daemon_is_initialized)
+#else
+  if (daemon_pipe[1] < 0 )
+#endif
+    error ("The daemon has already been initialized");
+
+  if (NILP (Vafter_init_time))
+    error ("This function can only be called after loading the init files");
+}
+
 DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
        doc: /* Mark the Emacs daemon as being initialized.
 This finishes the daemonization process by doing the other half of detaching
 from the parent process and its tty file descriptors.  */)
   (void)
 {
+#ifdef WINDOWSNT
+  daemon_check_preconditions ();
+
+  w32_daemon_event = CreateEvent (NULL, TRUE, FALSE, W32_EMACS_SERVER_GUID);
+  if (w32_daemon_event == NULL)
+    error ("Couldn't create event for Windows daemon");
+  if (GetLastError () == ERROR_ALREADY_EXISTS)
+    /* Signal the waiting emacsclient process.  */
+    SetEvent (w32_daemon_event);
+  CloseHandle (w32_daemon_event);
+  w32_daemon_is_initialized = true;
+#else
   int nfd;
   bool err = 0;
 
-  if (!IS_DAEMON)
-    error ("This function can only be called if emacs is run as a daemon");
-
-  if (daemon_pipe[1] < 0)
-    error ("The daemon has already been initialized");
-
-  if (NILP (Vafter_init_time))
-    error ("This function can only be called after loading the init files");
+  daemon_check_preconditions ();
 
   /* Get rid of stdin, stdout and stderr.  */
   nfd = emacs_open ("/dev/null", O_RDWR, 0);
@@ -2350,6 +2383,7 @@ from the parent process and its tty file descriptors.  */)
 
   if (err)
     error ("I/O error during daemon initialization");
+#endif
   return Qt;
 }
 
diff --git a/src/keyboard.c b/src/keyboard.c
index 383c109..fac9615 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3829,7 +3829,11 @@ kbd_buffer_get_event (KBOARD **kbp,
   if (noninteractive
       /* In case we are running as a daemon, only do this before
 	 detaching from the terminal.  */
+#ifdef WINDOWSNT
+      || (IS_DAEMON && !w32_daemon_is_initialized))
+#else
       || (IS_DAEMON && daemon_pipe[1] >= 0))
+#endif
     {
       int c = getchar ();
       XSETINT (obj, c);
diff --git a/src/lisp.h b/src/lisp.h
index f5242ab..f9e5dc4 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4215,9 +4215,15 @@ extern bool noninteractive;
 extern bool no_site_lisp;
 
 /* Pipe used to send exit notification to the daemon parent at
-   startup.  */
+   startup.  On Windows, we use a kernel event instead.  */
+#ifndef WINDOWSNT
 extern int daemon_pipe[2];
 #define IS_DAEMON (daemon_pipe[1] != 0)
+#elif defined(WINDOWSNT)
+extern bool w32_is_daemon;
+extern bool w32_daemon_is_initialized;
+#define IS_DAEMON (w32_is_daemon != 0)
+#endif
 
 /* True if handling a fatal error already.  */
 extern bool fatal_error_in_progress;
diff --git a/src/minibuf.c b/src/minibuf.c
index 3408bb9..3dceb27 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -459,7 +459,11 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   if ((noninteractive
        /* In case we are running as a daemon, only do this before
 	  detaching from the terminal.  */
+#ifdef WINDOWSNT
+       || (IS_DAEMON && !w32_daemon_is_initialized))
+#else
        || (IS_DAEMON && (daemon_pipe[1] >= 0)))
+#endif
       && NILP (Vexecuting_kbd_macro))
     {
       val = read_minibuf_noninteractive (map, initial, prompt,
diff --git a/src/server-guid.h b/src/server-guid.h
new file mode 100644
index 0000000..a58ebf4
--- /dev/null
+++ b/src/server-guid.h
@@ -0,0 +1,25 @@
+/* Event GUID for when emacsclient starts the Emacs daemon on Windows.
+
+Copyright (C) 1986-1987, 1994, 1999-2014 Free Software Foundation, Inc.
+
+This file is part of GNU Emacs.
+
+GNU Emacs is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation, either version 3 of the License, or
+(at your option) any later version.
+
+GNU Emacs is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef SERVER_GUID_H
+#define SERVER_GUID_H
+
+#define W32_EMACS_SERVER_GUID "{0B8E5DCB-D7CF-4423-A9F1-2F6927F0D318}"
+
+#endif

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-01-25 19:18 bug#19688: [patch] add support for emacs daemon on Windows Mark Laws
@ 2015-01-25 20:34 ` Eli Zaretskii
       [not found]   ` <CADemMPM+Tix-6FJ+CO3HA8y7Cq6AV0kv_e6_qn7BaSw1QMOwTQ@mail.gmail.com>
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2015-01-25 20:34 UTC (permalink / raw)
  To: Mark Laws; +Cc: 19688

> Date: Mon, 26 Jan 2015 04:18:34 +0900
> From: Mark Laws <mdl@60hz.org>
> 
> Attached is a patch which should apply cleanly on top of master
> (a3689d3c661fe36df971c875760f8d500b5ae994 as of this email). It allows
> Emacs to run as a daemon on Windows. Without daemon mode, emacsclient
> -a "" does not work, which makes it impossible to pass elisp to Emacs
> via emacsclient unless Emacs is already running. In other words, this
> will now work correctly:
> 
> emacsclient -a "" -e "(ediff-merge-files-with-ancestor ...)"

Thanks.

Did you get your paperwork with the FSF clerk done?  I don't see your
assignment on file yet.

> +#define W32_EMACS_SERVER_GUID "{0B8E5DCB-D7CF-4423-A9F1-2F6927F0D318}"

Where did this GUID come from?

In any case, instead of adding a one-line header file, it is better to
put this on nt/inc/ms-w32.h, which is included by both emacsclient.c
and all src/*.c files via src/config.h.

> +#ifndef WINDOWSNT
>  /* Pipe used to send exit notification to the daemon parent at
>     startup.  */
>  int daemon_pipe[2];
> +#else
> +bool w32_is_daemon;
> +bool w32_daemon_is_initialized;
> +static HANDLE w32_daemon_event;
> +#endif

Why do we need anything beyond the event handle?  Can't it serve
double duty as a flag as well?  We could use INVALID_HANDLE_VALUE
and/or NULL as distinct values with specific meanings.

> +#ifndef WINDOWSNT
>    /* Make sure IS_DAEMON starts up as false.  */
>    daemon_pipe[1] = 0;
> +#endif

You should do a similar initialization on WINDOWSNT, to avoid using
the value that was initialized when Emacs was dumped.

> +#ifdef WINDOWSNT
> +  daemon_check_preconditions ();
> +
> +  w32_daemon_event = CreateEvent (NULL, TRUE, FALSE, W32_EMACS_SERVER_GUID);
> +  if (w32_daemon_event == NULL)
> +    error ("Couldn't create event for Windows daemon");
> +  if (GetLastError () == ERROR_ALREADY_EXISTS)
> +    /* Signal the waiting emacsclient process.  */
> +    SetEvent (w32_daemon_event);
> +  CloseHandle (w32_daemon_event);
> +  w32_daemon_is_initialized = true;
> +#else

Please move this code to a function in w32.c, and here just call that
function.

Also, the call to daemon_check_preconditions should be outside of the
#ifdef, as it is used on all platforms.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
       [not found]   ` <CADemMPM+Tix-6FJ+CO3HA8y7Cq6AV0kv_e6_qn7BaSw1QMOwTQ@mail.gmail.com>
@ 2015-01-26  6:00     ` Eli Zaretskii
  2015-01-26  7:40       ` Mark Laws
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2015-01-26  6:00 UTC (permalink / raw)
  To: Mark Laws; +Cc: 19688

[Please keep the bug address on the CC list, so that this whole
discussion gets archived.]

> Date: Mon, 26 Jan 2015 08:16:38 +0900
> From: Mark Laws <mdl@60hz.org>
> 
> >> +#define W32_EMACS_SERVER_GUID "{0B8E5DCB-D7CF-4423-A9F1-2F6927F0D318}"
> >
> > Where did this GUID come from?
> >
> I generated it myself.

Is that safe?  Do we care whether this GUID is globally unique?  Why
exactly do we need it to begin with?

> Moved to ms-w32.h and removed changes relating to the header.

Thanks.

> >> +#else
> >> +bool w32_is_daemon;
> >> +bool w32_daemon_is_initialized;
> >> +static HANDLE w32_daemon_event;
> >> +#endif
> >
> > Why do we need anything beyond the event handle?  Can't it serve
> > double duty as a flag as well?  We could use INVALID_HANDLE_VALUE
> > and/or NULL as distinct values with specific meanings.
> 
> I've reworked it to use only w32_daemon_event. One minor issue: to
> avoid including any Windows-related headers in lisp.h, the extern
> declaration of w32_daemon_event there is void* instead of HANDLE. I
> doubt the type of HANDLE will ever change, so it shouldn't matter
> much.

That should be fine, but there's no need to have the handle in lisp.h,
either.  We could put it on w32.h, for example, which is already
included by emacs.c and other files.  If w32.h doesn't fit the bill,
please try to find some other w32*.h which does.

> >> +#ifndef WINDOWSNT
> >>    /* Make sure IS_DAEMON starts up as false.  */
> >>    daemon_pipe[1] = 0;
> >> +#endif
> >
> > You should do a similar initialization on WINDOWSNT, to avoid using
> > the value that was initialized when Emacs was dumped.
> 
> I'm not sure I understand. Do you mean for w32_daemon_event? If so, it
> should already always be zero when Emacs starts up.

It might not be zero.  Recall that Emacs is run during the build
process, when it executes some code, and then "dumps itself", which
involves writing portions of its memory to a disk file that thereafter
becomes the emacs.exe you run normally.  If, for some reason,
w32_daemon_event is initialized in this first instance, it will not be
zero in the dumped image.

So we always re-initialize such variables explicitly, instead of
relying on the linker initializations.  You can see that in the
sources, and that is also the reason why the Posix code initializes
daemon_pipe[1].

> > Also, the call to daemon_check_preconditions should be outside of the
> > #ifdef, as it is used on all platforms.
> 
> daemon_check_preconditions already gets called for both WINDOWSNT and
> !WINDOWSNT (see the #else). If we move it outside the #ifdef to
> eliminate having to mention it for both cases, then "nfd" and "err"
> will also have to be outside the ifdef in order to be compatible with
> strict pre-C99 compilers that only accept variable declarations at the
> beginning of blocks.

C99 compliant compiler is a prerequisite in the development version of
Emacs, so this problem doesn't exist.

Thanks.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-01-26  6:00     ` Eli Zaretskii
@ 2015-01-26  7:40       ` Mark Laws
  2015-01-26 11:56         ` Daniel Colascione
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-01-26  7:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 19688

On Mon, Jan 26, 2015 at 3:00 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> [Please keep the bug address on the CC list, so that this whole
> discussion gets archived.]

Oops, blindly hit reply last time without noticing where it was going
to--sorry about that.

>> Date: Mon, 26 Jan 2015 08:16:38 +0900
>> From: Mark Laws <mdl@60hz.org>
>>
>> >> +#define W32_EMACS_SERVER_GUID "{0B8E5DCB-D7CF-4423-A9F1-2F6927F0D318}"
>> >
>> > Where did this GUID come from?
>> >
>> I generated it myself.
>
> Is that safe?  Do we care whether this GUID is globally unique?  Why
> exactly do we need it to begin with?

It should be safe. On UNIX, Emacs uses a pipe to tell emacsclient when
it's done initializing. On Windows, since we don't have fork, the
easiest options are either a named event object[1] or specifying that
the child process inherit the event handle in CreateProcess. The
former is simpler, so I went with it. We could call it
"EmacsDaemonEvent" or something instead; it doesn't really matter as
long as it's a name nothing else is likely to use.

[1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms682396%28v=vs.85%29.aspx

>> > Why do we need anything beyond the event handle?  Can't it serve
>> > double duty as a flag as well?  We could use INVALID_HANDLE_VALUE
>> > and/or NULL as distinct values with specific meanings.
>>
>> I've reworked it to use only w32_daemon_event. One minor issue: to
>> avoid including any Windows-related headers in lisp.h, the extern
>> declaration of w32_daemon_event there is void* instead of HANDLE. I
>> doubt the type of HANDLE will ever change, so it shouldn't matter
>> much.
>
> That should be fine, but there's no need to have the handle in lisp.h,
> either.  We could put it on w32.h, for example, which is already
> included by emacs.c and other files.  If w32.h doesn't fit the bill,
> please try to find some other w32*.h which does.

IS_DAEMON is defined in lisp.h, so it seems to make the most sense to
put it there rather than having a different IS_DAEMON for Windows in
another place. We'd have to #include a w32 header just for IS_DAEMON
in a lot more files if we move it to one of them, too.

>> >> +#ifndef WINDOWSNT
>> >>    /* Make sure IS_DAEMON starts up as false.  */
>> >>    daemon_pipe[1] = 0;
>> >> +#endif
>> >
>> > You should do a similar initialization on WINDOWSNT, to avoid using
>> > the value that was initialized when Emacs was dumped.
>>
>> I'm not sure I understand. Do you mean for w32_daemon_event? If so, it
>> should already always be zero when Emacs starts up.
>
> It might not be zero.  Recall that Emacs is run during the build
> process, when it executes some code, and then "dumps itself", which
> involves writing portions of its memory to a disk file that thereafter
> becomes the emacs.exe you run normally.  If, for some reason,
> w32_daemon_event is initialized in this first instance, it will not be
> zero in the dumped image.
>
> So we always re-initialize such variables explicitly, instead of
> relying on the linker initializations.  You can see that in the
> sources, and that is also the reason why the Posix code initializes
> daemon_pipe[1].

OK, fixed.

>> > Also, the call to daemon_check_preconditions should be outside of the
>> > #ifdef, as it is used on all platforms.
>>
>> daemon_check_preconditions already gets called for both WINDOWSNT and
>> !WINDOWSNT (see the #else). If we move it outside the #ifdef to
>> eliminate having to mention it for both cases, then "nfd" and "err"
>> will also have to be outside the ifdef in order to be compatible with
>> strict pre-C99 compilers that only accept variable declarations at the
>> beginning of blocks.
>
> C99 compliant compiler is a prerequisite in the development version of
> Emacs, so this problem doesn't exist.

Fixed.

I'll attach another patch when we figure out what to do about the
remaining issues.

-- 
|v\ /\ |\ |< |_ /\ \^| //





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-01-26  7:40       ` Mark Laws
@ 2015-01-26 11:56         ` Daniel Colascione
  2015-01-27  8:40           ` Mark Laws
  0 siblings, 1 reply; 31+ messages in thread
From: Daniel Colascione @ 2015-01-26 11:56 UTC (permalink / raw)
  To: Mark Laws, Eli Zaretskii; +Cc: 19688

[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]

On 01/25/2015 11:40 PM, Mark Laws wrote:
> On Mon, Jan 26, 2015 at 3:00 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> [Please keep the bug address on the CC list, so that this whole
>> discussion gets archived.]
> 
> Oops, blindly hit reply last time without noticing where it was going
> to--sorry about that.
> 
>>> Date: Mon, 26 Jan 2015 08:16:38 +0900
>>> From: Mark Laws <mdl@60hz.org>
>>>
>>>>> +#define W32_EMACS_SERVER_GUID "{0B8E5DCB-D7CF-4423-A9F1-2F6927F0D318}"
>>>>
>>>> Where did this GUID come from?
>>>>
>>> I generated it myself.
>>
>> Is that safe?  Do we care whether this GUID is globally unique?  Why
>> exactly do we need it to begin with?
> 
> It should be safe. On UNIX, Emacs uses a pipe to tell emacsclient when
> it's done initializing. On Windows, since we don't have fork, the
> easiest options are either a named event object[1] or specifying that
> the child process inherit the event handle in CreateProcess. The
> former is simpler, so I went with it. We could call it
> "EmacsDaemonEvent" or something instead; it doesn't really matter as
> long as it's a name nothing else is likely to use.

Inheriting an anonymous event feels a bit cleaner to me; you can provide
the HANDLE value in an environment variable or a command line parameter.
Failing that, the event name should at least contain "emacs" somewhere
so as to not confuse people browsing named object directories.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-01-26 11:56         ` Daniel Colascione
@ 2015-01-27  8:40           ` Mark Laws
  2015-01-30  0:36             ` Mark Laws
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-01-27  8:40 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: 19688

[-- Attachment #1: Type: text/plain, Size: 1315 bytes --]

On Mon, Jan 26, 2015 at 8:56 PM, Daniel Colascione <dancol@dancol.org> wrote:
> Inheriting an anonymous event feels a bit cleaner to me; you can provide
> the HANDLE value in an environment variable or a command line parameter.
> Failing that, the event name should at least contain "emacs" somewhere
> so as to not confuse people browsing named object directories.

It may seem cleaner in that the event doesn't leak into a shared
namespace, but it requires significantly more code. Other handles may
also get unnecessarily inherited as a result, and it would require yet
more code to ensure they don't--it's difficult to ensure that it's
doing exactly what you want. The named event method is much, much
simpler. See for yourself; I've attached the patch written three ways:
a named event (simple), an inherited event passed through the
environment (a bit complicated), and an inherited event passed via the
command line (pretty heinous).

Also, I added multiple daemon support--after another look at the code,
I saw no reason to not include it, though there's still one difference
between the Windows and UNIX implementations: emacsclient can't
provide a name to be used for the daemon when it's starting a new
Emacs (but a user could provide one if they used emacs --daemon=foo).

-- 
|v\ /\ |\ |< |_ /\ \^| //

[-- Attachment #2: emacs-windows-daemon-inheritev.patch --]
[-- Type: application/octet-stream, Size: 12586 bytes --]

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index a04dda6..a1d1c44 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -595,13 +595,6 @@ decode_options (int argc, char **argv)
       display = NULL;
       tty = 1;
     }
-
-  if (alternate_editor && alternate_editor[0] == '\0')
-    {
-      message (true, "--alternate-editor argument or ALTERNATE_EDITOR variable cannot be\n\
-an empty string");
-      exit (EXIT_FAILURE);
-    }
 #endif /* WINDOWSNT */
 }
 
@@ -642,10 +635,8 @@ The following OPTIONS are accepted:\n\
 			Set filename of the TCP authentication file\n\
 -a EDITOR, --alternate-editor=EDITOR\n\
 			Editor to fallback to if the server is not running\n"
-#ifndef WINDOWSNT
 "			If EDITOR is the empty string, start Emacs in daemon\n\
 			mode and try connecting again\n"
-#endif /* not WINDOWSNT */
 "\n\
 Report bugs with M-x report-emacs-bug.\n");
   exit (EXIT_SUCCESS);
@@ -1459,9 +1450,66 @@ w32_give_focus (void)
 /* Start the emacs daemon and try to connect to it.  */
 
 static void
+connect_to_emacs_socket (void)
+{
+#ifdef WINDOWSNT
+  /* It's just a progress message, so don't pop a dialog if this is
+     emacsclientw.  */
+  if (!w32_window_app ())
+#endif
+  message (true, "Emacs daemon should have started, trying to connect again\n");
+  if ((emacs_socket = set_socket (1)) == INVALID_SOCKET)
+    {
+      message (true, "Error: Cannot connect even after starting the Emacs daemon\n");
+      exit (EXIT_FAILURE);
+    }
+}
+
+static void
 start_daemon_and_retry_set_socket (void)
 {
-#ifndef WINDOWSNT
+#ifdef WINDOWSNT
+  DWORD wait_result;
+  HANDLE w32_daemon_event;
+  STARTUPINFO si;
+  PROCESS_INFORMATION pi;
+  SECURITY_ATTRIBUTES sa;
+  char evbuf[32];
+
+  ZeroMemory (&si, sizeof si);
+  si.cb = sizeof si;
+  ZeroMemory (&pi, sizeof pi);
+
+  sa.nLength = sizeof sa;
+  sa.lpSecurityDescriptor = NULL;
+  sa.bInheritHandle = TRUE;
+
+  w32_daemon_event = CreateEvent (&sa, TRUE, FALSE, NULL);
+  if (w32_daemon_event == NULL)
+    {
+      message (true, "Couldn't create Windows daemon event");
+      exit (EXIT_FAILURE);
+    }
+
+  snprintf (evbuf, sizeof evbuf, "%p", w32_daemon_event);
+  SetEnvironmentVariable ("EMACS_W32_DAEMON_EVENT", evbuf);
+  if (!CreateProcess (NULL, "emacs --daemon", NULL, NULL, TRUE,
+                      CREATE_NO_WINDOW, NULL, NULL, &si, &pi))
+    {
+      message (true, "%s: error starting emacs daemon\n", progname);
+      exit (EXIT_FAILURE);
+    }
+
+  if (WaitForSingleObject (w32_daemon_event, INFINITE) != WAIT_OBJECT_0)
+    {
+      message (true, "Error while waiting for Windows daemon event");
+      exit (EXIT_FAILURE);
+    }
+  CloseHandle (w32_daemon_event);
+
+  /* Try connecting, the daemon should have started by now.  */
+  connect_to_emacs_socket ();
+#elif !defined(WINDOWSNT)
   pid_t dpid;
   int status;
 
@@ -1479,12 +1527,7 @@ start_daemon_and_retry_set_socket (void)
 	}
 
       /* Try connecting, the daemon should have started by now.  */
-      message (true, "Emacs daemon should have started, trying to connect again\n");
-      if ((emacs_socket = set_socket (1)) == INVALID_SOCKET)
-	{
-	  message (true, "Error: Cannot connect even after starting the Emacs daemon\n");
-	  exit (EXIT_FAILURE);
-	}
+      connect_to_emacs_socket ();
     }
   else if (dpid < 0)
     {
@@ -1511,7 +1554,7 @@ start_daemon_and_retry_set_socket (void)
       execvp ("emacs", d_argv);
       message (true, "%s: error starting emacs daemon\n", progname);
     }
-#endif /* WINDOWSNT */
+#endif /* !WINDOWSNT */
 }
 
 int
diff --git a/lisp/frame.el b/lisp/frame.el
index 1d5bbf2..23bbc0d 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -536,7 +536,8 @@ is not considered (see `next-frame')."
 Return nil if we don't know how to interpret DISPLAY."
   ;; MS-Windows doesn't know how to create a GUI frame in a -nw session.
   (if (and (eq system-type 'windows-nt)
-	   (null (window-system)))
+	   (null (window-system))
+	   (not (daemonp)))
       nil
     (cl-loop for descriptor in display-format-alist
 	     for pattern = (car descriptor)
diff --git a/lisp/frameset.el b/lisp/frameset.el
index 4a06374..17fe39b 100644
--- a/lisp/frameset.el
+++ b/lisp/frameset.el
@@ -1022,8 +1022,8 @@ Internal use only."
 (defun frameset-keep-original-display-p (force-display)
   "True if saved frames' displays should be honored.
 For the meaning of FORCE-DISPLAY, see `frameset-restore'."
-  (cond ((daemonp) t)
-	((eq system-type 'windows-nt) nil) ;; Does ns support more than one display?
+  (cond ((eq system-type 'windows-nt) nil) ;; Does ns support more than one display?
+	((daemonp) t)
 	(t (not force-display))))
 
 (defun frameset-minibufferless-first-p (frame1 _frame2)
diff --git a/lisp/server.el b/lisp/server.el
index 166cd44..9585b17 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1139,9 +1139,12 @@ The following commands are accepted by the client:
                  ;; frame.  If running a GUI server, force the frame
                  ;; type to GUI.  (Cygwin is perfectly happy with
                  ;; multi-tty support, so don't override the user's
-                 ;; choice there.)
+                 ;; choice there.)  In daemon mode on Windows, we can't
+                 ;; make tty frames, so force the frame type to GUI
+                 ;; there too.
                  (when (and (eq system-type 'windows-nt)
-                            (eq window-system 'w32))
+                            (or (daemonp)
+                                (eq window-system 'w32)))
                    (push "-window-system" args-left)))
 
                 ;; -position LINE[:COLUMN]:  Set point to the given
@@ -1215,7 +1218,10 @@ The following commands are accepted by the client:
 					   terminal-frame)))))
 		    (setq tty-name nil tty-type nil)
 		    (if display (server-select-display display)))
-		   ((eq tty-name 'window-system)
+		   ((or (and (eq system-type 'windows-nt)
+			     (daemonp)
+			     (setq display "w32"))
+		        (eq tty-name 'window-system))
 		    (server-create-window-system-frame display nowait proc
 						       parent-id
 						       frame-parameters))
diff --git a/src/dispnew.c b/src/dispnew.c
index 3c8117e..e86fbb8 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -5974,9 +5974,12 @@ init_display (void)
     }
 #endif /* SIGWINCH */
 
-  /* If running as a daemon, no need to initialize any frames/terminal. */
+  /* If running as a daemon, no need to initialize any frames/terminal,
+     except on Windows, where we at least want to initialize it.  */
+#ifndef WINDOWSNT
   if (IS_DAEMON)
       return;
+#endif
 
   /* If the user wants to use a window system, we shouldn't bother
      initializing the terminal.  This is especially important when the
diff --git a/src/emacs.c b/src/emacs.c
index 345fe3e..725605f 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -195,9 +195,13 @@ bool no_site_lisp;
 /* Name for the server started by the daemon.*/
 static char *daemon_name;
 
+#ifndef WINDOWSNT
 /* Pipe used to send exit notification to the daemon parent at
    startup.  */
 int daemon_pipe[2];
+#else
+HANDLE w32_daemon_event;
+#endif
 
 /* Save argv and argc.  */
 char **initial_argv;
@@ -980,8 +984,12 @@ main (int argc, char **argv)
       exit (0);
     }
 
+#ifndef WINDOWSNT
   /* Make sure IS_DAEMON starts up as false.  */
   daemon_pipe[1] = 0;
+#else
+  w32_daemon_event = NULL;
+#endif
 
   if (argmatch (argv, argc, "-daemon", "--daemon", 5, NULL, &skip_args)
       || argmatch (argv, argc, "-daemon", "--daemon", 5, &dname_arg, &skip_args))
@@ -1111,10 +1119,44 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
       emacs_close (daemon_pipe[0]);
 
       setsid ();
-#else /* DOS_NT */
+#elif defined(WINDOWSNT)
+      DWORD ret;
+      char evbuf[32];
+
+      ret = GetEnvironmentVariable ("EMACS_W32_DAEMON_EVENT", evbuf,
+                                    sizeof evbuf);
+      if (ret == 0)
+        {
+          if (GetLastError () == ERROR_ENVVAR_NOT_FOUND)
+            {
+              /* We weren't called by emacsclient; the user started the
+                 daemon.  */
+              w32_daemon_event = CreateEvent (NULL, TRUE, FALSE, NULL);
+            }
+          else
+            {
+              fprintf (stderr, "Couldn't copy handle address from environment\n");
+              exit (1);
+            }
+        }
+      else if (sscanf (evbuf, "%p", &w32_daemon_event) < 1)
+        {
+          fprintf (stderr, "Couldn't parse handle address from environment\n");
+          exit (1);
+        }
+
+      if (w32_daemon_event == NULL)
+        {
+          fprintf (stderr, "Couldn't create or get event handle for daemon\n");
+          exit (1);
+        }
+
+      if (dname_arg)
+        daemon_name = xstrdup (dname_arg);
+#else /* MSDOS */
       fprintf (stderr, "This platform does not support the -daemon flag.\n");
       exit (1);
-#endif /* DOS_NT */
+#endif /* MSDOS */
     }
 
 #if defined HAVE_PTHREAD && !defined SYSTEM_MALLOC \
@@ -2310,23 +2352,40 @@ If the daemon was given a name argument, return that name. */)
     return Qnil;
 }
 
-DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
-       doc: /* Mark the Emacs daemon as being initialized.
-This finishes the daemonization process by doing the other half of detaching
-from the parent process and its tty file descriptors.  */)
-  (void)
+static void
+daemon_check_preconditions (void)
 {
-  int nfd;
-  bool err = 0;
-
   if (!IS_DAEMON)
     error ("This function can only be called if emacs is run as a daemon");
 
-  if (daemon_pipe[1] < 0)
+#ifdef WINDOWSNT
+  /* IS_DAEMON above already checks that w32_daemon_event != NULL, so only
+     check that we successfully started the daemon here.  */
+  if (w32_daemon_event == INVALID_HANDLE_VALUE)
+#else
+  if (daemon_pipe[1] < 0 )
+#endif
     error ("The daemon has already been initialized");
 
   if (NILP (Vafter_init_time))
     error ("This function can only be called after loading the init files");
+}
+
+DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
+       doc: /* Mark the Emacs daemon as being initialized.
+This finishes the daemonization process by doing the other half of detaching
+from the parent process and its tty file descriptors.  */)
+  (void)
+{
+  daemon_check_preconditions ();
+#ifdef WINDOWSNT
+  /* Signal the waiting emacsclient process.  */
+  SetEvent (w32_daemon_event);
+  CloseHandle (w32_daemon_event);
+  w32_daemon_event = INVALID_HANDLE_VALUE;
+#else
+  int nfd;
+  bool err = 0;
 
   /* Get rid of stdin, stdout and stderr.  */
   nfd = emacs_open ("/dev/null", O_RDWR, 0);
@@ -2350,6 +2409,7 @@ from the parent process and its tty file descriptors.  */)
 
   if (err)
     error ("I/O error during daemon initialization");
+#endif
   return Qt;
 }
 
diff --git a/src/keyboard.c b/src/keyboard.c
index 383c109..8f0e18f 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3829,7 +3829,11 @@ kbd_buffer_get_event (KBOARD **kbp,
   if (noninteractive
       /* In case we are running as a daemon, only do this before
 	 detaching from the terminal.  */
+#ifdef WINDOWSNT
+      || (IS_DAEMON && w32_daemon_event != INVALID_HANDLE_VALUE))
+#else
       || (IS_DAEMON && daemon_pipe[1] >= 0))
+#endif
     {
       int c = getchar ();
       XSETINT (obj, c);
diff --git a/src/lisp.h b/src/lisp.h
index f5242ab..0b85817 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4215,9 +4215,14 @@ extern bool noninteractive;
 extern bool no_site_lisp;
 
 /* Pipe used to send exit notification to the daemon parent at
-   startup.  */
+   startup.  On Windows, we use a kernel event instead.  */
+#ifndef WINDOWSNT
 extern int daemon_pipe[2];
 #define IS_DAEMON (daemon_pipe[1] != 0)
+#elif defined(WINDOWSNT)
+extern void *w32_daemon_event;
+#define IS_DAEMON (w32_daemon_event != NULL)
+#endif
 
 /* True if handling a fatal error already.  */
 extern bool fatal_error_in_progress;
diff --git a/src/minibuf.c b/src/minibuf.c
index 3408bb9..dee7ce8 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -459,7 +459,11 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   if ((noninteractive
        /* In case we are running as a daemon, only do this before
 	  detaching from the terminal.  */
+#ifdef WINDOWSNT
+       || (IS_DAEMON && w32_daemon_event != INVALID_HANDLE_VALUE))
+#else
        || (IS_DAEMON && (daemon_pipe[1] >= 0)))
+#endif
       && NILP (Vexecuting_kbd_macro))
     {
       val = read_minibuf_noninteractive (map, initial, prompt,

[-- Attachment #3: emacs-windows-daemon-inheritev-cmdline.patch --]
[-- Type: application/octet-stream, Size: 14251 bytes --]

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index a04dda6..a9bce5a 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -595,13 +595,6 @@ decode_options (int argc, char **argv)
       display = NULL;
       tty = 1;
     }
-
-  if (alternate_editor && alternate_editor[0] == '\0')
-    {
-      message (true, "--alternate-editor argument or ALTERNATE_EDITOR variable cannot be\n\
-an empty string");
-      exit (EXIT_FAILURE);
-    }
 #endif /* WINDOWSNT */
 }
 
@@ -642,10 +635,8 @@ The following OPTIONS are accepted:\n\
 			Set filename of the TCP authentication file\n\
 -a EDITOR, --alternate-editor=EDITOR\n\
 			Editor to fallback to if the server is not running\n"
-#ifndef WINDOWSNT
 "			If EDITOR is the empty string, start Emacs in daemon\n\
 			mode and try connecting again\n"
-#endif /* not WINDOWSNT */
 "\n\
 Report bugs with M-x report-emacs-bug.\n");
   exit (EXIT_SUCCESS);
@@ -1459,9 +1450,65 @@ w32_give_focus (void)
 /* Start the emacs daemon and try to connect to it.  */
 
 static void
+connect_to_emacs_socket (void)
+{
+#ifdef WINDOWSNT
+  /* It's just a progress message, so don't pop a dialog if this is
+     emacsclientw.  */
+  if (!w32_window_app ())
+#endif
+  message (true, "Emacs daemon should have started, trying to connect again\n");
+  if ((emacs_socket = set_socket (1)) == INVALID_SOCKET)
+    {
+      message (true, "Error: Cannot connect even after starting the Emacs daemon\n");
+      exit (EXIT_FAILURE);
+    }
+}
+
+static void
 start_daemon_and_retry_set_socket (void)
 {
-#ifndef WINDOWSNT
+#ifdef WINDOWSNT
+  DWORD wait_result;
+  HANDLE w32_daemon_event;
+  STARTUPINFO si;
+  PROCESS_INFORMATION pi;
+  SECURITY_ATTRIBUTES sa;
+  char execstr[64];
+
+  ZeroMemory (&si, sizeof si);
+  si.cb = sizeof si;
+  ZeroMemory (&pi, sizeof pi);
+
+  sa.nLength = sizeof sa;
+  sa.lpSecurityDescriptor = NULL;
+  sa.bInheritHandle = TRUE;
+
+  w32_daemon_event = CreateEvent (&sa, TRUE, FALSE, NULL);
+  if (w32_daemon_event == NULL)
+    {
+      message (true, "Couldn't create Windows daemon event");
+      exit (EXIT_FAILURE);
+    }
+
+  snprintf (execstr, sizeof execstr, "emacs --daemon=*%p", w32_daemon_event);
+  if (!CreateProcess (NULL, execstr, NULL, NULL, TRUE, CREATE_NO_WINDOW, NULL,
+                      NULL, &si, &pi))
+    {
+      message (true, "%s: error starting emacs daemon\n", progname);
+      exit (EXIT_FAILURE);
+    }
+
+  if (WaitForSingleObject (w32_daemon_event, INFINITE) != WAIT_OBJECT_0)
+    {
+      message (true, "Error while waiting for Windows daemon event");
+      exit (EXIT_FAILURE);
+    }
+  CloseHandle (w32_daemon_event);
+
+  /* Try connecting, the daemon should have started by now.  */
+  connect_to_emacs_socket ();
+#elif !defined(WINDOWSNT)
   pid_t dpid;
   int status;
 
@@ -1479,12 +1526,7 @@ start_daemon_and_retry_set_socket (void)
 	}
 
       /* Try connecting, the daemon should have started by now.  */
-      message (true, "Emacs daemon should have started, trying to connect again\n");
-      if ((emacs_socket = set_socket (1)) == INVALID_SOCKET)
-	{
-	  message (true, "Error: Cannot connect even after starting the Emacs daemon\n");
-	  exit (EXIT_FAILURE);
-	}
+      connect_to_emacs_socket ();
     }
   else if (dpid < 0)
     {
@@ -1511,7 +1553,7 @@ start_daemon_and_retry_set_socket (void)
       execvp ("emacs", d_argv);
       message (true, "%s: error starting emacs daemon\n", progname);
     }
-#endif /* WINDOWSNT */
+#endif /* !WINDOWSNT */
 }
 
 int
diff --git a/lisp/frame.el b/lisp/frame.el
index 1d5bbf2..23bbc0d 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -536,7 +536,8 @@ is not considered (see `next-frame')."
 Return nil if we don't know how to interpret DISPLAY."
   ;; MS-Windows doesn't know how to create a GUI frame in a -nw session.
   (if (and (eq system-type 'windows-nt)
-	   (null (window-system)))
+	   (null (window-system))
+	   (not (daemonp)))
       nil
     (cl-loop for descriptor in display-format-alist
 	     for pattern = (car descriptor)
diff --git a/lisp/frameset.el b/lisp/frameset.el
index 4a06374..17fe39b 100644
--- a/lisp/frameset.el
+++ b/lisp/frameset.el
@@ -1022,8 +1022,8 @@ Internal use only."
 (defun frameset-keep-original-display-p (force-display)
   "True if saved frames' displays should be honored.
 For the meaning of FORCE-DISPLAY, see `frameset-restore'."
-  (cond ((daemonp) t)
-	((eq system-type 'windows-nt) nil) ;; Does ns support more than one display?
+  (cond ((eq system-type 'windows-nt) nil) ;; Does ns support more than one display?
+	((daemonp) t)
 	(t (not force-display))))
 
 (defun frameset-minibufferless-first-p (frame1 _frame2)
diff --git a/lisp/server.el b/lisp/server.el
index 166cd44..9585b17 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1139,9 +1139,12 @@ The following commands are accepted by the client:
                  ;; frame.  If running a GUI server, force the frame
                  ;; type to GUI.  (Cygwin is perfectly happy with
                  ;; multi-tty support, so don't override the user's
-                 ;; choice there.)
+                 ;; choice there.)  In daemon mode on Windows, we can't
+                 ;; make tty frames, so force the frame type to GUI
+                 ;; there too.
                  (when (and (eq system-type 'windows-nt)
-                            (eq window-system 'w32))
+                            (or (daemonp)
+                                (eq window-system 'w32)))
                    (push "-window-system" args-left)))
 
                 ;; -position LINE[:COLUMN]:  Set point to the given
@@ -1215,7 +1218,10 @@ The following commands are accepted by the client:
 					   terminal-frame)))))
 		    (setq tty-name nil tty-type nil)
 		    (if display (server-select-display display)))
-		   ((eq tty-name 'window-system)
+		   ((or (and (eq system-type 'windows-nt)
+			     (daemonp)
+			     (setq display "w32"))
+		        (eq tty-name 'window-system))
 		    (server-create-window-system-frame display nowait proc
 						       parent-id
 						       frame-parameters))
diff --git a/src/dispnew.c b/src/dispnew.c
index 3c8117e..e86fbb8 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -5974,9 +5974,12 @@ init_display (void)
     }
 #endif /* SIGWINCH */
 
-  /* If running as a daemon, no need to initialize any frames/terminal. */
+  /* If running as a daemon, no need to initialize any frames/terminal,
+     except on Windows, where we at least want to initialize it.  */
+#ifndef WINDOWSNT
   if (IS_DAEMON)
       return;
+#endif
 
   /* If the user wants to use a window system, we shouldn't bother
      initializing the terminal.  This is especially important when the
diff --git a/src/emacs.c b/src/emacs.c
index 345fe3e..d54643e 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -195,9 +195,13 @@ bool no_site_lisp;
 /* Name for the server started by the daemon.*/
 static char *daemon_name;
 
+#ifndef WINDOWSNT
 /* Pipe used to send exit notification to the daemon parent at
    startup.  */
 int daemon_pipe[2];
+#else
+HANDLE w32_daemon_event;
+#endif
 
 /* Save argv and argc.  */
 char **initial_argv;
@@ -687,6 +691,43 @@ close_output_streams (void)
      _exit (EXIT_FAILURE);
 }
 
+static void
+handle_dname_arg (char **dname_arg, char *dname_arg2, int sep)
+{
+  int has_dname = *dname_arg != NULL;
+  /* Were we called by emacsclient or did the user start the daemon?  */
+#ifdef WINDOWSNT
+  int user_started_daemon = has_dname && !strchr (*dname_arg, sep);
+  if (!has_dname || user_started_daemon)
+    {
+      w32_daemon_event = CreateEvent (NULL, TRUE, FALSE, NULL);
+      if (user_started_daemon)
+        /* We already have a name in dname_arg or none was given.  */
+        return;
+      else
+        goto process_dname;
+    }
+#endif
+  if (!has_dname || !strchr (*dname_arg, sep)
+      || strlen (*dname_arg) < 1 || strlen (*dname_arg) > 70)
+    {
+      fprintf (stderr, "emacs daemon: daemon name absent or too long\n");
+      exit (EXIT_CANNOT_INVOKE);
+    }
+
+process_dname:
+  dname_arg2[0] = '\0';
+#ifdef DAEMON_MUST_EXEC
+  sscanf (*dname_arg, "\n%d,%d\n%s", &(daemon_pipe[0]), &(daemon_pipe[1]),
+          dname_arg2);
+#elif defined(WINDOWSNT)
+  sscanf (*dname_arg, "*%p", &w32_daemon_event);
+#endif
+  /* On Windows, emacsclient will never pass a daemon name, so this will always
+     be NULL.  */
+  *dname_arg = *dname_arg2 ? dname_arg2 : NULL;
+}
+
 /* ARGSUSED */
 int
 main (int argc, char **argv)
@@ -704,9 +745,7 @@ main (int argc, char **argv)
   bool no_loadup = 0;
   char *junk = 0;
   char *dname_arg = 0;
-#ifdef DAEMON_MUST_EXEC
   char dname_arg2[80];
-#endif
   char *ch_to_dir = 0;
 
   /* If we use --chdir, this records the original directory.  */
@@ -980,8 +1019,12 @@ main (int argc, char **argv)
       exit (0);
     }
 
+#ifndef WINDOWSNT
   /* Make sure IS_DAEMON starts up as false.  */
   daemon_pipe[1] = 0;
+#else
+  w32_daemon_event = NULL;
+#endif
 
   if (argmatch (argv, argc, "-daemon", "--daemon", 5, NULL, &skip_args)
       || argmatch (argv, argc, "-daemon", "--daemon", 5, &dname_arg, &skip_args))
@@ -1090,17 +1133,7 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
 	    exit (errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE);
           }
 
-        /* In exec'd: parse special dname into pipe and name info. */
-        if (!dname_arg || !strchr (dname_arg, '\n')
-            || strlen (dname_arg) < 1 || strlen (dname_arg) > 70)
-          {
-            fprintf (stderr, "emacs daemon: daemon name absent or too long\n");
-            exit (EXIT_CANNOT_INVOKE);
-          }
-        dname_arg2[0] = '\0';
-        sscanf (dname_arg, "\n%d,%d\n%s", &(daemon_pipe[0]), &(daemon_pipe[1]),
-                dname_arg2);
-        dname_arg = *dname_arg2 ? dname_arg2 : NULL;
+        handle_dname_arg (&dname_arg, dname_arg2, '\n');
 	fcntl (daemon_pipe[1], F_SETFD, FD_CLOEXEC);
       }
 #endif /* DAEMON_MUST_EXEC */
@@ -1111,10 +1144,21 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
       emacs_close (daemon_pipe[0]);
 
       setsid ();
-#else /* DOS_NT */
+#elif defined(WINDOWSNT)
+      handle_dname_arg (&dname_arg, dname_arg2, '*');
+
+      if (w32_daemon_event == NULL)
+        {
+          fprintf (stderr, "Couldn't create or get event handle for daemon\n");
+          exit (1);
+        }
+
+      if (dname_arg)
+        daemon_name = xstrdup (dname_arg);
+#else /* MSDOS */
       fprintf (stderr, "This platform does not support the -daemon flag.\n");
       exit (1);
-#endif /* DOS_NT */
+#endif /* MSDOS */
     }
 
 #if defined HAVE_PTHREAD && !defined SYSTEM_MALLOC \
@@ -2310,23 +2354,40 @@ If the daemon was given a name argument, return that name. */)
     return Qnil;
 }
 
-DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
-       doc: /* Mark the Emacs daemon as being initialized.
-This finishes the daemonization process by doing the other half of detaching
-from the parent process and its tty file descriptors.  */)
-  (void)
+static void
+daemon_check_preconditions (void)
 {
-  int nfd;
-  bool err = 0;
-
   if (!IS_DAEMON)
     error ("This function can only be called if emacs is run as a daemon");
 
-  if (daemon_pipe[1] < 0)
+#ifdef WINDOWSNT
+  /* IS_DAEMON above already checks that w32_daemon_event != NULL, so only
+     check that we successfully started the daemon here.  */
+  if (w32_daemon_event == INVALID_HANDLE_VALUE)
+#else
+  if (daemon_pipe[1] < 0 )
+#endif
     error ("The daemon has already been initialized");
 
   if (NILP (Vafter_init_time))
     error ("This function can only be called after loading the init files");
+}
+
+DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
+       doc: /* Mark the Emacs daemon as being initialized.
+This finishes the daemonization process by doing the other half of detaching
+from the parent process and its tty file descriptors.  */)
+  (void)
+{
+  daemon_check_preconditions ();
+#ifdef WINDOWSNT
+  /* Signal the waiting emacsclient process.  */
+  SetEvent (w32_daemon_event);
+  CloseHandle (w32_daemon_event);
+  w32_daemon_event = INVALID_HANDLE_VALUE;
+#else
+  int nfd;
+  bool err = 0;
 
   /* Get rid of stdin, stdout and stderr.  */
   nfd = emacs_open ("/dev/null", O_RDWR, 0);
@@ -2350,6 +2411,7 @@ from the parent process and its tty file descriptors.  */)
 
   if (err)
     error ("I/O error during daemon initialization");
+#endif
   return Qt;
 }
 
diff --git a/src/keyboard.c b/src/keyboard.c
index 383c109..8f0e18f 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3829,7 +3829,11 @@ kbd_buffer_get_event (KBOARD **kbp,
   if (noninteractive
       /* In case we are running as a daemon, only do this before
 	 detaching from the terminal.  */
+#ifdef WINDOWSNT
+      || (IS_DAEMON && w32_daemon_event != INVALID_HANDLE_VALUE))
+#else
       || (IS_DAEMON && daemon_pipe[1] >= 0))
+#endif
     {
       int c = getchar ();
       XSETINT (obj, c);
diff --git a/src/lisp.h b/src/lisp.h
index f5242ab..0b85817 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4215,9 +4215,14 @@ extern bool noninteractive;
 extern bool no_site_lisp;
 
 /* Pipe used to send exit notification to the daemon parent at
-   startup.  */
+   startup.  On Windows, we use a kernel event instead.  */
+#ifndef WINDOWSNT
 extern int daemon_pipe[2];
 #define IS_DAEMON (daemon_pipe[1] != 0)
+#elif defined(WINDOWSNT)
+extern void *w32_daemon_event;
+#define IS_DAEMON (w32_daemon_event != NULL)
+#endif
 
 /* True if handling a fatal error already.  */
 extern bool fatal_error_in_progress;
diff --git a/src/minibuf.c b/src/minibuf.c
index 3408bb9..dee7ce8 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -459,7 +459,11 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   if ((noninteractive
        /* In case we are running as a daemon, only do this before
 	  detaching from the terminal.  */
+#ifdef WINDOWSNT
+       || (IS_DAEMON && w32_daemon_event != INVALID_HANDLE_VALUE))
+#else
        || (IS_DAEMON && (daemon_pipe[1] >= 0)))
+#endif
       && NILP (Vexecuting_kbd_macro))
     {
       val = read_minibuf_noninteractive (map, initial, prompt,

[-- Attachment #4: emacs-windows-daemon-namedev.patch --]
[-- Type: application/octet-stream, Size: 12039 bytes --]

diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index a04dda6..eeb9ce2 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -595,13 +595,6 @@ decode_options (int argc, char **argv)
       display = NULL;
       tty = 1;
     }
-
-  if (alternate_editor && alternate_editor[0] == '\0')
-    {
-      message (true, "--alternate-editor argument or ALTERNATE_EDITOR variable cannot be\n\
-an empty string");
-      exit (EXIT_FAILURE);
-    }
 #endif /* WINDOWSNT */
 }
 
@@ -642,10 +635,8 @@ The following OPTIONS are accepted:\n\
 			Set filename of the TCP authentication file\n\
 -a EDITOR, --alternate-editor=EDITOR\n\
 			Editor to fallback to if the server is not running\n"
-#ifndef WINDOWSNT
 "			If EDITOR is the empty string, start Emacs in daemon\n\
 			mode and try connecting again\n"
-#endif /* not WINDOWSNT */
 "\n\
 Report bugs with M-x report-emacs-bug.\n");
   exit (EXIT_SUCCESS);
@@ -1459,9 +1450,57 @@ w32_give_focus (void)
 /* Start the emacs daemon and try to connect to it.  */
 
 static void
+connect_to_emacs_socket (void)
+{
+#ifdef WINDOWSNT
+  /* It's just a progress message, so don't pop a dialog if this is
+     emacsclientw.  */
+  if (!w32_window_app ())
+#endif
+  message (true, "Emacs daemon should have started, trying to connect again\n");
+  if ((emacs_socket = set_socket (1)) == INVALID_SOCKET)
+    {
+      message (true, "Error: Cannot connect even after starting the Emacs daemon\n");
+      exit (EXIT_FAILURE);
+    }
+}
+
+static void
 start_daemon_and_retry_set_socket (void)
 {
-#ifndef WINDOWSNT
+#ifdef WINDOWSNT
+  DWORD wait_result;
+  HANDLE w32_daemon_event;
+  STARTUPINFO si;
+  PROCESS_INFORMATION pi;
+
+  ZeroMemory (&si, sizeof si);
+  si.cb = sizeof si;
+  ZeroMemory (&pi, sizeof pi);
+
+  if (!CreateProcess (NULL, "emacs --daemon", NULL, NULL, FALSE,
+                      CREATE_NO_WINDOW, NULL, NULL, &si, &pi))
+    {
+      message (true, "%s: error starting emacs daemon\n", progname);
+      exit (EXIT_FAILURE);
+    }
+
+  w32_daemon_event = CreateEvent (NULL, TRUE, FALSE, W32_DAEMON_EVENT);
+  if (w32_daemon_event == NULL)
+    {
+      message (true, "Couldn't create Windows daemon event");
+      exit (EXIT_FAILURE);
+    }
+  if (WaitForSingleObject (w32_daemon_event, INFINITE) != WAIT_OBJECT_0)
+    {
+      message (true, "Error while waiting for Windows daemon event");
+      exit (EXIT_FAILURE);
+    }
+  CloseHandle (w32_daemon_event);
+
+  /* Try connecting, the daemon should have started by now.  */
+  connect_to_emacs_socket ();
+#elif !defined(WINDOWSNT)
   pid_t dpid;
   int status;
 
@@ -1479,12 +1518,7 @@ start_daemon_and_retry_set_socket (void)
 	}
 
       /* Try connecting, the daemon should have started by now.  */
-      message (true, "Emacs daemon should have started, trying to connect again\n");
-      if ((emacs_socket = set_socket (1)) == INVALID_SOCKET)
-	{
-	  message (true, "Error: Cannot connect even after starting the Emacs daemon\n");
-	  exit (EXIT_FAILURE);
-	}
+      connect_to_emacs_socket ();
     }
   else if (dpid < 0)
     {
@@ -1511,7 +1545,7 @@ start_daemon_and_retry_set_socket (void)
       execvp ("emacs", d_argv);
       message (true, "%s: error starting emacs daemon\n", progname);
     }
-#endif /* WINDOWSNT */
+#endif /* !WINDOWSNT */
 }
 
 int
diff --git a/lisp/frame.el b/lisp/frame.el
index 1d5bbf2..23bbc0d 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -536,7 +536,8 @@ is not considered (see `next-frame')."
 Return nil if we don't know how to interpret DISPLAY."
   ;; MS-Windows doesn't know how to create a GUI frame in a -nw session.
   (if (and (eq system-type 'windows-nt)
-	   (null (window-system)))
+	   (null (window-system))
+	   (not (daemonp)))
       nil
     (cl-loop for descriptor in display-format-alist
 	     for pattern = (car descriptor)
diff --git a/lisp/frameset.el b/lisp/frameset.el
index 4a06374..17fe39b 100644
--- a/lisp/frameset.el
+++ b/lisp/frameset.el
@@ -1022,8 +1022,8 @@ Internal use only."
 (defun frameset-keep-original-display-p (force-display)
   "True if saved frames' displays should be honored.
 For the meaning of FORCE-DISPLAY, see `frameset-restore'."
-  (cond ((daemonp) t)
-	((eq system-type 'windows-nt) nil) ;; Does ns support more than one display?
+  (cond ((eq system-type 'windows-nt) nil) ;; Does ns support more than one display?
+	((daemonp) t)
 	(t (not force-display))))
 
 (defun frameset-minibufferless-first-p (frame1 _frame2)
diff --git a/lisp/server.el b/lisp/server.el
index 166cd44..9585b17 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -1139,9 +1139,12 @@ The following commands are accepted by the client:
                  ;; frame.  If running a GUI server, force the frame
                  ;; type to GUI.  (Cygwin is perfectly happy with
                  ;; multi-tty support, so don't override the user's
-                 ;; choice there.)
+                 ;; choice there.)  In daemon mode on Windows, we can't
+                 ;; make tty frames, so force the frame type to GUI
+                 ;; there too.
                  (when (and (eq system-type 'windows-nt)
-                            (eq window-system 'w32))
+                            (or (daemonp)
+                                (eq window-system 'w32)))
                    (push "-window-system" args-left)))
 
                 ;; -position LINE[:COLUMN]:  Set point to the given
@@ -1215,7 +1218,10 @@ The following commands are accepted by the client:
 					   terminal-frame)))))
 		    (setq tty-name nil tty-type nil)
 		    (if display (server-select-display display)))
-		   ((eq tty-name 'window-system)
+		   ((or (and (eq system-type 'windows-nt)
+			     (daemonp)
+			     (setq display "w32"))
+		        (eq tty-name 'window-system))
 		    (server-create-window-system-frame display nowait proc
 						       parent-id
 						       frame-parameters))
diff --git a/nt/inc/ms-w32.h b/nt/inc/ms-w32.h
index adac2e3..8439163 100644
--- a/nt/inc/ms-w32.h
+++ b/nt/inc/ms-w32.h
@@ -597,5 +597,7 @@ extern void _DebPrint (const char *fmt, ...);
 #endif
 #endif
 
+/* Event name for when emacsclient starts the Emacs daemon on Windows.  */
+#define W32_DAEMON_EVENT "EmacsServerEvent"
 
 /* ============================================================ */
diff --git a/src/dispnew.c b/src/dispnew.c
index 3c8117e..e86fbb8 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -5974,9 +5974,12 @@ init_display (void)
     }
 #endif /* SIGWINCH */
 
-  /* If running as a daemon, no need to initialize any frames/terminal. */
+  /* If running as a daemon, no need to initialize any frames/terminal,
+     except on Windows, where we at least want to initialize it.  */
+#ifndef WINDOWSNT
   if (IS_DAEMON)
       return;
+#endif
 
   /* If the user wants to use a window system, we shouldn't bother
      initializing the terminal.  This is especially important when the
diff --git a/src/emacs.c b/src/emacs.c
index 345fe3e..2932e36 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -195,9 +195,13 @@ bool no_site_lisp;
 /* Name for the server started by the daemon.*/
 static char *daemon_name;
 
+#ifndef WINDOWSNT
 /* Pipe used to send exit notification to the daemon parent at
    startup.  */
 int daemon_pipe[2];
+#else
+HANDLE w32_daemon_event;
+#endif
 
 /* Save argv and argc.  */
 char **initial_argv;
@@ -980,8 +984,12 @@ main (int argc, char **argv)
       exit (0);
     }
 
+#ifndef WINDOWSNT
   /* Make sure IS_DAEMON starts up as false.  */
   daemon_pipe[1] = 0;
+#else
+  w32_daemon_event = NULL;
+#endif
 
   if (argmatch (argv, argc, "-daemon", "--daemon", 5, NULL, &skip_args)
       || argmatch (argv, argc, "-daemon", "--daemon", 5, &dname_arg, &skip_args))
@@ -1111,10 +1119,20 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
       emacs_close (daemon_pipe[0]);
 
       setsid ();
-#else /* DOS_NT */
+#elif defined(WINDOWSNT)
+      /* Indicate that we want daemon mode.  */
+      w32_daemon_event = CreateEvent (NULL, TRUE, FALSE, W32_DAEMON_EVENT);
+      if (w32_daemon_event == NULL)
+        {
+          fprintf (stderr, "Couldn't create event for Windows daemon\n");
+          exit (1);
+        }
+      if (dname_arg)
+        daemon_name = xstrdup (dname_arg);
+#else /* MSDOS */
       fprintf (stderr, "This platform does not support the -daemon flag.\n");
       exit (1);
-#endif /* DOS_NT */
+#endif /* MSDOS */
     }
 
 #if defined HAVE_PTHREAD && !defined SYSTEM_MALLOC \
@@ -2310,23 +2328,40 @@ If the daemon was given a name argument, return that name. */)
     return Qnil;
 }
 
-DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
-       doc: /* Mark the Emacs daemon as being initialized.
-This finishes the daemonization process by doing the other half of detaching
-from the parent process and its tty file descriptors.  */)
-  (void)
+static void
+daemon_check_preconditions (void)
 {
-  int nfd;
-  bool err = 0;
-
   if (!IS_DAEMON)
     error ("This function can only be called if emacs is run as a daemon");
 
-  if (daemon_pipe[1] < 0)
+#ifdef WINDOWSNT
+  /* IS_DAEMON above already checks that w32_daemon_event != NULL, so only
+     check that we successfully started the daemon here.  */
+  if (w32_daemon_event == INVALID_HANDLE_VALUE)
+#else
+  if (daemon_pipe[1] < 0 )
+#endif
     error ("The daemon has already been initialized");
 
   if (NILP (Vafter_init_time))
     error ("This function can only be called after loading the init files");
+}
+
+DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
+       doc: /* Mark the Emacs daemon as being initialized.
+This finishes the daemonization process by doing the other half of detaching
+from the parent process and its tty file descriptors.  */)
+  (void)
+{
+  daemon_check_preconditions ();
+#ifdef WINDOWSNT
+  /* Signal the waiting emacsclient process.  */
+  SetEvent (w32_daemon_event);
+  CloseHandle (w32_daemon_event);
+  w32_daemon_event = INVALID_HANDLE_VALUE;
+#else
+  int nfd;
+  bool err = 0;
 
   /* Get rid of stdin, stdout and stderr.  */
   nfd = emacs_open ("/dev/null", O_RDWR, 0);
@@ -2350,6 +2385,7 @@ from the parent process and its tty file descriptors.  */)
 
   if (err)
     error ("I/O error during daemon initialization");
+#endif
   return Qt;
 }
 
diff --git a/src/keyboard.c b/src/keyboard.c
index 383c109..8f0e18f 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -3829,7 +3829,11 @@ kbd_buffer_get_event (KBOARD **kbp,
   if (noninteractive
       /* In case we are running as a daemon, only do this before
 	 detaching from the terminal.  */
+#ifdef WINDOWSNT
+      || (IS_DAEMON && w32_daemon_event != INVALID_HANDLE_VALUE))
+#else
       || (IS_DAEMON && daemon_pipe[1] >= 0))
+#endif
     {
       int c = getchar ();
       XSETINT (obj, c);
diff --git a/src/lisp.h b/src/lisp.h
index f5242ab..0b85817 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4215,9 +4215,14 @@ extern bool noninteractive;
 extern bool no_site_lisp;
 
 /* Pipe used to send exit notification to the daemon parent at
-   startup.  */
+   startup.  On Windows, we use a kernel event instead.  */
+#ifndef WINDOWSNT
 extern int daemon_pipe[2];
 #define IS_DAEMON (daemon_pipe[1] != 0)
+#elif defined(WINDOWSNT)
+extern void *w32_daemon_event;
+#define IS_DAEMON (w32_daemon_event != NULL)
+#endif
 
 /* True if handling a fatal error already.  */
 extern bool fatal_error_in_progress;
diff --git a/src/minibuf.c b/src/minibuf.c
index 3408bb9..dee7ce8 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -459,7 +459,11 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   if ((noninteractive
        /* In case we are running as a daemon, only do this before
 	  detaching from the terminal.  */
+#ifdef WINDOWSNT
+       || (IS_DAEMON && w32_daemon_event != INVALID_HANDLE_VALUE))
+#else
        || (IS_DAEMON && (daemon_pipe[1] >= 0)))
+#endif
       && NILP (Vexecuting_kbd_macro))
     {
       val = read_minibuf_noninteractive (map, initial, prompt,

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-01-27  8:40           ` Mark Laws
@ 2015-01-30  0:36             ` Mark Laws
  2015-01-30  6:28               ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-01-30  0:36 UTC (permalink / raw)
  To: Daniel Colascione; +Cc: 19688

I just emailed a signed copy of the copyright assignment form to the
clerk, so that should be taken care of now. Did either of you have a
chance to check the latest patch(es) I sent?

-- 
|v\ /\ |\ |< |_ /\ \^| //





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-01-30  0:36             ` Mark Laws
@ 2015-01-30  6:28               ` Eli Zaretskii
  2015-02-13  0:07                 ` Mark Laws
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2015-01-30  6:28 UTC (permalink / raw)
  To: Mark Laws; +Cc: 19688

> Date: Fri, 30 Jan 2015 09:36:25 +0900
> From: Mark Laws <mdl@60hz.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, 19688@debbugs.gnu.org
> 
> I just emailed a signed copy of the copyright assignment form to the
> clerk, so that should be taken care of now.

Thanks.  We need to wait till their process is complete, though.

> Did either of you have a chance to check the latest patch(es) I
> sent?

Not yet.  Will do so soon.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-01-30  6:28               ` Eli Zaretskii
@ 2015-02-13  0:07                 ` Mark Laws
  2015-02-13  8:49                   ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-02-13  0:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 19688

Any chance to check out the patches yet?

-- 
|v\ /\ |\ |< |_ /\ \^| //





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-13  0:07                 ` Mark Laws
@ 2015-02-13  8:49                   ` Eli Zaretskii
  2015-02-14 12:10                     ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2015-02-13  8:49 UTC (permalink / raw)
  To: Mark Laws; +Cc: 19688

> Date: Fri, 13 Feb 2015 09:07:04 +0900
> From: Mark Laws <mdl@60hz.org>
> Cc: Daniel Colascione <dancol@dancol.org>, 19688@debbugs.gnu.org
> 
> Any chance to check out the patches yet?

It's on my todo.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-13  8:49                   ` Eli Zaretskii
@ 2015-02-14 12:10                     ` Eli Zaretskii
  2015-02-14 13:16                       ` Mark Laws
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2015-02-14 12:10 UTC (permalink / raw)
  To: mdl; +Cc: 19688

> Date: Fri, 13 Feb 2015 10:49:31 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 19688@debbugs.gnu.org
> 
> > Date: Fri, 13 Feb 2015 09:07:04 +0900
> > From: Mark Laws <mdl@60hz.org>
> > Cc: Daniel Colascione <dancol@dancol.org>, 19688@debbugs.gnu.org
> > 
> > Any chance to check out the patches yet?
> 
> It's on my todo.

Sorry for the long delay (life intervened).

After reading the last messages, I must ask that we back up a notch
and discuss something I'm not sure I understand correctly.
Specifically, this issue:

> >> >> +#define W32_EMACS_SERVER_GUID "{0B8E5DCB-D7CF-4423-A9F1-2F6927F0D318}"
> >> >
> >> > Where did this GUID come from?
> >> >
> >> I generated it myself.
> >
> > Is that safe?  Do we care whether this GUID is globally unique?  Why
> > exactly do we need it to begin with?
> 
> It should be safe. On UNIX, Emacs uses a pipe to tell emacsclient when
> it's done initializing. On Windows, since we don't have fork, the
> easiest options are either a named event object[1] or specifying that
> the child process inherit the event handle in CreateProcess.

Maybe I'm missing something, but I don't see a pipe being used on Unix
for synchronization between emacsclient and the daemon started by
emacsclient.  Rather, on Unix we do this:

    dpid = fork ();

    if (dpid > 0)
      {
	pid_t w;
	w = waitpid (dpid, &status, WUNTRACED | WCONTINUED);

	if ((w == -1) || !WIFEXITED (status) || WEXITSTATUS (status))
	  {
	    message (true, "Error: Could not start the Emacs daemon\n");
	    exit (EXIT_FAILURE);
	  }

	/* Try connecting, the daemon should have started by now.  */
	message (true, "Emacs daemon should have started, trying to connect again\n");
	if ((emacs_socket = set_socket (1)) == INVALID_SOCKET)

My reading of this is that we use 'waitpid' to tell us when the daemon
has started and is ready to receive our connection.  There's no pipe
involved here, AFAICT.  Am I missing something?

My understanding is that your Windows variant of the above is to wait
on an event that is signaled by Emacs when it starts in daemon mode.
My question is: can we use something similar to Unix here, like
'WaitForInputIdle'?  After all, the above call to 'waitpid' just tells
us the daemon process is past its initialization stage, as far as the
OS is concerned, which isn't too fine-grained.  Perhaps even
repeatedly calling 'GetExitCodeProcess' until it returns STILL_ACTIVE
for the first time would be a faithful enough emulation of what
'waitpid' does here?

Could you please try these techniques and see if they do the job?  If
they do, we can get rid of the event stuff, I think.

Thanks.

P.S. What's up with your copyright assignment?  I still don't see it
on file.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 12:10                     ` Eli Zaretskii
@ 2015-02-14 13:16                       ` Mark Laws
  2015-02-14 13:28                         ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-02-14 13:16 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 19688

On Sat, Feb 14, 2015 at 9:10 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> Maybe I'm missing something, but I don't see a pipe being used on Unix
> for synchronization between emacsclient and the daemon started by
> emacsclient.  Rather, on Unix we do this:
>
>     dpid = fork ();
>
>     if (dpid > 0)
>       {
>         pid_t w;
>         w = waitpid (dpid, &status, WUNTRACED | WCONTINUED);
>
>         if ((w == -1) || !WIFEXITED (status) || WEXITSTATUS (status))
>           {
>             message (true, "Error: Could not start the Emacs daemon\n");
>             exit (EXIT_FAILURE);
>           }
>
>         /* Try connecting, the daemon should have started by now.  */
>         message (true, "Emacs daemon should have started, trying to connect again\n");
>         if ((emacs_socket = set_socket (1)) == INVALID_SOCKET)
>
> My reading of this is that we use 'waitpid' to tell us when the daemon
> has started and is ready to receive our connection.  There's no pipe
> involved here, AFAICT.  Am I missing something?

On Unix, emacsclient forks and execs (#1) emacs --daemon, which in
turn execs (#2) itself. emacsclient waits on #1, which is told when to
quit via the pipe between #1 and #2, and when it quits, we know that
the daemon must be ready.

> My understanding is that your Windows variant of the above is to wait
> on an event that is signaled by Emacs when it starts in daemon mode.
> My question is: can we use something similar to Unix here, like
> 'WaitForInputIdle'?  After all, the above call to 'waitpid' just tells
> us the daemon process is past its initialization stage, as far as the
> OS is concerned, which isn't too fine-grained.  Perhaps even
> repeatedly calling 'GetExitCodeProcess' until it returns STILL_ACTIVE
> for the first time would be a faithful enough emulation of what
> 'waitpid' does here?

On Windows, the Emacs process created by emacsclient initializes
daemon mode itself, so neither of those would give us a way of knowing
that the daemon has actually been initialized. That's why we have to
use an event on Windows. In other words, the reason we can just use
waitpid in emacsclient on Unix is because the act of waiting on the
daemon to start is happening *between the two forked emacs children*,
whereas on Windows, the waiting is happening directly between
emacsclient and emacs.

> P.S. What's up with your copyright assignment?  I still don't see it
> on file.

Sorry, I sent it to copyright-clerk@fsf.org (since that's where it
came from) instead of assign@gnu.org. I sent it to the latter just
now.

-- 
|v\ /\ |\ |< |_ /\ \^| //





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 13:16                       ` Mark Laws
@ 2015-02-14 13:28                         ` Eli Zaretskii
  2015-02-14 13:37                           ` Mark Laws
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2015-02-14 13:28 UTC (permalink / raw)
  To: Mark Laws; +Cc: 19688

> Date: Sat, 14 Feb 2015 22:16:46 +0900
> From: Mark Laws <mdl@60hz.org>
> Cc: 19688@debbugs.gnu.org
> 
> On Unix, emacsclient forks and execs (#1) emacs --daemon, which in
> turn execs (#2) itself. emacsclient waits on #1, which is told when to
> quit via the pipe between #1 and #2, and when it quits, we know that
> the daemon must be ready.
> 
> > My understanding is that your Windows variant of the above is to wait
> > on an event that is signaled by Emacs when it starts in daemon mode.
> > My question is: can we use something similar to Unix here, like
> > 'WaitForInputIdle'?  After all, the above call to 'waitpid' just tells
> > us the daemon process is past its initialization stage, as far as the
> > OS is concerned, which isn't too fine-grained.  Perhaps even
> > repeatedly calling 'GetExitCodeProcess' until it returns STILL_ACTIVE
> > for the first time would be a faithful enough emulation of what
> > 'waitpid' does here?
> 
> On Windows, the Emacs process created by emacsclient initializes
> daemon mode itself, so neither of those would give us a way of knowing
> that the daemon has actually been initialized. That's why we have to
> use an event on Windows.

I don't see why 'WaitForInputIdle' wouldn't work.  Can you explain?
AFAIU, it waits until the process is idle, which means it did all the
initialization and is ready for accepting connections.  Am I missing
something?

> > P.S. What's up with your copyright assignment?  I still don't see it
> > on file.
> 
> Sorry, I sent it to copyright-clerk@fsf.org (since that's where it
> came from) instead of assign@gnu.org. I sent it to the latter just
> now.

Thanks.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 13:28                         ` Eli Zaretskii
@ 2015-02-14 13:37                           ` Mark Laws
  2015-02-14 15:24                             ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-02-14 13:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 19688

On Sat, Feb 14, 2015 at 10:28 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> I don't see why 'WaitForInputIdle' wouldn't work.  Can you explain?
> AFAIU, it waits until the process is idle, which means it did all the
> initialization and is ready for accepting connections.  Am I missing
> something?

http://blogs.msdn.com/b/oldnewthing/archive/2010/03/26/9985422.aspx

I can try it if you want, but it seems fragile compared to the event thing.

-- 
|v\ /\ |\ |< |_ /\ \^| //





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 13:37                           ` Mark Laws
@ 2015-02-14 15:24                             ` Eli Zaretskii
  2015-02-14 16:34                               ` Mark Laws
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2015-02-14 15:24 UTC (permalink / raw)
  To: Mark Laws; +Cc: 19688

> Date: Sat, 14 Feb 2015 22:37:55 +0900
> From: Mark Laws <mdl@60hz.org>
> Cc: 19688@debbugs.gnu.org
> 
> On Sat, Feb 14, 2015 at 10:28 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> > I don't see why 'WaitForInputIdle' wouldn't work.  Can you explain?
> > AFAIU, it waits until the process is idle, which means it did all the
> > initialization and is ready for accepting connections.  Am I missing
> > something?
> 
> http://blogs.msdn.com/b/oldnewthing/archive/2010/03/26/9985422.aspx

Thanks, it's good to know about those subtleties.

However, after looking at the Emacs initialization code, I think those
problems will not affect Emacs, because we only start the thread that
reads Windows messages inside the call to Frecursive_edit, and by that
time the daemon initialization is already done.

OTOH, there's one other caveat in the documentation of
'WaitForInputIdle': it returns immediately "if the process is a
console application or does not have a message queue".  Emacs is built
as a console application, and I'm not sure at which point it begins to
"have a message queue", as far as this function is concerned.

> I can try it if you want, but it seems fragile compared to the event thing.

Please do try it, I think it will either work reliably or not at all.

The reason I'd prefer to use it if it works is because doing so
side-steps the entire issue of how to pass the event ID, which,
together with the code which handles the event, adds non-trivial
amount of code to your patch.  It would be nice to avoid that, if
possible.

Thanks.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 15:24                             ` Eli Zaretskii
@ 2015-02-14 16:34                               ` Mark Laws
  2015-02-14 16:53                                 ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-02-14 16:34 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 19688

On Sun, Feb 15, 2015 at 12:24 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> However, after looking at the Emacs initialization code, I think those
> problems will not affect Emacs, because we only start the thread that
> reads Windows messages inside the call to Frecursive_edit, and by that
> time the daemon initialization is already done.
>
> OTOH, there's one other caveat in the documentation of
> 'WaitForInputIdle': it returns immediately "if the process is a
> console application or does not have a message queue".  Emacs is built
> as a console application, and I'm not sure at which point it begins to
> "have a message queue", as far as this function is concerned.
>
>> I can try it if you want, but it seems fragile compared to the event thing.
>
> Please do try it, I think it will either work reliably or not at all.
>
> The reason I'd prefer to use it if it works is because doing so
> side-steps the entire issue of how to pass the event ID, which,
> together with the code which handles the event, adds non-trivial
> amount of code to your patch.  It would be nice to avoid that, if
> possible.

I gave it a try, but it doesn't work--most likely because, as you
said, Emacs is built as a console application, so WaitForInputIdle
simply returns immediately because of the race between emacsclient
attempting to wait on Emacs and Emacs establishing a Windows message
queue. So, the options are either using an event or modifying Emacs to
run under the Windows subsystem--I don't think I need to tell you
which one is the less invasive change. :)

-- 
|v\ /\ |\ |< |_ /\ \^| //





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 16:34                               ` Mark Laws
@ 2015-02-14 16:53                                 ` Eli Zaretskii
  2015-02-14 16:57                                   ` Mark Laws
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2015-02-14 16:53 UTC (permalink / raw)
  To: Mark Laws; +Cc: 19688

> Date: Sun, 15 Feb 2015 01:34:48 +0900
> From: Mark Laws <mdl@60hz.org>
> Cc: 19688@debbugs.gnu.org
> 
> I gave it a try, but it doesn't work--most likely because, as you
> said, Emacs is built as a console application, so WaitForInputIdle
> simply returns immediately because of the race between emacsclient
> attempting to wait on Emacs and Emacs establishing a Windows message
> queue.

No, I think it returns immediately, period.  As I wrote, when Emacs
establishes the input queue, it's already way past the daemon
initialization.

> So, the options are either using an event or modifying Emacs to
> run under the Windows subsystem--I don't think I need to tell you
> which one is the less invasive change. :)

The latter is not invasive, it simply cannot be done without losing
some very valuable features.  So it's out of the question.

Back to the event-based method: is using a named event with a fixed
name workable?  That is, is a situation imaginable where we would need
more than one event at the same time on the same system?

If a single event is enough, I think a named event with a name
specific to Emacs (but not a GUID) is a better alternative.
Otherwise, we will have to pass the handle via the command line; using
the environment is not an idea I like, because it means complications:
possible conflict with other variables, possible overflow of the 32K
limit on environment variables, possible conflicts in nested
invocations, etc.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 16:53                                 ` Eli Zaretskii
@ 2015-02-14 16:57                                   ` Mark Laws
  2015-02-14 17:23                                     ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-02-14 16:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 19688

On Sun, Feb 15, 2015 at 1:53 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> Back to the event-based method: is using a named event with a fixed
> name workable?  That is, is a situation imaginable where we would need
> more than one event at the same time on the same system?

Even with multiple users logged in (the RDP/Terminal Services case),
using the same event name won't cause a problem.

> If a single event is enough, I think a named event with a name
> specific to Emacs (but not a GUID) is a better alternative.
> Otherwise, we will have to pass the handle via the command line; using
> the environment is not an idea I like, because it means complications:
> possible conflict with other variables, possible overflow of the 32K
> limit on environment variables, possible conflicts in nested
> invocations, etc.

A single event will be fine, so I would suggest going with the named
event for the reasons you just gave. The code is much simpler that way
too.

-- 
|v\ /\ |\ |< |_ /\ \^| //





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 16:57                                   ` Mark Laws
@ 2015-02-14 17:23                                     ` Eli Zaretskii
  2015-02-14 17:30                                       ` Mark Laws
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2015-02-14 17:23 UTC (permalink / raw)
  To: Mark Laws; +Cc: 19688

> Date: Sun, 15 Feb 2015 01:57:58 +0900
> From: Mark Laws <mdl@60hz.org>
> Cc: 19688@debbugs.gnu.org
> 
> A single event will be fine, so I would suggest going with the named
> event for the reasons you just gave. The code is much simpler that way
> too.

OK, so please let's see a patch along these lines, with all the
previous comments incorporated.

Thanks.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 17:23                                     ` Eli Zaretskii
@ 2015-02-14 17:30                                       ` Mark Laws
  2015-02-14 17:42                                         ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-02-14 17:30 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 19688

On Sun, Feb 15, 2015 at 2:23 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> OK, so please let's see a patch along these lines, with all the
> previous comments incorporated.

It's here, attached as "emacs-windows-daemon-namedev.patch":

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=19688#20

-- 
|v\ /\ |\ |< |_ /\ \^| //





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 17:30                                       ` Mark Laws
@ 2015-02-14 17:42                                         ` Eli Zaretskii
  2015-02-14 17:57                                           ` Mark Laws
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2015-02-14 17:42 UTC (permalink / raw)
  To: Mark Laws; +Cc: 19688

> Date: Sun, 15 Feb 2015 02:30:54 +0900
> From: Mark Laws <mdl@60hz.org>
> Cc: 19688@debbugs.gnu.org
> 
> On Sun, Feb 15, 2015 at 2:23 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> > OK, so please let's see a patch along these lines, with all the
> > previous comments incorporated.
> 
> It's here, attached as "emacs-windows-daemon-namedev.patch":
> 
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=19688#20

OK, I have only one comment to that: why did you need to make part of
daemon-initialized a separate function?

In general, it looks to me that we should make a separate
implementation of daemon-initialized for Windows, and put it on one of
the w32*.c files.

Other than that, we are waiting for your assignment.

Thanks.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 17:42                                         ` Eli Zaretskii
@ 2015-02-14 17:57                                           ` Mark Laws
  2015-02-14 18:26                                             ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-02-14 17:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 19688

On Sun, Feb 15, 2015 at 2:42 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> OK, I have only one comment to that: why did you need to make part of
> daemon-initialized a separate function?

That was because of the pre-C99 concern I mentioned earlier in the
thread, but since you said emacs-25 will require a C99-compliant
compiler, it shouldn't be necessary. However, see my next comment.

> In general, it looks to me that we should make a separate
> implementation of daemon-initialized for Windows, and put it on one of
> the w32*.c files.

Given that the Windows implementation of the function is three lines
long and consists solely of side effects that are already localized to
emacs.c, I think it's much clearer to keep it as-is.

> Other than that, we are waiting for your assignment.

OK. It should be waiting for whoever reads assign@gnu.org.

-- 
|v\ /\ |\ |< |_ /\ \^| //





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 17:57                                           ` Mark Laws
@ 2015-02-14 18:26                                             ` Eli Zaretskii
  2015-02-14 19:21                                               ` Mark Laws
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2015-02-14 18:26 UTC (permalink / raw)
  To: Mark Laws; +Cc: 19688

> Date: Sun, 15 Feb 2015 02:57:13 +0900
> From: Mark Laws <mdl@60hz.org>
> Cc: 19688@debbugs.gnu.org
> 
> > In general, it looks to me that we should make a separate
> > implementation of daemon-initialized for Windows, and put it on one of
> > the w32*.c files.
> 
> Given that the Windows implementation of the function is three lines
> long and consists solely of side effects that are already localized to
> emacs.c, I think it's much clearer to keep it as-is.

Length of the function is not my concern.  The concern is to keep
stuff that needs windows.h confined to w32*.c source files.  It's true
that currently emacs.c already includes windows.h indirectly, but I'd
like to keep that to a bare minimum.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 18:26                                             ` Eli Zaretskii
@ 2015-02-14 19:21                                               ` Mark Laws
  2015-02-14 19:29                                                 ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-02-14 19:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 19688

On Sun, Feb 15, 2015 at 3:26 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> Length of the function is not my concern.  The concern is to keep
> stuff that needs windows.h confined to w32*.c source files.  It's true
> that currently emacs.c already includes windows.h indirectly, but I'd
> like to keep that to a bare minimum.

Ah, I see. In that case,the best place for this appears to be w32.c.
Do you want to move it or should I submit another patch?

-- 
|v\ /\ |\ |< |_ /\ \^| //





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 19:21                                               ` Mark Laws
@ 2015-02-14 19:29                                                 ` Eli Zaretskii
  2015-02-14 21:15                                                   ` Mark Laws
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2015-02-14 19:29 UTC (permalink / raw)
  To: Mark Laws; +Cc: 19688

> Date: Sun, 15 Feb 2015 04:21:20 +0900
> From: Mark Laws <mdl@60hz.org>
> Cc: 19688@debbugs.gnu.org
> 
> On Sun, Feb 15, 2015 at 3:26 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> > Length of the function is not my concern.  The concern is to keep
> > stuff that needs windows.h confined to w32*.c source files.  It's true
> > that currently emacs.c already includes windows.h indirectly, but I'd
> > like to keep that to a bare minimum.
> 
> Ah, I see. In that case,the best place for this appears to be w32.c.
> Do you want to move it or should I submit another patch?

It's up to you.  It's no big deal either way.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 19:29                                                 ` Eli Zaretskii
@ 2015-02-14 21:15                                                   ` Mark Laws
  2015-02-19 16:31                                                     ` Mark Laws
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-02-14 21:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 19688

On Sun, Feb 15, 2015 at 4:29 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> It's up to you.  It's no big deal either way.

Well, we'd need to also split the Windows part of
daemon_check_preconditions into a one-line function just to avoid
having INVALID_HANDLE_VALUE in emacs.c:

#ifdef WINDOWSNT
  /* IS_DAEMON above already checks that w32_daemon_event != NULL, so only
     check that we successfully started the daemon here.  */
  if (w32_daemon_event == INVALID_HANDLE_VALUE)
#else
  if (daemon_pipe[1] < 0 )
#endif

...and we'd have to move the code earlier in the file that calls
CreateEvent. Also, since w32.c seems to mostly be a place for utility
functions (it does not contain state variables), and the Unix
daemon_pipe stuff is in emacs.c, it would be weird to put the Windows
daemon variable in a completely different place.

Basically, it doesn't really seem worth complicating the general flow
of emacs.c for the sake of moving a few Windows-isms when w32.h is
already included anyway--not to mention, there's already other
OS-specific code throughout the file.

Sorry if I've misinterpreted you, i.e. if what you meant was "please
just move the Windows junk to w32.c", in which case I'll get on it
later today. But otherwise, I think the current patch makes the code
easier to follow.

-- 
|v\ /\ |\ |< |_ /\ \^| //





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-14 21:15                                                   ` Mark Laws
@ 2015-02-19 16:31                                                     ` Mark Laws
  2015-02-19 16:56                                                       ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-02-19 16:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 19688

FYI, I just received notice of completion of my assignment.

-- 
|v\ /\ |\ |< |_ /\ \^| //





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-19 16:31                                                     ` Mark Laws
@ 2015-02-19 16:56                                                       ` Eli Zaretskii
  2015-02-21 13:03                                                         ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2015-02-19 16:56 UTC (permalink / raw)
  To: Mark Laws; +Cc: 19688

> Date: Fri, 20 Feb 2015 01:31:08 +0900
> From: Mark Laws <mdl@60hz.org>
> Cc: 19688@debbugs.gnu.org
> 
> FYI, I just received notice of completion of my assignment.

Thanks, I will install in a day or two.





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-19 16:56                                                       ` Eli Zaretskii
@ 2015-02-21 13:03                                                         ` Eli Zaretskii
  2015-02-21 19:30                                                           ` Mark Laws
  0 siblings, 1 reply; 31+ messages in thread
From: Eli Zaretskii @ 2015-02-21 13:03 UTC (permalink / raw)
  To: mdl; +Cc: 19688

> Date: Thu, 19 Feb 2015 18:56:45 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 19688@debbugs.gnu.org
> 
> > Date: Fri, 20 Feb 2015 01:31:08 +0900
> > From: Mark Laws <mdl@60hz.org>
> > Cc: 19688@debbugs.gnu.org
> > 
> > FYI, I just received notice of completion of my assignment.
> 
> Thanks, I will install in a day or two.

Can you provide some simple test I could run after applying, to make
sure everything works as intended?





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-21 13:03                                                         ` Eli Zaretskii
@ 2015-02-21 19:30                                                           ` Mark Laws
  2015-02-27 14:26                                                             ` Eli Zaretskii
  0 siblings, 1 reply; 31+ messages in thread
From: Mark Laws @ 2015-02-21 19:30 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 19688

On Sat, Feb 21, 2015 at 10:03 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> Can you provide some simple test I could run after applying, to make
> sure everything works as intended?

$ emacsclient -ca ""

Should launch Emacs if it's not already running, and connect to an
existing Emacs process if it is. Try doing it a few times in a row to
make sure both cases work and that the double-free issue doesn't
happen (I have no reason to believe it would, but just in case).

-- 
|v\ /\ |\ |< |_ /\ \^| //





^ permalink raw reply	[flat|nested] 31+ messages in thread

* bug#19688: [patch] add support for emacs daemon on Windows
  2015-02-21 19:30                                                           ` Mark Laws
@ 2015-02-27 14:26                                                             ` Eli Zaretskii
  0 siblings, 0 replies; 31+ messages in thread
From: Eli Zaretskii @ 2015-02-27 14:26 UTC (permalink / raw)
  To: Mark Laws; +Cc: 19688-done

> Date: Sun, 22 Feb 2015 04:30:29 +0900
> From: Mark Laws <mdl@60hz.org>
> Cc: 19688@debbugs.gnu.org
> 
> On Sat, Feb 21, 2015 at 10:03 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> > Can you provide some simple test I could run after applying, to make
> > sure everything works as intended?
> 
> $ emacsclient -ca ""
> 
> Should launch Emacs if it's not already running, and connect to an
> existing Emacs process if it is. Try doing it a few times in a row to
> make sure both cases work and that the double-free issue doesn't
> happen (I have no reason to believe it would, but just in case).

Thanks.

I pushed the changes, with slight variations to make them less
pervasive where possible, as commit 805fe50 on the master branch.  I
then followed that up with the few necessary changes in the
documentation and in NEWS, as should happen with every user-visible
change.

Thanks again for working on this, and sorry for delays in landing this
on master.

I'm closing the bug.





^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2015-02-27 14:26 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-25 19:18 bug#19688: [patch] add support for emacs daemon on Windows Mark Laws
2015-01-25 20:34 ` Eli Zaretskii
     [not found]   ` <CADemMPM+Tix-6FJ+CO3HA8y7Cq6AV0kv_e6_qn7BaSw1QMOwTQ@mail.gmail.com>
2015-01-26  6:00     ` Eli Zaretskii
2015-01-26  7:40       ` Mark Laws
2015-01-26 11:56         ` Daniel Colascione
2015-01-27  8:40           ` Mark Laws
2015-01-30  0:36             ` Mark Laws
2015-01-30  6:28               ` Eli Zaretskii
2015-02-13  0:07                 ` Mark Laws
2015-02-13  8:49                   ` Eli Zaretskii
2015-02-14 12:10                     ` Eli Zaretskii
2015-02-14 13:16                       ` Mark Laws
2015-02-14 13:28                         ` Eli Zaretskii
2015-02-14 13:37                           ` Mark Laws
2015-02-14 15:24                             ` Eli Zaretskii
2015-02-14 16:34                               ` Mark Laws
2015-02-14 16:53                                 ` Eli Zaretskii
2015-02-14 16:57                                   ` Mark Laws
2015-02-14 17:23                                     ` Eli Zaretskii
2015-02-14 17:30                                       ` Mark Laws
2015-02-14 17:42                                         ` Eli Zaretskii
2015-02-14 17:57                                           ` Mark Laws
2015-02-14 18:26                                             ` Eli Zaretskii
2015-02-14 19:21                                               ` Mark Laws
2015-02-14 19:29                                                 ` Eli Zaretskii
2015-02-14 21:15                                                   ` Mark Laws
2015-02-19 16:31                                                     ` Mark Laws
2015-02-19 16:56                                                       ` Eli Zaretskii
2015-02-21 13:03                                                         ` Eli Zaretskii
2015-02-21 19:30                                                           ` Mark Laws
2015-02-27 14:26                                                             ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).