Re: [PATCH 0/6] commit-graph: use 'struct object_directory *' everywhere
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Feb 3, 2020, 04:38 UTC
- Message-ID
- <20200203043803.GC23625@syl.local>
- In-Reply-To
- <60a4d129-c6a9-1e5f-a467-7db0babbcffa@gmail.com>
On Fri, Jan 31, 2020 at 08:22:42AM -0500, Derrick Stolee wrote:
Show 25 quoted lines
> On 1/31/2020 5:30 AM, Jeff King wrote: > > On Thu, Jan 30, 2020 at 03:00:40PM -0800, Taylor Blau wrote: > > > >> This series became a little bit longer than I was expecting it to be, so > >> here is the high-level structure: > >> > >> - 1/6 fixes a bug in a test that would cause a subsequent failure if > >> unaddressed. > >> > >> - 2/6 does the first half of the removal by using 'struct > >> object_directory *'s within the 'commit_graph' structure. > >> > >> - 4/6 does the second half by removing 'char *object_dir' usage in the > >> 'write_commit_graph_context' structure. > >> > >> - 5/6 ties 2/6 and 4/6 together by removing all path normalization > >> completely, fixing the uninitialized read bug. > >> > >> - And 6/6 cleans up. > > > > With the exception of the patch-ordering discussion in the sub-thread > > with Martin, this looks good to me. > > I agree. Martin's comment is a good one. I can't find anything else > to improve the series.
Thanks for your review!
Show 13 quoted lines
> > Patch 3 is a change in user-visible behavior, as it restricts how > > --object-dir can be used (it must be the main object-dir or an alternate > > within the repository). I don't _think_ anybody would care, as the > > semantics of those options seemed kind of ill-defined to me in the first > > place. But it's worth calling out as a potential risk. I suppose the > > alternative is to make a one-off fake "struct object_directory" within > > the process that isn't connected to the repository. But if nobody cares, > > I'd just as soon avoid that. > > I think that this change of behavior is fine, especially because if > someone writes a commit-graph to an --object-dir that is not an > alternate, then that repo will not discover the new commit-graph > anyway.
And thanks for the ack. I would be somewhat surprised if someone were really relying on this behavior in practice.
Show 5 quoted lines
> I do like that you state a possible work-around in case someone shows > up with a legitimate use case for a non-alternate object-dir. > > Thanks, > -Stolee
Thanks, Taylor