Thanks, Peff and Junio for your comments. Here's an updated version and some replies to comments. > I also found this quite subtle. However, I don't think that > has_sha1_file() actually freshens. It's a bit confusing because > has_loose_object() reuses the check_and_freshen() function to do the > lookup, but it actually sets the "freshen" flag to false. > > That's why in 33d4221c7 (write_sha1_file: freshen existing objects, > 2014-10-15), which introduced the freshening functions and converted > has_loose_object(), the actual write_sha1_file() function switched to > using the freshening functions directly (and obviously sets the freshen > parameter to true). Good catch. > I actually think all of that infrastructure could become part of > Jonathan's consolidated lookup, too. We would just need: > > 1. A QUICK flag to avoid re-reading objects/pack when we don't find > anything (which it looks like he already has). > > 2. A FRESHEN flag to update the mtime of any item that we do find. > > I suspect we may also need something like ONLY_LOOSE and ONLY_NONLOCAL > to meet all the callers (e.g., has_loose_object_nonlocal). Those should > be easy to implement, I'd think. For things like FRESHEN, ONLY_LOOSE, and ONLY_NONLOCAL, I was thinking that I would like to restrict these patches to only handle the cases that are agnostic to the type of storage (in preparation for missing blob handling patches). > I had the same thoughts (both on the name and the "vocabularies"). IMHO > we should consider allocating the bits from the same set. There's only > one HAS_SHA1 flag, and it has an exact match in OBJECT_INFO_QUICK. Agreed - in this patch set, I have also consolidated the relevant flags, including LOOKUP_REPLACE_OBJECT and LOOKUP_UNKNOWN_OBJECT. In addition, Junio has mentioned the potential confusion in behavior between a NULL and an empty struct passed to sha1_object_info_extended(). In this patch set, I require non-NULL, and have added an optimization that avoids accessing the pack in certain situations, but this optimization requires checking a lot of fields. Let me know what you think. Jonathan Tan (8): sha1_file: teach packed_object_info about typename sha1_file: rename LOOKUP_UNKNOWN_OBJECT sha1_file: rename LOOKUP_REPLACE_OBJECT sha1_file: move delta base cache code up sha1_file: refactor read_object sha1_file: improve sha1_object_info_extended sha1_file: do not access pack if unneeded sha1_file: refactor has_sha1_file_with_flags builtin/cat-file.c | 7 +- builtin/fetch.c | 10 +- builtin/index-pack.c | 3 +- cache.h | 37 +++-- sha1_file.c | 391 ++++++++++++++++++++++++++------------------------- streaming.c | 1 + 6 files changed, 228 insertions(+), 221 deletions(-) -- 2.13.1.611.g7e3b11ae1-goog
