From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Michael Albinus Newsgroups: gmane.emacs.bugs Subject: bug#26126: 26.0.50; file-notify-rm-watch removes arbitrary watches Date: Wed, 22 Mar 2017 14:17:10 +0100 Message-ID: <877f3h4fjd.fsf@detlef> References: <87r31x9ulw.fsf@luca> <87shmcney8.fsf@detlef> <87efxw7xvc.fsf@luca> <87mvcjophx.fsf@detlef> <87tw6rssoi.fsf@luca> <87pohfkmvh.fsf@detlef> <87lgs2sobr.fsf@luca> <87y3w2gywc.fsf@detlef> <8737e8excq.fsf@luca> <87bmsuixuj.fsf@detlef> <87efxqmzyp.fsf@luca> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: blaine.gmane.org 1490188693 20404 195.159.176.226 (22 Mar 2017 13:18:13 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 22 Mar 2017 13:18:13 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux) Cc: 26126@debbugs.gnu.org To: Andreas Politz Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Mar 22 14:18:09 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cqg9Q-0004yQ-La for geb-bug-gnu-emacs@m.gmane.org; Wed, 22 Mar 2017 14:18:08 +0100 Original-Received: from localhost ([::1]:51038 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cqg9W-0001mU-Ao for geb-bug-gnu-emacs@m.gmane.org; Wed, 22 Mar 2017 09:18:14 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:39772) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cqg9P-0001mF-Rk for bug-gnu-emacs@gnu.org; Wed, 22 Mar 2017 09:18:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cqg9K-000501-PH for bug-gnu-emacs@gnu.org; Wed, 22 Mar 2017 09:18:07 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:40583) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cqg9K-0004zw-Ll for bug-gnu-emacs@gnu.org; Wed, 22 Mar 2017 09:18:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1cqg9K-0007AG-Gt for bug-gnu-emacs@gnu.org; Wed, 22 Mar 2017 09:18:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Michael Albinus Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 22 Mar 2017 13:18:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 26126 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 26126-submit@debbugs.gnu.org id=B26126.149018864127489 (code B ref 26126); Wed, 22 Mar 2017 13:18:02 +0000 Original-Received: (at 26126) by debbugs.gnu.org; 22 Mar 2017 13:17:21 +0000 Original-Received: from localhost ([127.0.0.1]:38782 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cqg8f-00079I-It for submit@debbugs.gnu.org; Wed, 22 Mar 2017 09:17:21 -0400 Original-Received: from mout.gmx.net ([212.227.17.21]:62750) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cqg8d-000795-Ob for 26126@debbugs.gnu.org; Wed, 22 Mar 2017 09:17:20 -0400 Original-Received: from detlef.gmx.de ([93.209.67.208]) by mail.gmx.com (mrgmx102 [212.227.17.168]) with ESMTPSA (Nemesis) id 0M34eJ-1c0mVT1BF1-00swvB; Wed, 22 Mar 2017 14:17:12 +0100 In-Reply-To: <87efxqmzyp.fsf@luca> (Andreas Politz's message of "Tue, 21 Mar 2017 16:06:22 +0100") X-Provags-ID: V03:K0:Q6uRgj7gPvHaT8qusRxWYiassgsf2Y6uM4jHaoaH4TUT8/OBegn NDL8Q6MSPhEOxBIJX03RLCdrOWGFdmyp+67qpPhF6r8aR+APUO/X0+qUiKt6W1DKBH2FWnH 74rIwHPgPyLxnho5xzXqqiM7YPQXdDwrPz9doGjBEMDop60Yh2l5AeF5L6EAYQIa2ukm/fe ZUURp8dAxHKDKhvAHDUVA== X-UI-Out-Filterresults: notjunk:1;V01:K0:/aOgZmbN7bI=:n/aeQ+qPa+DndzQKkFdCrY 05XKAEkBac1SwXywGiqJQrc0A6uiKLhLp9L7BwZ1o+sIcQ2ylnf8mkaGxYWsLRV8CfkRPmfvg 4yVzcz3GBKtqnFO9WHuHB/TkL0lNZdViicMbNNkoVgp5Smoy92NruQWXQCSZM03T99o3apsVs RCVwgjDth17VvA5CrxinUyuFCafuJNpcY8RHyLsBrs8Vfs2SZ9AI2Ygw2PtlpguSN7NjN+mQO naN+pg70Asa2ocsovCmfYSz/fCq7s/I6wepjYrgfhNoiQUqM2NoZEpSxxWMeKiaFoXZrGyn1a 2ID1CNYUUNOE8rnn5ZiLRPsxt6X476QK724jwGBuyMBttMjh6ZECRT70oIpVHrPZKM8B6JBmE tYXbGbOxBK2wQhoRZ/QOUmc0qHsoe+ewlLtxfO8vIhS0/xXcHogIU/7vzIQgIF2tFCr7Eq68G NQXzDten6DoFy2AP80LLdHqZFb4H/MPYvzwR7ybHSiK9XmkqDNX9Ck1okbpDyQuBnaAMc6QJp S4tuqSgwF8iIA0LU7tXTJbmhcZMrDDyDmrbNjMRo3Z/nG+lUG1+x7VRQcIt6Y4m5sWuTZ0aZV JNgnuIGCp7VMPdU6h17nqFOyiQ5wJlfye0tCGAVQHnFa6zSUb7irXp7UQ+qmbTtv7jiqB2DTG Buz82Hlv0ov8zGV+QMlWHYR3uFdTiK/RVRgZL4Ig4vzeUHSlsE+Cz2PXCEB3CmSYI4KisGE20 QbUe13zjS2UWfmuYg/F+0ylRQnO9I3OKkxGNasYoW8bJFI2wehnY6Py2VoeJhrEMjcLl54G3 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:130803 Archived-At: Andreas Politz writes: Hi Andreas, >> I've applied the patch, and filenotify-tests.el passes all tests >> (except `file-notify-test04-autorevert-remote', but that's another >> story). So I believe it is OK to apply it to master, and see how it goes >> (waiting for feedback). > > Let me work on this a little more. I think, I'm not removing the > descriptors in inotify.c correctly. OK. Take your time. >> I'm not sure we can eliminate the `file' binding. I believe, it is >> needed for the kqueue library. Maybe you add a TODO comment for >> retesting instead. > > Shouldn't be, since kqueue, w32notify and gfilenotify all return a > pointer wrapped in a Lisp-Integer, i.e. for these back-ends the file > value was already nil all the time. We shall recheck, once your changed inotify implementation has hit the repo. >> I'm a little bit undecided, whether we shall add this as extra test >> case, or whether we shall integrate it into >> `file-notify-test03-events'. The former might be better, but it would >> also mean that we shall break down `file-notify-test03-events' into >> smaller tests. > > I think it would be better to split those tests into smaller units. For > once it makes it easier to determine which should-form actually failed. > And secondly, it makes it easier to add a new test (especially for > people not to familiar with the code), without being anxious about > interfering with existing ones. Likely yes. I have the same feeling, but haven't done due to lack of energy and time. For the time being, I have added a modified version of your test removing watch descriptors out of order to `file-notify-test02-rm-watch'. Since this fails for inotify, I've added an :expected-result tag to this test. Can be removed, when fixed in inotify.c. >>> * inotify_add_watch internally uses a single watch per directory, which >>> may be shared by multiple clients (using filenotify.el). The problem >>> here seems to be that these clients may use different FLAGS as >>> argument to file-notify-add-watch. Currently, the last added client's >>> FLAGS become the effective mask for the underlying C-descriptor, >>> meaning that clients added before may not receive change or >>> attribute-change events if the mask was modified accordingly. >> >> I'm aware of this problem (it happens also for other libraries, I >> believe). No idea yet whether it is important to fix it. But maybe you >> add a TODO entry at the end of filenotify.el. > > I think, it is important. For example, auto-revert's file-notify > mechanism (using '(change attribute-change) as flags) would break, if > some other package decides to watch the same file, but for > attribute-changes only. > > It seems to me that this only affects inotify, since all other back-ends > return a newly created descriptor, but I haven't explicitly checked > this. So I would let it for you to implement it in inotify.c. When there is also a respective test, we will see whether it is a problem for other backends, too. > -ap Best regards, Michael.