On Wed, Nov 16, 2022 at 01:14:18PM -0800, Jonathan Tan wrote: > > But there may be some subtlety I'm missing. I'm cc-ing Jonathan Tan, who > > added has_object(), and who added the top call to repo_has_object_file() > > via df11e19648 (rev-list: support termination at promisor objects, > > 2017-12-08). > > Thanks for CC-ing me on this. Looking at that commit and the code at that time, > I'm not sure why I added that call either. My best guess is that I was worried > that the streaming interface wouldn't support missing objects, but both then > and now, a call to istream_source() is made before any streaming occurs (which > does perform the lazy fetch). > > So yes, I also think that you can remove these calls. Thanks. After staring at this a bit, I noticed there is an even more subtle issue with the case you touched back then, which is that it fails to notice when a think we expect to be a blob isn't one. Your old patch didn't make anything worse there, but it also wasn't sufficient to catch the problem. See patch 2 for details. I'm adding Taylor to the cc as the author of t6102, when we were tracking down all of these "oops, it's not really a blob" cases. This fixes one of the lingering cases from that test script. [1/2]: parse_object(): drop extra "has" check before checking object type [2/2]: parse_object(): check on-disk type of suspected blob object.c | 5 ++--- t/t6102-rev-list-unexpected-objects.sh | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) -Peff
