From: Jonathan Tan Date: Fri, 19 Aug 2022 17:25:19 GMT Subject: Re: [PATCH v2 2/2] revision: allow --ancestry-path to take an argument Message-ID: <20220819172519.3703282-1-jonathantanmy@google.com> In-Reply-To: Elijah Newren writes: > > Optional: Besides that, from what I can tell, sometimes the C commits > > themselves are marked with ANCESTRY_PATH (when they are explicitly > > specified) and sometimes they are not (when they are not explicitly > > specified). It's not a bug here, but it might be worth handling that in > > the ancestry_path_need_bottoms codepath (instead of explicitly setting > > TMP_MARK on the bottoms in limit_to_ancestry() - that way, I think we > > can also use ANCESTRY_PATH instead of TMP_MARK throughout the ancestry > > path codepaths, but I haven't tested it), at least to prevent possible > > future bugs. > > That sounds like you're trying to duplicate the bug in my first > attempt at this patch. If you try to coalesce ANCESTRY_PATH and > TMP_MARK, then you not only get all descendants of C, you also get all > descendants of any ancestor of C, which defeats the whole point of my > changes. Ah, yes you're right. > It's true that I don't mark implicit C commits with ANCESTRY_PATH, but > those are always bottom commits that are the excluded end of a range > anyway. While those could be marked without causing problems, it > would always be a waste of effort. Yes, that's true.