Merge lp:~gz/bzr/kindness_to_FAT_and_other_utime_stories_epilogue into lp:bzr

Proposed by Martin Packman
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~gz/bzr/kindness_to_FAT_and_other_utime_stories_epilogue
Merge into: lp:bzr
Prerequisite: lp:~gz/bzr/kindness_to_FAT_and_other_utime_stories
Diff against target: 17 lines (+3/-4)
1 file modified
bzrlib/diff.py (+3/-4)
To merge this branch: bzr merge lp:~gz/bzr/kindness_to_FAT_and_other_utime_stories_epilogue
Reviewer Review Type Date Requested Status
John A Meinel Approve
Review via email: mp+23545@code.launchpad.net

Commit message

When writing temporary files for external diff do not set the file utime if we don't know when the in-tree file was last modified.

Description of the change

As per the discussion in the prerequisite branch, this change stops the diff code trying to set the date of files with undetermined timestamps to start of the unix epoch.

As I didn't actually get a failure from this, I presume there's no test coverage for this path, it may not even be reachable. However, there seems to be some agreement that not setting the mtime if there's no timestamp is the right option.

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin [gz] wrote:
> Martin [gz] has proposed merging lp:~gz/bzr/kindness_to_FAT_and_other_utime_stories_epilogue into lp:bzr with lp:~gz/bzr/kindness_to_FAT_and_other_utime_stories as a prerequisite.
>
> Requested reviews:
> bzr-core (bzr-core)
>
>
> As per the discussion in the prerequisite branch, this change stops the diff code trying to set the date of files with undetermined timestamps to start of the unix epoch.
>
> As I didn't actually get a failure from this, I presume there's no test coverage for this path, it may not even be reachable. However, there seems to be some agreement that not setting the mtime if there's no timestamp is the right option.
>

I think is is reasonable.

John
=:->

  review: approve

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkvIlR8ACgkQJdeBCYSNAAOmhgCeJMRCFMQuyE2pLhmwSDRI0Yj0
4GsAoKLAL+eu1mewn9MwRVVO7fQUkT/W
=2uaX
-----END PGP SIGNATURE-----

review: Approve
Revision history for this message
bzr PQM (bzr-pqm) wrote :

Exception processing merge: 'NoneType' object is not iterable

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/diff.py'
2--- bzrlib/diff.py 2010-04-16 13:23:21 +0000
3+++ bzrlib/diff.py 2010-04-16 13:23:23 +0000
4@@ -801,10 +801,9 @@
5 try:
6 mtime = tree.get_file_mtime(file_id)
7 except errors.FileTimestampUnavailable:
8- # GZ 2010-04-13: Zero is a bad 'unavailable' time as it predates
9- # the earliest allowable date on FAT filesystems
10- mtime = 0
11- os.utime(full_path, (mtime, mtime))
12+ pass
13+ else:
14+ os.utime(full_path, (mtime, mtime))
15 if not allow_write:
16 osutils.make_readonly(full_path)
17 return full_path