[PATCH] export: only close files which export itself has opened

[Adrian Buehlmann]

On 2011-02-23 22:13, Matt Mackall wrote:
> On Thu, 2011-02-24 at 02:03 +0500, Waqas Hussain wrote:
>> On Thu, Feb 24, 2011 at 1:45 AM, Matt Mackall <mpm at selenic.com> wrote:
>>> On Wed, 2011-02-23 at 13:30 +0500, Waqas Hussain wrote:
>>>> # HG changeset patch
>>>> # User Waqas Hussain <waqas20 at gmail.com>
>>>> # Date 1298449315 -18000
>>>> # Branch stable
>>>> # Node ID 90cc203ee41d5eeeb884e39fbf20c148db9e1b16
>>>> # Parent  22f948c027a97e640ae74021dfdfd8014717705d
>>>> export: only close files which export itself has opened
>>>
>>> What bug does this fix?
>>>
>>
>> cmdutil.export() closes files it hasn't opened. I was passing it an
>> output=writable_object, which it used as fp (see cmdutil.make_file(),
>> which checks for write() on the object). It attempts to close the
>> provided file (make_file() returns it as is).
>>
>> This issue was introduced in revision 14f3795a5ed7, and broke
>> functionality in tortoisehg. This patch checks to make sure the file
>> being closed isn't one it didn't create.
> 
> Ok, queued, thanks.
> 

Oh and there we have yet another instance of the usual file open mode
handling defect pattern (failing to treat r+ and rb+ as modifying the
file).

The fix for make_file would be:

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -230,7 +230,7 @@ def make_filename(repo, pat, node,
 def make_file(repo, pat, node=None,
               total=None, seqno=None, revwidth=None, mode='wb', pathname=None):

-    writable = 'w' in mode or 'a' in mode
+    writable = mode not in ('r', 'rb')

     if not pat or pat == '-':
         fp = writable and sys.stdout or sys.stdin


(I should probably do a full code scan after the code freeze is over...)