From: Nix <nix@esperi.org.uk>
To: Stefan Monnier <monnier@IRO.UMontreal.CA>
Cc: emacs-devel@gnu.org
Subject: Re: [PATCH] A myriad of interrelated daemon-mode startup bugs
Date: Tue, 13 Nov 2012 18:41:59 +0000 [thread overview]
Message-ID: <87zk2lny4o.fsf@spindle.srvr.nix> (raw)
In-Reply-To: <jwvsj8dz83j.fsf-monnier+emacs@gnu.org> (Stefan Monnier's message of "Tue, 13 Nov 2012 13:34:52 -0500")
On 13 Nov 2012, Stefan Monnier spake thusly:
>> [Stefan Cc:ed because part of this is fixing lexical-binding fallout,
>> and as a way of saying thank you for fixing a bug of mine: here's a
>> fix for a bug of yours! :) ]
>
> Actually, it's got nothing to do with lexical-binding, but is due to
> a bug of mine in gv-define-simpler-setter which was fixed yesterday.
Oh. you mean I wasted all that time? ;}
>> --- lisp/desktop.el.orig 2012-11-13 16:36:38.833375966 +0000
>> +++ lisp/desktop.el 2012-11-13 16:38:18.634889091 +0000
>> @@ -627,6 +627,7 @@
>> (and
>> (or (memq desktop-save '(ask ask-if-new))
>> (and exists (eq desktop-save 'ask-if-exists)))
>> + (and (not noninteractive))
>> (y-or-n-p "Save desktop? ")))))
>
> No need for `and' here, but otherwise, that looks OK. Tho maybe a better
> fix is to make it so y-or-n-p just returns nil when there's no live
> terminal.
Good idea. (Whatever part of 'noninteractive' 'no live terminal'
equates to.)
>> - (cl-letf (((default-file-modes) ?\700)) (make-directory dir t))
>> + (make-directory dir t ?\700)
>
> The cl-letf ends up doing the exact same thing, via dynamic scoping
> (note how it's not a simple `let' and it doesn't bind a variable but
> a "place").
Ah! Mind you, there's not much hint in the docs for cl-letf that it's
dynamically bound...
> I'm not completely opposed to adding a "modes" argument to
> make-directory, but I'm not sure it's worth the trouble.
It's a potential security hole to make directories with the wrong mode,
so it seems like a good idea. (Sure, you can bind `default-file-modes',
but could you really call the code above particularly clear?)
>> - (server-start)
>> + (condition-case err
>> + (server-start)
>> + (error (message (error-message-string err))))
>> (if server-process
>> (daemon-initialized)
>> (if (stringp dn)
>
> I'm not sure I understand enough of what could/should happen. E.g. how
> does the above compare to using:
>
> (unwind-protect
> (server-start)
> (if server-process
> (daemon-initialized)
> (if (stringp dn)
> ...
That works, but it throws away the error message. If `server-start'
raised an error, you want to show it to the user before you die.
>> + ;; We must pretend to be noninteractive at this point to tell
>> + ;; kill-emacs-hook functions that read input if interactive
>> + ;; not to do so, since reads from stdin are shared with the
>> + ;; parent.
>> + (let ((noninteractive t))
>> + (kill-emacs 1)))))
>
> Actually, `noninteractive' can sometimes interact with the user via
> stdin/stdout.
OK, so that ugly kludge is presumably unnecessary? Phew.
>> - /* End of file in -batch run causes exit here. */
>> - if (noninteractive)
>> + /* End of file in -batch run or before the daemon is
>> + initialized causes exit here. */
>> + if ((noninteractive) ||
>> + (IS_DAEMON && !daemon_initialized))
>> Fkill_emacs (Qt);
>
> How 'bout having a Lisp-exported variable that controls whether to call
> kill-emacs here? Then server.el could set this var (after successfully
> starting the daemon) to prevent exit.
Instead of daemon-initialized? The worry there is that server-start can
be called from places *other* than daemonization, and it seems like a
layering violation to have it saying 'yeah, you can exit now' if it was
called from somewhere else. The relevant thing here isn't really 'a
server is started': it's 'we have disconnected from our parent and
gone daemon, so having a command loop is acceptable because it won't
mislead users into thinking Emacs has hung'.
> Also, maybe the right way to solve this is to think harder about what
> noninteractive means and how to split it into several variables.
> E.g. one meaning is "no input events", another is "I/O is via
> stdin/stdout", and another we need is "there's no I/O available right
> now".
That's exactly what I was thinking while I got repeatedly confused over
just what 'noninteractive' meant right now. At the moment, it seems to
be used to mean whichever of the above is most convenient at the time :)
--
NULL && (void)
next prev parent reply other threads:[~2012-11-13 18:41 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-13 17:26 [PATCH] A myriad of interrelated daemon-mode startup bugs Nix
2012-11-13 18:34 ` Stefan Monnier
2012-11-13 18:41 ` Nix [this message]
2012-11-13 19:41 ` Stefan Monnier
2012-12-11 16:50 ` Nix
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zk2lny4o.fsf@spindle.srvr.nix \
--to=nix@esperi.org.uk \
--cc=emacs-devel@gnu.org \
--cc=monnier@IRO.UMontreal.CA \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).