Code review comment for lp:~jameinel/bzr/2.1.0rc1-set-mtime

> That may be why you were confused earlier about why we have the if
> clause.

Yes, that's exactly it, I misread the function earlier. My problem with that file is every other function does laziness a different way... See osutils.file_kind_from_stat_mode vs. osutils.get_user_encoding vs. osutils._walkdirs_utf8 (your style).

Found (hopefully) one last problem when doing some more testing. The new test in bt.test_transform fails without the compiled extension, despite the fact both the new TestFSetMtime tests pass. Fiddled around with the tests some more trying to work out where the problem was, pull from my branch again to see if you get the (marked expected) failure as well. The tests are back more like they were before, use `bzr diff -r4942.. bzrlib/tests` for a clean view of the changes.

I'm not sure how to fix it. Only idea so far is to change fset_mtime to set_mtime_and_close, would work by calling os.utime after f.close in the non-compiled case, but is a pretty horrid api.