Re: [PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-25 Thread René Scharfe
Am 24.12.2017 um 15:22 schrieb Jeff King: > The single-traversal thing I suspect doesn't matter much in practice. In > both cases if we would visit commit X twice, we'd immediately see on the > second visit that it has already been cleared and not do anymore work. Good point. That makes clear_com

Re: [PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-24 Thread Jeff King
On Thu, Dec 21, 2017 at 07:41:44PM +0100, René Scharfe wrote: > Am 20.12.2017 um 14:08 schrieb Jeff King: > > On Tue, Dec 19, 2017 at 10:33:55AM -0800, Junio C Hamano wrote: > > > >> Should we take the patch posted as-is and move forward? > > > > I suppose so. I don't really have anything again

Re: [PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-21 Thread René Scharfe
Am 20.12.2017 um 14:08 schrieb Jeff King: > On Tue, Dec 19, 2017 at 10:33:55AM -0800, Junio C Hamano wrote: > >> Should we take the patch posted as-is and move forward? > > I suppose so. I don't really have anything against the patch. My main > complaint was just that I don't think it's actually

Re: [PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-20 Thread Jeff King
On Tue, Dec 19, 2017 at 10:33:55AM -0800, Junio C Hamano wrote: > >> Why does prepare_revision_walk() clear the list of pending objects at > >> all? Assuming the list is append-only then perhaps remembering the > >> last handled index would suffice. > > For that matter, why does it copy, instead

Re: [PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-19 Thread Junio C Hamano
Jeff King writes: > On Mon, Dec 18, 2017 at 08:18:19PM +0100, René Scharfe wrote: > >> > The root of the matter is that the revision-walking code doesn't clean >> > up after itself. In every case, the caller is just saving these to clean >> > up commit marks, isn't it? >> >> bundle also checks i

Re: [PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-19 Thread Jeff King
On Mon, Dec 18, 2017 at 08:18:19PM +0100, René Scharfe wrote: > > The root of the matter is that the revision-walking code doesn't clean > > up after itself. In every case, the caller is just saving these to clean > > up commit marks, isn't it? > > bundle also checks if the pending objects exists

Re: [PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-18 Thread René Scharfe
Am 18.12.2017 um 16:10 schrieb Jeff King: > On Sat, Dec 16, 2017 at 01:12:16PM +0100, René Scharfe wrote: > >> prepare_revision_walk() allows callers to take ownership of the array of >> pending objects by setting the rev_info flag "leak_pending" and copying >> the object_array "pending". They us

Re: [PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-18 Thread Jeff King
On Sat, Dec 16, 2017 at 01:12:16PM +0100, René Scharfe wrote: > prepare_revision_walk() allows callers to take ownership of the array of > pending objects by setting the rev_info flag "leak_pending" and copying > the object_array "pending". They use it to clear commit marks after > setup is done.

Re: [PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-17 Thread Martin Ågren
On 16 December 2017 at 13:12, René Scharfe wrote: > prepare_revision_walk() allows callers to take ownership of the array of > pending objects by setting the rev_info flag "leak_pending" and copying > the object_array "pending". They use it to clear commit marks after > setup is done. This inter

[PATCH] revision: introduce prepare_revision_walk_extended()

2017-12-16 Thread René Scharfe
prepare_revision_walk() allows callers to take ownership of the array of pending objects by setting the rev_info flag "leak_pending" and copying the object_array "pending". They use it to clear commit marks after setup is done. This interface is brittle enough that it requires extensive comments.