From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Added basic file system watching support. Date: Mon, 01 Oct 2012 00:38:09 -0400 Message-ID: References: <6218185.ViukoKRdFp@descartes> <1604303.YKOVcvQH0F@descartes> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1349066298 29985 80.91.229.3 (1 Oct 2012 04:38:18 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 1 Oct 2012 04:38:18 +0000 (UTC) Cc: Leo , emacs-devel@gnu.org To: =?iso-8859-1?Q?R=FCdiger?= Sonderfeld Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Oct 01 06:38:24 2012 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1TIXlu-00044D-89 for ged-emacs-devel@m.gmane.org; Mon, 01 Oct 2012 06:38:22 +0200 Original-Received: from localhost ([::1]:37638 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TIXlo-0003U9-SS for ged-emacs-devel@m.gmane.org; Mon, 01 Oct 2012 00:38:16 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:57857) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TIXlm-0003Tt-6g for emacs-devel@gnu.org; Mon, 01 Oct 2012 00:38:15 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TIXlk-0004eP-Mx for emacs-devel@gnu.org; Mon, 01 Oct 2012 00:38:14 -0400 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.182]:13060) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TIXlk-0004eG-Ia for emacs-devel@gnu.org; Mon, 01 Oct 2012 00:38:12 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Av0EAG6Zu09FxLT4/2dsb2JhbAA7CbQRgQiCFQEBBAEnLyMFCws0EhQYDSQuh24FugmLGIUsA6MzgViDBQ X-IronPort-AV: E=Sophos;i="4.75,637,1330923600"; d="scan'208";a="200343494" Original-Received: from 69-196-180-248.dsl.teksavvy.com (HELO pastel.home) ([69.196.180.248]) by ironport2-out.teksavvy.com with ESMTP/TLS/ADH-AES256-SHA; 01 Oct 2012 00:38:10 -0400 Original-Received: by pastel.home (Postfix, from userid 20848) id 0AA64594CA; Mon, 1 Oct 2012 00:38:09 -0400 (EDT) In-Reply-To: <1604303.YKOVcvQH0F@descartes> (=?iso-8859-1?Q?=22R=FCdiger?= Sonderfeld"'s message of "Fri, 28 Sep 2012 18:27:04 +0200") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 206.248.154.182 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:153799 Archived-At: >> I'm not very familiar with inotify and other file-system watching >> facilities, so just as a guideline for others's reviews: I'm OK with >> installing in 24.3 a non-complete version of the code, and I'm OK to >> install before the freeze a code that needs more testing, *but* only if >> those missing functionalities and testing won't require a redesign of >> the API. > It could require a minor redesign of the API. I know of "minor changes" (where backward compatibility is easy to preserve) and "redesigns" but I don't know what a "minor redesign" would look like. > At least I want to gather some experience using the API first. > And maybe porting it to other systems will require API adjustments. > Both BSD's and Windows' filesystem watch APIs are not as powerful > as inotify. [ I'm probably rehashing something already discussed at the time. ] If the API is sufficiently general that it can be reused for other systems, that's great. If there's a good chance this won't work without breaking compatibility, maybe a better option is to provide a low-level API that maps very closely to inotify and then an Elisp layer on top which abstracts away differences between different systems. In that case we can install the inotify support right away while we're still experimenting with the higher-level abstraction. >> I understand that NAME refers to the added/removed file, but I'm >> wondering why we don't also add FILENAME (the name of the directory) to >> the event. If inotify doesn't provide it, we can provide it >> ourselves, right? >> Or is it rarely/never needed? >> Or is it because that directory may change name without us knowing? > The name might change. OK, makes sense. >> I can't remember enough of the context, but objects that are "unique" >> are plentiful. A cons cell would do, regardless of its value, for >> example (now, as to whether it's better than a number, that depends on >> what it's use for, e.g. whether you can put the cons's fields to good >> use). > Currently the only risk is that cookie is bigger than > MOST_POSITIVE_FIXNUM and then gets mapped to a value that is already > taken by another cookie. This is only a problem on 32bit systems > without the use of wide-int because cookie is uint32_t. Ah, I remember what this is now. Rather than `cookie', I'd rather name it `identifier' or `identity'. So IIUC Emacs's code doesn't never needs to pass this cookie back to the inotify code, right? So the only issue with the current "make_number (ev->cookie)" is that it might incorrectly lead to matching two events that are really unrelated, in case they have the same lower 30bits. I don't know how important those last 2bits are in practice. But if they're unlikely to be important in practice, then I guess the current solution might be acceptable. OTOH we should stipulate that COOKIES should be compared with `equal', not `eq', so we could later use something like Fcons (make_number (ev->cookie / 65536), make_number (ev->cookie % 65536)). > There is a similar problem with the watch descriptor. This is slightly different in that here, the descriptor is later passed back to inotify, so it's indispensable that it be preserved correctly. Also, descriptors aren't usually compared for equality: it might be OK to have two watch-descriptors that refer to the same watch object but which aren't `eq'. The docstring of file-watch doesn't really make it clear what file-watch returns (it only says it indirectly with "The ID argument is the id returned by file-watch") so that should be fixed. Also this ID should be called WD for "watch descriptor", WH for "watch handle", or FW for "file-watcher". > Inotify uses an incremental system for watch descriptors and should > be run out of descriptors by the time MOST_POSITIVE_FIXNUM is > reached. But there is no guarantee for it. Therefore something like > SAVE_VALUE should probably be used. If watch descriptors are documented to be "small integers", like file descriptors, it might be OK to use fixnums, but it's a bit ugly. I think the cleaner option is to define a new object type for it. It could be either a new Lisp_Misc type, so you can make them print as something like "#" (take a look at "enum Lisp_Misc_Type" and "union Lisp_Misc" in src/lisp.h for starters; adding a new type will require adding corresponding branches to the switch statements in alloc.c and in print.c). Stefan