Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd84b960

Browse files
committed
cfg_TCs,#519: FIX config resource leaks
+ Modify lock/read-config-file code to ansure files closed+ Use `with GitConfigarser()` more systematically in TCs.+ Clear any locks left hanging from pev Tcs
1 parentb114f3b commitd84b960

File tree

2 files changed

+116
-115
lines changed

2 files changed

+116
-115
lines changed

‎git/config.py

Lines changed: 23 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -388,23 +388,18 @@ def read(self):
388388
whilefiles_to_read:
389389
file_path=files_to_read.pop(0)
390390
fp=file_path
391-
close_fp=False
391+
file_ok=False
392392

393-
# assume a path if it is not a file-object
394-
ifnothasattr(fp,"seek"):
393+
ifhasattr(fp,"seek"):
394+
self._read(fp,fp.name)
395+
else:
396+
# assume a path if it is not a file-object
395397
try:
396-
fp=open(file_path,'rb')
397-
close_fp=True
398+
withopen(file_path,'rb')asfp:
399+
file_ok=True
400+
self._read(fp,fp.name)
398401
exceptIOError:
399402
continue
400-
# END fp handling
401-
402-
try:
403-
self._read(fp,fp.name)
404-
finally:
405-
ifclose_fp:
406-
fp.close()
407-
# END read-handling
408403

409404
# Read includes and append those that we didn't handle yet
410405
# We expect all paths to be normalized and absolute (and will assure that is the case)
@@ -413,7 +408,7 @@ def read(self):
413408
ifinclude_path.startswith('~'):
414409
include_path=os.path.expanduser(include_path)
415410
ifnotos.path.isabs(include_path):
416-
ifnotclose_fp:
411+
ifnotfile_ok:
417412
continue
418413
# end ignore relative paths if we don't know the configuration file path
419414
assertos.path.isabs(file_path),"Need absolute paths to be sure our cycle checks will work"
@@ -477,34 +472,25 @@ def write(self):
477472
# end
478473

479474
fp=self._file_or_files
480-
close_fp=False
481475

482476
# we have a physical file on disk, so get a lock
483-
ifisinstance(fp,string_types+ (FileType, )):
477+
is_file_lock=isinstance(fp,string_types+ (FileType, ))
478+
ifis_file_lock:
484479
self._lock._obtain_lock()
485-
# END get lock for physical files
486-
487-
ifnothasattr(fp,"seek"):
488-
fp=open(self._file_or_files,"wb")
489-
close_fp=True
490-
else:
491-
fp.seek(0)
492-
# make sure we do not overwrite into an existing file
493-
ifhasattr(fp,'truncate'):
494-
fp.truncate()
495-
# END
496-
# END handle stream or file
497-
498-
# WRITE DATA
499480
try:
500-
self._write(fp)
481+
ifnothasattr(fp,"seek"):
482+
withopen(self._file_or_files,"wb")asfp:
483+
self._write(fp)
484+
else:
485+
fp.seek(0)
486+
# make sure we do not overwrite into an existing file
487+
ifhasattr(fp,'truncate'):
488+
fp.truncate()
489+
self._write(fp)
501490
finally:
502-
ifclose_fp:
503-
fp.close()
504-
# END data writing
505-
506-
# we do not release the lock - it will be done automatically once the
507-
# instance vanishes
491+
# we release the lock - it will not vanish automatically in PY3.5+
492+
ifis_file_lock:
493+
self._lock._release_lock()
508494

509495
def_assure_writable(self,method_name):
510496
ifself.read_only:

‎git/test/test_config.py

Lines changed: 93 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -4,80 +4,98 @@
44
# This module is part of GitPython and is released under
55
# the BSD License: http://www.opensource.org/licenses/bsd-license.php
66

7-
fromgit.test.libimport (
8-
TestCase,
9-
fixture_path,
10-
assert_equal,
11-
)
12-
fromgit.test.libimportwith_rw_directory
7+
importglob
8+
importio
9+
importos
10+
1311
fromgitimport (
1412
GitConfigParser
1513
)
1614
fromgit.compatimport (
1715
string_types,
18-
)
19-
importio
20-
importos
16+
is_win,)
2117
fromgit.configimportcp
18+
fromgit.test.libimport (
19+
TestCase,
20+
fixture_path,
21+
)
22+
fromgit.test.libimportwith_rw_directory
23+
24+
importos.pathasosp
25+
26+
27+
_tc_lock_fpaths=osp.join(osp.dirname(__file__),'fixtures/*.lock')
28+
29+
30+
def_rm_lock_files():
31+
forlfpinglob.glob(_tc_lock_fpaths):
32+
ifis_winandosp.isfile(lfp):
33+
os.chmod(lfp,0o777)
34+
os.remove(lfp)
2235

