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

Work around non-deterministic failure of uncompress on Windows #21599

Closed
embray opened this issue Sep 26, 2016 · 12 comments
Closed

Work around non-deterministic failure of uncompress on Windows #21599

embray opened this issue Sep 26, 2016 · 12 comments

Comments

@embray
Copy link
Contributor

embray commented Sep 26, 2016

I've noticed sometimes while building on Windows / Cygwin (though this isn't cygwin-specific) some package builds can fail randomly, usually during the uncompress step. Re-rerunning the build a second time always succeeds.

This happened one time recently and I realized there was a Python traceback leading back to the os.rename call wrapped by the attached patch.

This can happen because if any background process happens to have any file in the tree open, even briefly, the renaming the entire directory can fail. So this is especially likely to happen when unpacking source tarballs containing a large number of files.

There may be other points where this can happen, and I'll apply the same workaround if/when I encounter them.

Component: porting: Cygwin

Author: Erik Bray

Branch/Commit: 69830cd

Reviewer: Jeroen Demeyer

Issue created by migration from https://trac.sagemath.org/ticket/21599

@embray embray added this to the sage-7.4 milestone Sep 26, 2016
@jdemeyer
Copy link

comment:2
  1. If the final try failed, the exception should be raised!

And then some bikeshedding:

  1. The usual way to handle such things is an exponential sleep. So I would suggest a delay *= 2 in the while loop.

  2. I would also prefer to see tries instead of retries. I find it easier to understand "3 tries" instead of "1 try and 2 retries". And you know that, because you found it necessary to explain in the docstring.

And of course this is an ugly hack, but that's not your fault.

@embray
Copy link
Contributor Author

embray commented Sep 26, 2016

comment:3

D'oh! Agreed with all of the above.

@embray
Copy link
Contributor Author

embray commented Sep 26, 2016

comment:4

And yes it's an ugly hack but not an uncommon one. I feel like I've done this a dozen other times for Windows support in various places.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 26, 2016

Changed commit from 21d46ab to 822f0d1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 26, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

822f0d1Re-raise the exception if it is raised on the final try.

@jdemeyer
Copy link

comment:7

Two minor things again:

  1. Replace func() by return func(). This way, you support functions which return something. It also means that you can remove the else: break.

  2. The condition while tries > 0: is now always true. So for simplicity, I would prefer to see while True there. That makes it clear that the loop can never be exited normally. By the way, this is a good example of code which would be cleaner with a goto statement.

@embray
Copy link
Contributor Author

embray commented Sep 27, 2016

comment:8

Agreed again on all of the above.

The only people who think gotos are evil have never written non-trivial C code and haven't read/don't understand the context of Dijkstra's essay :)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 28, 2016

Changed commit from 822f0d1 to 69830cd

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 28, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

69830cdAdditional minor cleanup:

@embray
Copy link
Contributor Author

embray commented Sep 28, 2016

New commits:

69830cdAdditional minor cleanup:

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@vbraun
Copy link
Member

vbraun commented Oct 29, 2016

Changed branch from u/embray/bug/uncompress-windows to 69830cd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants