[MERGE] make sha1_provider a parameter to DirState()

[Martin Pool]

Martin Pool has voted tweak.
Status is now: Conditionally approved
Comment:
Could you change it to SHA (in uppercase) which is consistent with other 
APIs?

+class DefaultSha1Provider(Sha1Provider):
+    """A Sha1Provider that reads directly from the filesystem."""
+
+    def sha1(self, abspath):
+        """Return the sha1 of a file given it's absolute path."""
+        return osutils.sha_file_by_name(abspath)
+
+    def stat_and_sha1(self, abspath):
+        """Return the stat and sha1 of a file given it's absolute 
path."""
+        file_obj = file(abspath, 'rb')
+        try:
+            statvalue = os.fstat(file_obj.fileno())
+            sha1 = osutils.sha_file(file_obj)
+        finally:
+            file_obj.close()

It's "its" like "his".

I feel a bit paranoid, maybe unjustifiably, that someone will do the 
stat after reading the file, allowing in a race, so I feel like that 
should be in the base class docstring.

@@ -2845,9 +2895,12 @@
              and stat_value.st_ctime < state._cutoff_time
              and len(entry[1]) > 1
              and entry[1][1][0] != 'a'):
-                # Could check for size changes for further optimised
-                # avoidance of sha1's. However the most prominent case 
of
-                # over-shaing is during initial add, which this 
catches.
+            # Could check for size changes for further optimised
+            # avoidance of sha1's. However the most prominent case of
+            # over-shaing is during initial add, which this catches.
+            # Besides, if content filtering happens, size and sha
+            # are calculated at the same time, so checking just the 
size
+            # gains nothing w.r.t. performance.
              link_or_sha1 = state._sha1_file(abspath)
              entry[1][0] = ('f', link_or_sha1, stat_value.st_size,
                             executable, packed_stat)

I realize you didn't originate it, but this comment seems spurious.  If 
the size is the same that's no guarantee the SHA is the same, and if 
it's changed I don't see how that helps.  (Maybe it does, I haven't 
studied the broader context.)  At any rate this change is ok.

It would be nice if you could just fire up a dirstate without needing a 
tree with commits but that's a bit out of scope.  Maybe next time 
someone works on dirstate.

Nice work!

For details, see: 
http://bundlebuggy.aaronbentley.com/project/bzr/request/%3C49BA1335.5060506%40internode.on.net%3E
Project: Bazaar