2336

2437
classTestBase(TestCase):
38+
defsetUp(self):
39+
_rm_lock_files()
40+
41+
deftearDown(self):
42+
forlfpinglob.glob(_tc_lock_fpaths):
43+
ifosp.isfile(lfp):
44+
raiseAssertionError('Previous TC left hanging git-lock file: %s',lfp)
2545

2646
def_to_memcache(self,file_path):
27-
fp=open(file_path,"rb")
28-
sio=io.BytesIO(fp.read())
47+
withopen(file_path,"rb")asfp:
48+
sio=io.BytesIO(fp.read())
2949
sio.name=file_path
3050
returnsio
3151

3252
deftest_read_write(self):
3353
# writer must create the exact same file as the one read before
3454
forfilenamein ("git_config","git_config_global"):
3555
file_obj=self._to_memcache(fixture_path(filename))
36-
w_config=GitConfigParser(file_obj,read_only=False)
37-
w_config.read()# enforce reading
38-
assertw_config._sections
39-
w_config.write()# enforce writing
40-
41-
# we stripped lines when reading, so the results differ
42-
assertfile_obj.getvalue()
43-
self.assertEqual(file_obj.getvalue(),self._to_memcache(fixture_path(filename)).getvalue())
44-
45-
# creating an additional config writer must fail due to exclusive access
46-
self.failUnlessRaises(IOError,GitConfigParser,file_obj,read_only=False)
47-
48-
# should still have a lock and be able to make changes
49-
assertw_config._lock._has_lock()
50-
51-
# changes should be written right away
52-
sname="my_section"
53-
oname="mykey"
54-
val="myvalue"
55-
w_config.add_section(sname)
56-
assertw_config.has_section(sname)
57-
w_config.set(sname,oname,val)
58-
assertw_config.has_option(sname,oname)
59-
assertw_config.get(sname,oname)==val
60-
61-
sname_new="new_section"
62-
oname_new="new_key"
63-
ival=10
64-
w_config.set_value(sname_new,oname_new,ival)
65-
assertw_config.get_value(sname_new,oname_new)==ival
66-
67-
file_obj.seek(0)
68-
r_config=GitConfigParser(file_obj,read_only=True)
69-
assertr_config.has_section(sname)
70-
assertr_config.has_option(sname,oname)
71-
assertr_config.get(sname,oname)==val
72-
w_config.release()
56+
withGitConfigParser(file_obj,read_only=False)asw_config:
57+
w_config.read()# enforce reading
58+
assertw_config._sections
59+
w_config.write()# enforce writing
60+
61+
# we stripped lines when reading, so the results differ
62+
assertfile_obj.getvalue()
63+
self.assertEqual(file_obj.getvalue(),self._to_memcache(fixture_path(filename)).getvalue())
64+
65+
# creating an additional config writer must fail due to exclusive access
66+
self.failUnlessRaises(IOError,GitConfigParser,file_obj,read_only=False)
67+
68+
# should still have a lock and be able to make changes
69+
assertw_config._lock._has_lock()
70+
71+
# changes should be written right away
72+
sname="my_section"
73+
oname="mykey"
74+
val="myvalue"
75+
w_config.add_section(sname)
76+
assertw_config.has_section(sname)
77+
w_config.set(sname,oname,val)
78+
assertw_config.has_option(sname,oname)
79+
assertw_config.get(sname,oname)==val
80+
81+
sname_new="new_section"
82+
oname_new="new_key"
83+
ival=10
84+
w_config.set_value(sname_new,oname_new,ival)
85+
assertw_config.get_value(sname_new,oname_new)==ival
86+
87+
file_obj.seek(0)
88+
r_config=GitConfigParser(file_obj,read_only=True)
89+
assertr_config.has_section(sname)
90+
assertr_config.has_option(sname,oname)
91+
assertr_config.get(sname,oname)==val
7392
# END for each filename
7493

7594
@with_rw_directory
7695
deftest_lock_reentry(self,rw_dir):
7796
fpl=os.path.join(rw_dir,'l')
78-
gcp=GitConfigParser(fpl,read_only=False)
79-
withgcpascw:
80-
cw.set_value('include','some_value','a')
97+
withGitConfigParser(fpl,read_only=False)asgcp:
98+
gcp.set_value('include','some_value','a')
8199
# entering again locks the file again...
82100
withgcpascw:
83101
cw.set_value('include','some_other_value','b')
@@ -91,21 +109,21 @@ def test_lock_reentry(self, rw_dir):
91109

