Re: [PATCHv1 4/6] git-p4: create helper function importRevisions()

[Junio C Hamano <gitster@pobox.com>]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Wed, Jan 29, 2020 at 6:13 AM Luke Diamand <luke@diamand.org> wrote:
>> This makes it easier to try/catch around this block of code to ensure
>> cleanup following p4 failures is handled properly.
>>
>> Signed-off-by: Luke Diamand <luke@diamand.org>
>> ---
>> diff --git a/git-p4.py b/git-p4.py
>> @@ -3555,6 +3555,73 @@ def importHeadRevision(self, revision):
>> +    def importRevisions(self, args, branch_arg_given):
>> +        if len(self.changesFile) > 0:
>> +            output = open(self.changesFile).readlines()
>
> Not a new problem (since this code is merely being relocated), but is
> this leaking the open file? Should there be an accompanying close()
> somewhere?
>
>     f = open(self.changesFile)
>     output = f.readlines()
>     close(f)
>
> or something.

Hmph, I was naively hoping that the (never assigned to any variable)
last reference going away at the end of the statement would make the
file object dead, and we can let eventual GC to close it.

Nevertheless it would not hurt to explicitly control the lifetime.