On Fri, Dec 18, 2020 at 01:18:48PM +0100, Stefan Sperling wrote:
> On Thu, Dec 17, 2020 at 07:59:16PM -0500, Josh Rickmar wrote:
> > On Thu, Dec 17, 2020 at 09:35:46PM +0100, Stefan Sperling wrote:
> > > On IRC jrick reported that this command triggers a segfault:
> > > 
> > >   $ got blame distrib/miniroot/install.sub
> > > 
> > >   assertion "blame->f2" failed: file "/home/jrick/src/got/tog/../lib/blame.c", line 480, function "close_file2_and_reuse_file1"
> > > 
> > > In fact, both file pointers in the 'blame' struct are NULL.
> > > 
> > > The problem is that the commit graph traverses one commit too far.
> > > This can be more easily reproduced with 'got log':
> > > 
> > >  got log -c 05f568 -P distrib/miniroot/install.sub
> > > 
> > > This lists two commits:
> > >   05f568ecc6aadefa1aff9064a29e798874a71409
> > >   7c0d87f00e480cdf004324dad6f3e6f4418f8f42 
> > > 
> > > commit 05f568ecc6aade... added the file distrib/miniroot/install.sub.
> > > The above 'got log' command returns not just this commit as expected,
> > > it additionally returns commit 7c0d87f00e480cd...
> > > 
> > > The problem is a bug in the logic of got_object_tree_path_changed(),
> > > which causes the commit graph to add 7c0d87f00e480...  to the list of
> > > commits which must still be traversed, even though the history of the
> > > logged path has already ended.
> > > 
> > > Note that "distrib/miniroot" exists in 7c0d87f00e480cdf004324... but not
> > > in the parent of this commit.
> > > 
> > > At the point where are traversing "distrib/miniroot" the tree entry te1
> > > exists and corresponds to this tree listing the contents of 'miniroot'
> > > in 7c0d87f00e480cdf004324...:
> > > 
> > > $ got cat 180aa33df8d1
> > 
> > Where does object come from?
> 
> It is the tree object for the path 'distrib/miniroot' in commit 7c0d87f:
> 
> $ got tree -c 7c0d87f -i distrib | grep miniroot
> 180aa33df8d134a83f10591911c91dc439db8c76 miniroot/
> 
> > Other than the question above (which is just me trying to understand
> > how this was debugged), this makes sense to me and the patch looks
> > right.  I confirmed that the segfault and the debug assert i added is
> > no longer being hit, and got log -P only shows the 05f568ecc6aade
> > commit.
> 
> Thanks. So with the above question addressed, this is ok by you?

Yep, ok.