git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 1/8] fetch-pack: split up everything_local()

From
BWBrandon Williams <bmwill@google.com>
Date
Jun 14, 2018, 17:26 UTC
Message-ID
<20180614172615.GA220741@google.com>
In-Reply-To
<5687c0b22bf3b2bdbbe29b09788cc305a89710fc.1528317619.git.jonathantanmy@google.com>
On 06/06, Jonathan Tan wrote:
Show 11 quoted lines
> The function everything_local(), despite its name, also (1) marks
> commits as COMPLETE and COMMON_REF and (2) invokes filter_refs() as
> important side effects. Extract (1) into its own function
> (mark_complete_and_common_ref()) and remove
> (2).
> 
> The restoring of save_commit_buffer, which was introduced in a1c6d7c1a7
> ("fetch-pack: restore save_commit_buffer after use", 2017-12-08), is a
> concern of the parse_object() call in mark_complete_and_common_ref(), so
> it has been moved from the end of everything_local() to the end of
> mark_complete_and_common_ref().
Thanks, this is much cleaner.
Show 89 quoted lines
> 
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  fetch-pack.c | 39 ++++++++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/fetch-pack.c b/fetch-pack.c
> index a320ce987..5c87bb8bb 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -734,12 +734,20 @@ static int add_loose_objects_to_set(const struct object_id *oid,
>  	return 0;
>  }
>  
> -static int everything_local(struct fetch_pack_args *args,
> -			    struct ref **refs,
> -			    struct ref **sought, int nr_sought)
> +/*
> + * Mark recent commits available locally and reachable from a local ref as
> + * COMPLETE. If args->no_dependents is false, also mark COMPLETE remote refs as
> + * COMMON_REF (otherwise, we are not planning to participate in negotiation, and
> + * thus do not need COMMON_REF marks).
> + *
> + * The cutoff time for recency is determined by this heuristic: it is the
> + * earliest commit time of the objects in refs that are commits and that we know
> + * the commit time of.
> + */
> +static void mark_complete_and_common_ref(struct fetch_pack_args *args,
> +					 struct ref **refs)
>  {
>  	struct ref *ref;
> -	int retval;
>  	int old_save_commit_buffer = save_commit_buffer;
>  	timestamp_t cutoff = 0;
>  	struct oidset loose_oid_set = OIDSET_INIT;
> @@ -812,7 +820,18 @@ static int everything_local(struct fetch_pack_args *args,
>  		}
>  	}
>  
> -	filter_refs(args, refs, sought, nr_sought);
> +	save_commit_buffer = old_save_commit_buffer;
> +}
> +
> +/*
> + * Returns 1 if every object pointed to by the given remote refs is available
> + * locally and reachable from a local ref, and 0 otherwise.
> + */
> +static int everything_local(struct fetch_pack_args *args,
> +			    struct ref **refs)
> +{
> +	struct ref *ref;
> +	int retval;
>  
>  	for (retval = 1, ref = *refs; ref ; ref = ref->next) {
>  		const struct object_id *remote = &ref->old_oid;
> @@ -829,8 +848,6 @@ static int everything_local(struct fetch_pack_args *args,
>  			      ref->name);
>  	}
>  
> -	save_commit_buffer = old_save_commit_buffer;
> -
>  	return retval;
>  }
>  
> @@ -1053,7 +1070,9 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,
>  	if (!server_supports("deepen-relative") && args->deepen_relative)
>  		die(_("Server does not support --deepen"));
>  
> -	if (everything_local(args, &ref, sought, nr_sought)) {
> +	mark_complete_and_common_ref(args, &ref);
> +	filter_refs(args, &ref, sought, nr_sought);
> +	if (everything_local(args, &ref)) {
>  		packet_flush(fd[1]);
>  		goto all_done;
>  	}
> @@ -1377,7 +1396,9 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
>  			for_each_cached_alternate(insert_one_alternate_object);
>  
>  			/* Filter 'ref' by 'sought' and those that aren't local */
> -			if (everything_local(args, &ref, sought, nr_sought))
> +			mark_complete_and_common_ref(args, &ref);
> +			filter_refs(args, &ref, sought, nr_sought);
> +			if (everything_local(args, &ref))
>  				state = FETCH_DONE;
>  			else
>  				state = FETCH_SEND_REQUEST;
> -- 
> 2.17.0.768.g1526ddbba1.dirty
> 
-- 
Brandon Williams
Previous: Jonathan TanNext: Jonathan Tan
Message 23 of 47 in “Refactor fetch negotiation into its own API”
  1. 0/6 Refactor fetch negotiation into its own APIJonathan Tan, Jun 4, 2018
  2. 1/6 fetch-pack: clear marks before everything_local()Jonathan Tan, Jun 4, 2018
  3. Jonathan NiederJun 5, 2018
  4. Jonathan TanJun 6, 2018
  5. 2/6 fetch-pack: truly stop negotiation upon ACK readyJonathan Tan, Jun 4, 2018
  6. Jonathan NiederJun 5, 2018
  7. Jonathan NiederJun 5, 2018
  8. Jonathan TanJun 6, 2018
  9. 3/6 fetch-pack: in protocol v2, enqueue commons firstJonathan Tan, Jun 4, 2018
  10. Jonathan NiederJun 5, 2018
  11. Jonathan TanJun 6, 2018
  12. 4/6 fetch-pack: make negotiation-related vars localJonathan Tan, Jun 4, 2018
  13. Jonathan NiederJun 5, 2018
  14. Jonathan TanJun 6, 2018
  15. 5/6 fetch-pack: move common check and marking togetherJonathan Tan, Jun 4, 2018
  16. Jonathan NiederJun 6, 2018
  17. Jonathan TanJun 6, 2018
  18. 6/6 fetch-pack: introduce negotiator APIJonathan Tan, Jun 4, 2018
  19. Jonathan NiederJun 6, 2018
  20. Jonathan TanJun 6, 2018
  21. 0/8 Refactor fetch negotiation into its own APIJonathan Tan, Jun 6, 2018
  22. 1/8 fetch-pack: split up everything_local()Jonathan Tan, Jun 6, 2018
  23. Brandon WilliamsJun 14, 2018
  24. 2/8 fetch-pack: clear marks before re-markingJonathan Tan, Jun 6, 2018
  25. 4/8 fetch-pack: use ref adv. to prune "have" sentJonathan Tan, Jun 6, 2018
  26. Brandon WilliamsJun 14, 2018
  27. Junio C HamanoJun 14, 2018
  28. 5/8 fetch-pack: make negotiation-related vars localJonathan Tan, Jun 6, 2018
  29. Brandon WilliamsJun 14, 2018
  30. Junio C HamanoJun 14, 2018
  31. 6/8 fetch-pack: move common check and marking togetherJonathan Tan, Jun 6, 2018
  32. 8/8 negotiator/default: use better style in commentsJonathan Tan, Jun 6, 2018
  33. Brandon WilliamsJun 14, 2018
  34. 7/8 fetch-pack: introduce negotiator APIJonathan Tan, Jun 6, 2018
  35. 3/8 fetch-pack: directly end negotiation if ACK readyJonathan Tan, Jun 6, 2018
  36. Brandon WilliamsJun 14, 2018
  37. Brandon WilliamsJun 14, 2018
  38. 0/7 Refactor fetch negotiation into its own APIJonathan Tan, Jun 14, 2018
  39. 1/7 fetch-pack: split up everything_local()Jonathan Tan, Jun 14, 2018
  40. 2/7 fetch-pack: clear marks before re-markingJonathan Tan, Jun 14, 2018
  41. 3/7 fetch-pack: directly end negotiation if ACK readyJonathan Tan, Jun 14, 2018
  42. Junio C HamanoJun 15, 2018
  43. 4/7 fetch-pack: use ref adv. to prune "have" sentJonathan Tan, Jun 14, 2018
  44. 5/7 fetch-pack: make negotiation-related vars localJonathan Tan, Jun 14, 2018
  45. 6/7 fetch-pack: move common check and marking togetherJonathan Tan, Jun 14, 2018
  46. 7/7 fetch-pack: introduce negotiator APIJonathan Tan, Jun 14, 2018
  47. Brandon WilliamsJun 25, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.