On Sat, Dec 26, 2020 at 09:39:25PM +0100, Christian Weisgerber wrote:
> Stefan Sperling:
> 
> > > Another change we may want is optional skipping of got_ref_list() inside
> > > got_repo_match_object_id(). It needs refs to match user input with tag names. 
> > 
> > This patch implements the above idea (on top of the previous patch).
> > Now 'tog diff A B' will not read references twice on startup.
> > And got and gotweb also avoid loading references repeatedly.
> > 
> 
> > diff refs/heads/togref refs/heads/match_object_id_refs
> 
> That looks fine.  One comment:
> 
> > @@ -6380,9 +6383,12 @@ tog_log_with_path(int argc, char *argv[])
> >  	if (error)
> >  		goto done;
> >  
> > +	error = got_ref_list(&refs, repo, NULL, got_ref_cmp_by_name, NULL);
> > +	if (error)
> > +		goto done;
> >  	error = got_repo_match_object_id(&commit_id, NULL, worktree ?
> >  	    got_worktree_get_head_ref_name(worktree) : GOT_REF_HEAD,
> > -	    GOT_OBJ_TYPE_COMMIT, 1, repo);
> > +	    GOT_OBJ_TYPE_COMMIT, &refs, repo);
> >  	if (error)
> >  		goto done;
> >  
> 
> So the mpi special loads the ref list twice.
> Could we also call tog_load_refs() there and have cmd_log() skip it?
> Too hackish?

Fine with me. Please commit this on top of my patch with my Ok.

> --- tog.c.orig	Sat Dec 26 21:26:54 2020
> +++ tog.c	Sat Dec 26 21:33:57 2020
> @@ -2756,9 +2756,12 @@ cmd_log(int argc, char *argv[])
>  	if (error)
>  		goto done;
>  
> -	error = tog_load_refs(repo);
> -	if (error)
> -		goto done;
> +	/* already loaded by tog_log_with_path()? */
> +	if (SIMPLEQ_EMPTY(&tog_refs)) {
> +		error = tog_load_refs(repo);
> +		if (error)
> +			goto done;
> +	}
>  
>  	if (start_commit == NULL) {
>  		error = got_repo_match_object_id(&start_id, &label,
> @@ -6353,10 +6356,7 @@ tog_log_with_path(int argc, char *argv[])
>  	struct got_object_id *commit_id = NULL, *id = NULL;
>  	char *cwd = NULL, *repo_path = NULL, *in_repo_path = NULL;
>  	char *commit_id_str = NULL, **cmd_argv = NULL;
> -	struct got_reflist_head refs;
>  
> -	SIMPLEQ_INIT(&refs);
> -
>  	cwd = getcwd(NULL, 0);
>  	if (cwd == NULL)
>  		return got_error_from_errno("getcwd");
> @@ -6383,12 +6383,12 @@ tog_log_with_path(int argc, char *argv[])
>  	if (error)
>  		goto done;
>  
> -	error = got_ref_list(&refs, repo, NULL, got_ref_cmp_by_name, NULL);
> +	error = tog_load_refs(repo);
>  	if (error)
>  		goto done;
>  	error = got_repo_match_object_id(&commit_id, NULL, worktree ?
>  	    got_worktree_get_head_ref_name(worktree) : GOT_REF_HEAD,
> -	    GOT_OBJ_TYPE_COMMIT, &refs, repo);
> +	    GOT_OBJ_TYPE_COMMIT, &tog_refs, repo);
>  	if (error)
>  		goto done;
>  
> @@ -6435,7 +6435,7 @@ done:
>  			free(cmd_argv[i]);
>  		free(cmd_argv);
>  	}
> -	got_ref_list_free(&refs);
> +	tog_free_refs();
>  	return error;
>  }
>  
> -- 
> Christian "naddy" Weisgerber                          naddy@mips.inka.de
> 
>