Couple of style points, one of which causes a bug.
Generally you want the body of try/except loops to be as minimal, hence my use of else in the example before. I have mixed feelings about nested functions that aren't actually being used as closures. Factoring common logic branches into one route is generally a good thing, even if it means using break-as-goto.
Rather than things along the lines of:
permissions = 0666
fd = os.open(filename, flags, permissions)
it's better to use:
fd = os.open(filename, flags, mode=0666)
which is still self-documenting, and avoids the variable.
As you do that with flags too, you're defeating the race protection of the loop by making all attempts after the first exclusive, potentially causing an infinite loop instead:
flags = os.O_WRONLY | os.O_APPEND | osutils.O_TEXT
while True:
...
fd = os.open(filename, flags)
...
flags = flags | os.O_CREAT | os.O_EXCL permissions = 0666
fd = os.open(filename, flags, permissions)
Also bt.test_trace.test__open_bzr_log_uses_stderr_for_failures was failing, which can be fixed by moving a later catch one step up the exception hierarchy.
Pull <lp:~gz/bzr/no-chown-if-bzrlog-exists> for my fixes for these. I've poked a couple of other minor bits while I was there, but didn't do anything major. (I'd really like windows to avoid this codepath entirely, but this doesn't need to be done right now.)
Couple of style points, one of which causes a bug.
Generally you want the body of try/except loops to be as minimal, hence my use of else in the example before. I have mixed feelings about nested functions that aren't actually being used as closures. Factoring common logic branches into one route is generally a good thing, even if it means using break-as-goto.
Rather than things along the lines of:
permissions = 0666
fd = os.open(filename, flags, permissions)
it's better to use:
fd = os.open(filename, flags, mode=0666)
which is still self-documenting, and avoids the variable.
As you do that with flags too, you're defeating the race protection of the loop by making all attempts after the first exclusive, potentially causing an infinite loop instead:
permission s = 0666
flags = os.O_WRONLY | os.O_APPEND | osutils.O_TEXT
while True:
...
fd = os.open(filename, flags)
...
flags = flags | os.O_CREAT | os.O_EXCL
fd = os.open(filename, flags, permissions)
Also bt.test_ trace.test_ _open_bzr_ log_uses_ stderr_ for_failures was failing, which can be fixed by moving a later catch one step up the exception hierarchy.
Pull <lp:~gz/bzr/no-chown-if-bzrlog-exists> for my fixes for these. I've poked a couple of other minor bits while I was there, but didn't do anything major. (I'd really like windows to avoid this codepath entirely, but this doesn't need to be done right now.)