From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.bugs Subject: bug#11757: Acknowledgement (24.1.50; vc-git calls `process-file' too many times) Date: Fri, 06 Jul 2012 19:55:54 +0400 Message-ID: <4FF70A8A.5060500@yandex.ru> References: <4FE2832A.1030308@yandex.ru> <4FE994E8.9020605@yandex.ru> <87zk7qs21q.fsf@gmx.de> <4FEBAAAA.3030102@yandex.ru> <87d34igrie.fsf@gmx.de> <4FEDD2A0.3010300@yandex.ru> <878vf66pgw.fsf@gmx.de> <4FEDDFD8.1010407@yandex.ru> <87wr2pgoiy.fsf@gmx.de> <4FEEF763.2060806@yandex.ru> <878vf5544d.fsf@gmx.de> <4FEF3A87.6000904@yandex.ru> <874nps63ki.fsf@gmx.de> <4FEF500A.9060103@yandex.ru> <87pq8f4xbv.fsf@gmx.de> <4FF062D7.7050402@yandex.ru> <878vf2sf7q.fsf@gmx.de> <4FF197A6.7060807@yandex.ru> <871ukrr297.fsf@gmx.de> <4FF47280.7040709@yandex.ru> <87fw95m2b7.fsf@gmx.de> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------050806010408080303050200" X-Trace: dough.gmane.org 1341590176 8267 80.91.229.3 (6 Jul 2012 15:56:16 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Fri, 6 Jul 2012 15:56:16 +0000 (UTC) Cc: 11757@debbugs.gnu.org To: Michael Albinus Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Jul 06 17:56:15 2012 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1SnAtB-0003TN-SA for geb-bug-gnu-emacs@m.gmane.org; Fri, 06 Jul 2012 17:56:14 +0200 Original-Received: from localhost ([::1]:48640 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SnAtA-0007li-QP for geb-bug-gnu-emacs@m.gmane.org; Fri, 06 Jul 2012 11:56:12 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:41408) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SnAt7-0007lJ-1S for bug-gnu-emacs@gnu.org; Fri, 06 Jul 2012 11:56:11 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SnAt3-0003YZ-P4 for bug-gnu-emacs@gnu.org; Fri, 06 Jul 2012 11:56:08 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:43423) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SnAt3-0003YR-EJ for bug-gnu-emacs@gnu.org; Fri, 06 Jul 2012 11:56:05 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1SnAxr-0000AD-Le for bug-gnu-emacs@gnu.org; Fri, 06 Jul 2012 12:01:03 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Dmitry Gutov Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 06 Jul 2012 16:01:03 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 11757 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 11757-submit@debbugs.gnu.org id=B11757.1341590458608 (code B ref 11757); Fri, 06 Jul 2012 16:01:03 +0000 Original-Received: (at 11757) by debbugs.gnu.org; 6 Jul 2012 16:00:58 +0000 Original-Received: from localhost ([127.0.0.1]:52967 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SnAxk-00009k-RX for submit@debbugs.gnu.org; Fri, 06 Jul 2012 12:00:58 -0400 Original-Received: from forward14.mail.yandex.net ([95.108.130.92]:49804) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SnAxg-00009Z-Jc for 11757@debbugs.gnu.org; Fri, 06 Jul 2012 12:00:55 -0400 Original-Received: from smtp12.mail.yandex.net (smtp12.mail.yandex.net [95.108.131.191]) by forward14.mail.yandex.net (Yandex) with ESMTP id 48F4E1982600; Fri, 6 Jul 2012 19:55:52 +0400 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1341590152; bh=r2mKzK/0pT+CkLapoT4rBgQ9kvqyC5OFR1CKuooFFqw=; h=Message-ID:Date:From:MIME-Version:To:CC:Subject:References: In-Reply-To:Content-Type; b=Ek+hX4RnjkeZ0NFprfU5tUNGYqvFXGesT5L7cCuVKYIicT9ufhREzDJyKm9HxSqav BOO066/QebdZnyw9L8LBHy7xw8YSaMl32bWe7oOreWUL3jMSglmSMkCuPNE5yo8/kK Cfbi8156fSfmnugYLd7FS2O+X4NagwEGpvdt2DLY= Original-Received: from smtp12.mail.yandex.net (localhost [127.0.0.1]) by smtp12.mail.yandex.net (Yandex) with ESMTP id 256CC16A05D8; Fri, 6 Jul 2012 19:55:52 +0400 (MSK) Original-Received: from 98-87.nwlink.spb.ru (98-87.nwlink.spb.ru [178.252.98.87]) by smtp12.mail.yandex.net (nwsmtp/Yandex) with ESMTP id tpOuTDOJ-tpOKCA4M; Fri, 6 Jul 2012 19:55:51 +0400 X-Yandex-Rcpt-Suid: michael.albinus@gmx.de X-Yandex-Rcpt-Suid: 11757@debbugs.gnu.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1341590152; bh=r2mKzK/0pT+CkLapoT4rBgQ9kvqyC5OFR1CKuooFFqw=; h=Message-ID:Date:From:User-Agent:MIME-Version:To:CC:Subject: References:In-Reply-To:Content-Type; b=fmHKtz0ptbiF6tWiGDDLXod/4b3KK0lPueqag16sRtybTaS6L9UXjXEOR6bTUAjyh FQtOCBlJHXIhmfWkOrcCoApLjJnsQSyoMmF20GiwL3Dy4f/htHi0rnezJ8wvnptj7A Cn3UoF1h4T/XqIPi++PTlzUgTrY2Hl1XyhzC8+D0= User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120614 Thunderbird/13.0.1 In-Reply-To: <87fw95m2b7.fsf@gmx.de> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 140.186.70.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-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:61648 Archived-At: This is a multi-part message in MIME format. --------------050806010408080303050200 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit On 06.07.2012 17:44, Michael Albinus wrote: > Dmitry Gutov writes: > >>> If we assume that there are no dangerous vc commands outside Emacs, we >>> wouldn't have a problem. >> >> In this case, the behavior of the first patch I posted here should be >> acceptable, right? It's simpler, has pretty much the same effect, and >> should be a tiny bit faster. > > That I don't know. Both patches do almost what we expect, and having a > cached value for `vc-registered' sounds more performant when applied > often enough. With the attached patch, `vc-git-registered' is usually called only once during the lifetime of a buffer. For caching to be an improvement, 'git-registered value has to be invalidated separately from 'vc-backend, yet less often than `vc-git-state' is called. >>> Yes. I don't know, whether we will be able to handle any surprise when >>> using caches. There will always be a scenario which lets fail a given >>> algorithm. I fear. >> >> Sure, but I'm just asking for one scenario that works better with >> explicitly caching 'git-registered, instead of not calling it in >> vc-git-state'. >> If `vc-git-state' doesn't call `vc-git-registered' (just assumes it's >> t), then `vc-registered' is the latter's only client, and so its >> return value is implicitly cached in 'vc-backend property. > > Maybe. Could you show a patch? (Please with ChangeLog entry, I would > commit if it looks good). * vc-git.el (vc-git-state): Don't call `vc-git-registered', assume it's always t. (vc-git-registered): Remove caching, the function is only called once. Thank you, --Dmitry --------------050806010408080303050200 Content-Type: text/plain; charset=windows-1251; name="vc-git-registered.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="vc-git-registered.diff" diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el index 8b48efb..f585f1b 100644 --- a/lisp/vc/vc-git.el +++ b/lisp/vc/vc-git.el @@ -173,31 +173,28 @@ matching the resulting Git log output, and KEYWORDS is a list of (defun vc-git-registered (file) "Check whether FILE is registered with git." - (or (vc-file-getprop file 'git-registered) - (vc-file-setprop - file 'git-registered - (let ((dir (vc-git-root file))) - (when dir - (with-temp-buffer - (let* (process-file-side-effects - ;; Do not use the `file-name-directory' here: git-ls-files - ;; sometimes fails to return the correct status for relative - ;; path specs. - ;; See also: http://marc.info/?l=git&m=125787684318129&w=2 - (name (file-relative-name file dir)) - (str (ignore-errors - (cd dir) - (vc-git--out-ok "ls-files" "-c" "-z" "--" name) - ;; If result is empty, use ls-tree to check for deleted - ;; file. - (when (eq (point-min) (point-max)) - (vc-git--out-ok "ls-tree" "--name-only" "-z" "HEAD" - "--" name)) - (buffer-string)))) - (and str - (> (length str) (length name)) - (string= (substring str 0 (1+ (length name))) - (concat name "\0")))))))))) + (let ((dir (vc-git-root file))) + (when dir + (with-temp-buffer + (let* (process-file-side-effects + ;; Do not use the `file-name-directory' here: git-ls-files + ;; sometimes fails to return the correct status for relative + ;; path specs. + ;; See also: http://marc.info/?l=git&m=125787684318129&w=2 + (name (file-relative-name file dir)) + (str (ignore-errors + (cd dir) + (vc-git--out-ok "ls-files" "-c" "-z" "--" name) + ;; If result is empty, use ls-tree to check for deleted + ;; file. + (when (eq (point-min) (point-max)) + (vc-git--out-ok "ls-tree" "--name-only" "-z" "HEAD" + "--" name)) + (buffer-string)))) + (and str + (> (length str) (length name)) + (string= (substring str 0 (1+ (length name))) + (concat name "\0")))))))) (defun vc-git--state-code (code) "Convert from a string to a added/deleted/modified state." @@ -218,23 +215,24 @@ matching the resulting Git log output, and KEYWORDS is a list of ;; is direct ancestor of corresponding upstream branch, and the file ;; was modified upstream. But we can't check that without a network ;; operation. - (if (not (vc-git-registered file)) - 'unregistered - (let ((diff (vc-git--run-command-string - file "diff-index" "-p" "--raw" "-z" "HEAD" "--"))) - (if (and diff - (string-match ":[0-7]\\{6\\} [0-7]\\{6\\} [0-9a-f]\\{40\\} [0-9a-f]\\{40\\} \\([ADMUT]\\)\0[^\0]+\0\\(.*\n.\\)?" - diff)) - (let ((diff-letter (match-string 1 diff))) - (if (not (match-beginning 2)) - ;; Empty diff: file contents is the same as the HEAD - ;; revision, but timestamps are different (eg, file - ;; was "touch"ed). Update timestamp in index: - (prog1 'up-to-date - (vc-git--call nil "add" "--refresh" "--" - (file-relative-name file))) - (vc-git--state-code diff-letter))) - (if (vc-git--empty-db-p) 'added 'up-to-date))))) + ;; This assumes that status is known to be not `unregistered' because + ;; we've been successfully dispatched here from `vc-state', that + ;; means `vc-git-registered' returned t earlier once. Bug#11757 + (let ((diff (vc-git--run-command-string + file "diff-index" "-p" "--raw" "-z" "HEAD" "--"))) + (if (and diff + (string-match ":[0-7]\\{6\\} [0-7]\\{6\\} [0-9a-f]\\{40\\} [0-9a-f]\\{40\\} \\([ADMUT]\\)\0[^\0]+\0\\(.*\n.\\)?" + diff)) + (let ((diff-letter (match-string 1 diff))) + (if (not (match-beginning 2)) + ;; Empty diff: file contents is the same as the HEAD + ;; revision, but timestamps are different (eg, file + ;; was "touch"ed). Update timestamp in index: + (prog1 'up-to-date + (vc-git--call nil "add" "--refresh" "--" + (file-relative-name file))) + (vc-git--state-code diff-letter))) + (if (vc-git--empty-db-p) 'added 'up-to-date)))) (defun vc-git-working-revision (_file) "Git-specific version of `vc-working-revision'." --------------050806010408080303050200--