92110
deftest_multi_line_config(self):
93111
file_obj=self._to_memcache(fixture_path("git_config_with_comments"))
94-
config=GitConfigParser(file_obj,read_only=False)
95-
ev="ruby -e '\n"
96-
ev+="system %(git), %(merge-file), %(--marker-size=%L), %(%A), %(%O), %(%B)\n"
97-
ev+="b = File.read(%(%A))\n"
98-
ev+="b.sub!(/^<+ .*\\nActiveRecord::Schema\\.define.:version => (\\d+). do\\n=+\\nActiveRecord::Schema\\."
99-
ev+="define.:version => (\\d+). do\\n>+ .*/) do\n"
100-
ev+=" %(ActiveRecord::Schema.define(:version => #{[$1, $2].max}) do)\n"
101-
ev+="end\n"
102-
ev+="File.open(%(%A), %(w)) {|f| f.write(b)}\n"
103-
ev+="exit 1 if b.include?(%(<)*%L)'"
104-
assert_equal(config.get('merge "railsschema"','driver'),ev)
105-
assert_equal(config.get('alias','lg'),
106-
"log --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr)%Creset'"
107-
" --abbrev-commit --date=relative")
108-
assertlen(config.sections())==23
112+
withGitConfigParser(file_obj,read_only=False)asconfig:
113+
ev="ruby -e '\n"
114+
ev+="system %(git), %(merge-file), %(--marker-size=%L), %(%A), %(%O), %(%B)\n"
115+
ev+="b = File.read(%(%A))\n"
116+
ev+="b.sub!(/^<+ .*\\nActiveRecord::Schema\\.define.:version => (\\d+). do\\n=+\\nActiveRecord::Schema\\."# noqa E501
117+
ev+="define.:version => (\\d+). do\\n>+ .*/) do\n"
118+
ev+=" %(ActiveRecord::Schema.define(:version => #{[$1, $2].max}) do)\n"
119+
ev+="end\n"
120+
ev+="File.open(%(%A), %(w)) {|f| f.write(b)}\n"
121+
ev+="exit 1 if b.include?(%(<)*%L)'"
122+
self.assertEqual(config.get('merge "railsschema"','driver'),ev)
123+
self.assertEqual(config.get('alias','lg'),
124+
"log --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr)%Creset'"
125+
" --abbrev-commit --date=relative")
126+
self.assertEqual(len(config.sections()),23)
109127

110128
deftest_base(self):
111129
path_repo=fixture_path("git_config")
@@ -202,22 +220,19 @@ def check_test_value(cr, value):
202220

203221
deftest_rename(self):
204222
file_obj=self._to_memcache(fixture_path('git_config'))
205-
cw=GitConfigParser(file_obj,read_only=False,merge_includes=False)
206-
207-
self.failUnlessRaises(ValueError,cw.rename_section,"doesntexist","foo")
208-
self.failUnlessRaises(ValueError,cw.rename_section,"core","include")
223+
withGitConfigParser(file_obj,read_only=False,merge_includes=False)ascw:
224+
self.failUnlessRaises(ValueError,cw.rename_section,"doesntexist","foo")
225+
self.failUnlessRaises(ValueError,cw.rename_section,"core","include")
209226

210-
nn="bee"
211-
assertcw.rename_section('core',nn)iscw
212-
assertnotcw.has_section('core')
213-
assertlen(cw.items(nn))==4
214-
cw.release()
227+
nn="bee"
228+
assertcw.rename_section('core',nn)iscw
229+
assertnotcw.has_section('core')
230+
assertlen(cw.items(nn))==4
215231

216232
deftest_complex_aliases(self):
217233
file_obj=self._to_memcache(fixture_path('.gitconfig'))
218-
w_config=GitConfigParser(file_obj,read_only=False)
219-
self.assertEqual(w_config.get('alias','rbi'),'"!g() { git rebase -i origin/${1:-master} ; } ; g"')
220-
w_config.release()
234+
withGitConfigParser(file_obj,read_only=False)asw_config:
235+
self.assertEqual(w_config.get('alias','rbi'),'"!g() { git rebase -i origin/${1:-master} ; } ; g"')
221236
self.assertEqual(file_obj.getvalue(),self._to_memcache(fixture_path('.gitconfig')).getvalue())
222237

223238
deftest_empty_config_value(self):

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp