From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#18141: 24.4.50; saving .gz file breaks file coding Date: Wed, 06 Aug 2014 17:36:00 +0300 Message-ID: <83iom5ptwv.fsf@gnu.org> References: <9pzjfrwvsl.fsf@fencepost.gnu.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1407335788 24015 80.91.229.3 (6 Aug 2014 14:36:28 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Wed, 6 Aug 2014 14:36:28 +0000 (UTC) Cc: 18141@debbugs.gnu.org, Vincent Lefevre , yamaoka@jpl.org To: Glenn Morris Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Aug 06 16:36:19 2014 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 1XF2KB-0003z2-By for geb-bug-gnu-emacs@m.gmane.org; Wed, 06 Aug 2014 16:36:19 +0200 Original-Received: from localhost ([::1]:39412 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XF2K9-000718-2p for geb-bug-gnu-emacs@m.gmane.org; Wed, 06 Aug 2014 10:36:17 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:33421) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XF2K0-000707-P0 for bug-gnu-emacs@gnu.org; Wed, 06 Aug 2014 10:36:14 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XF2Jv-0002t0-CC for bug-gnu-emacs@gnu.org; Wed, 06 Aug 2014 10:36:08 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:53485) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XF2Jv-0002sw-8B for bug-gnu-emacs@gnu.org; Wed, 06 Aug 2014 10:36:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1XF2Ju-0000nu-Hl for bug-gnu-emacs@gnu.org; Wed, 06 Aug 2014 10:36:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 06 Aug 2014 14:36:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 18141 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 18141-submit@debbugs.gnu.org id=B18141.14073357603081 (code B ref 18141); Wed, 06 Aug 2014 14:36:02 +0000 Original-Received: (at 18141) by debbugs.gnu.org; 6 Aug 2014 14:36:00 +0000 Original-Received: from localhost ([127.0.0.1]:60428 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XF2Jr-0000nY-2H for submit@debbugs.gnu.org; Wed, 06 Aug 2014 10:35:59 -0400 Original-Received: from mtaout25.012.net.il ([80.179.55.181]:45366) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1XF2Jm-0000nH-EY for 18141@debbugs.gnu.org; Wed, 06 Aug 2014 10:35:56 -0400 Original-Received: from conversion-daemon.mtaout25.012.net.il by mtaout25.012.net.il (HyperSendmail v2007.08) id <0N9W003002QTN900@mtaout25.012.net.il> for 18141@debbugs.gnu.org; Wed, 06 Aug 2014 17:30:56 +0300 (IDT) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by mtaout25.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0N9W00PU22ZK3640@mtaout25.012.net.il>; Wed, 06 Aug 2014 17:30:56 +0300 (IDT) In-reply-to: X-012-Sender: halo1@inter.net.il X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x 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:92175 Archived-At: > From: Glenn Morris > Date: Tue, 05 Aug 2014 04:34:35 -0400 > Cc: Katsumi Yamaoka > > Perhaps someone could bisect this to find the cause? I looked into this bug. It is a side effect of the following commit: revno: 111638 fixes bug: http://debbugs.gnu.org/13522 committer: Glenn Morris branch nick: trunk timestamp: Wed 2013-01-30 22:35:45 -0800 message: Reduce delay between backing up a file and saving new version * lisp/files.el (basic-save-buffer-2): Choose coding system for writing the file before backing it up. * src/fileio.c (choose_write_coding_system): Make it callable from Lisp. (Fwrite_region): If coding-system-for-write is set, don't call choose_write_coding_system. Move the last piece of choose_write_coding_system here. (syms_of_fileio): Add choose-write-coding-system. That commit determines the encoding using the .gz file name, which of course yields no-conversion. Thus jka-compr is forced to use no-conversion when it writes a temporary file to be submitted to gzip. I can fix that with the simple changes at the end of this message. However, I'd like first to argue that the changes in the above commit are ill-advised and should be reverted. Here's why: . Bug#13522, which these changes were trying to solve, happens when Emacs is killed by an external signal during the time between backing up the original file and writing the new one. (This time can be quite large if write-region asks the user to choose a suitable encoding for the new content.) That is a pretty rare situation, and IMO it's perfectly OK for Emacs to leave just the backup file if it was brutally killed during that time window. . The problem reported in bug#13522 exists because backing up the old contents and writing the new one is not a transaction. The changes in r111638 didn't change that basic fact, they just made the window between the backup and the saving smaller, when Emacs needs to ask the user about the encoding. But the window, albeit a smaller one, is still there, and so it is still possible to cause the same problem by killing Emacs at a suitably chosen opportune moment. . The changes effectively moved the selection of the encoding to the very beginning of write-region. By contrast, the place where write-region calls choose-write-coding-system was carefully chosen through a long trial-and-error process. As shown by this bug report, doing so too early is clearly wrong for "magic" and remote files, but it is also wrong for files whose saving needs to use annotations, as this comment in write-region says: /* Decide the coding-system to encode the data with. We used to make this choice before calling build_annotations, but that leads to problems when a write-annotate-function takes care of unsavable chars (as was the case with X-Symbol). */ So I guess X-Symbol is likely also broken now, as are potentially any packages that use write-annotate-functions. It took us 1.5 years to find this bug; who knows how much time will pass until we find and fix those with write-annotate-functions, a feature that is nowadays used very little? If we do want to make sure the users of write-annotate-functions are not broken, we should probably also refrain from calling choose-write-coding-system from basic-save-buffer-2 when write-annotate-functions are non-nil. But that means part of write-region will now be dragged into basic-save-buffer-2. Where does that end? So I think simply reverting the r111638 changes and leaving bug#13522 as wontfix is an easier way out. An even better fix would be to make the backing up and saving a transaction-like process. That would solve the problem entirely. But I'm not sure this is feasible/practical, with parts of the puzzle implemented in C and parts in Lisp. If it is possible, it will probably be anything but simple, and so likely inappropriate for the emacs-24 branch. Here's the patch I promised that solves this particular bug without reverting r111638: --- lisp/files.el~0 2014-06-29 06:54:20 +0300 +++ lisp/files.el 2014-08-06 17:26:42 +0300 @@ -4835,13 +4835,17 @@ (nth 1 setmodes))) (set-file-modes buffer-file-name (logior (car setmodes) 128)))))) - (let (success) + (let ((filename-is-magic (find-file-name-handler buffer-file-name + 'write-region)) + success) (unwind-protect ;; Pass in nil&nil rather than point-min&max to indicate ;; we're saving the buffer rather than just a region. ;; write-region-annotate-functions may make us of it. - (let ((coding-system-for-write writecoding) - (coding-system-require-warning nil)) + (let ((coding-system-for-write + (if filename-is-magic coding-system-for-write writecoding)) + (coding-system-require-warning + (if filename-is-magic coding-system-require-warning))) (write-region nil nil buffer-file-name nil t buffer-file-truename) (setq success t))