Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Close all files. #186

Merged
merged 4 commits into from Jun 5, 2014
Merged

Close all files. #186

merged 4 commits into from Jun 5, 2014

Conversation

garyvdm
Copy link
Contributor

@garyvdm garyvdm commented Jun 1, 2014

Python 3.4 add as new feature that emits a warning if a file object gets deleted or garbage collected before it is closed.

Running the test suite under 3.4, I was getting a lot of these warnings. This patch fixes all of these warnings.

In addition, it also fixes a bunch of errors I was getting when running the test suite under wine, similar to:

======================================================================
ERROR: test_simple (dulwich.tests.test_porcelain.PullTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Python27\lib\shutil.py", line 250, in rmtree
    onerror(os.remove, fullname, sys.exc_info())
  File "C:\Python27\lib\shutil.py", line 248, in rmtree
    os.remove(fullname)
WindowsError: [Error 32] Sharing violation: 'c:\\users\\garyvdm\\temp\\tmpabr7v1\\tmpa3bguj'

@garyvdm
Copy link
Contributor Author

garyvdm commented Jun 1, 2014

Got merge conflicts. Rebased version on the way.

@jelmer
Copy link
Owner

jelmer commented Jun 4, 2014

Can you split this up?I agree that this the correct thing to do for proto.

For repositories I am less certain. We should definitely do something, but another approach would be to use a "read locks"-style approach like in bzrlib. That would have the added benefit of not having to stat so much when looking for objects, to see if loose object files are present or if the pack file directory has changed.

@garyvdm
Copy link
Contributor Author

garyvdm commented Jun 4, 2014

On Wed, Jun 4, 2014 at 2:44 AM, Jelmer Vernooij notifications@github.com
wrote:

Can you split this up?I agree that this the correct thing to do for proto.

Sure, will do.

@garyvdm
Copy link
Contributor Author

garyvdm commented Jun 4, 2014

I've removed the repo changes, which I will return to later, and I have split up the changes in to different commits. (Doing this was valuable as I picked up a number of mistakes during this process.)

As most of the lines that have changed in this patch are just a change in indentation, I find using meld with it's synchronization feature very helpful to review the diff. e.g:
http://picpaste.com/Screenshot_from_2014-06-04_14_11_16.png

@jelmer jelmer merged commit 9d90813 into jelmer:master Jun 5, 2014
@garyvdm garyvdm deleted the python3-close-files branch June 5, 2014 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants