Re: [PATCH] remote: handle negative refspecs in git remote show

From: Taylor Blau <me@ttaylorr.com>
Date: Jun 14, 2022, 01:03 UTC
Message-ID: <Yqfec9yvT3LKomNK@nand.local>
In-Reply-To: <20220614003251.16765-1-jacob.e.keller@intel.com>

On Mon, Jun 13, 2022 at 05:32:51PM -0700, Jacob Keller wrote:

> Fix this by checking negative refspecs inside of get_ref_states. For
> each ref which matches a negative refspec, copy it into a "skipped" list
> and remove it from the fetch map. This allows us to show the following
> output instead:

Seems sensible.

Show 18 quoted lines

> +	/* handle negative refspecs first */
> +	for (tail = &fetch_map; *tail; ) {
> +		ref = *tail;
> +
> +		if (omit_name_by_refspec(ref->name, &states->remote->fetch)) {
> +			string_list_append(&states->skipped, abbrev_branch(ref->name));
> +
> +			/* Matched a negative refspec, so remove this ref from
> +			 * consideration for being a new or tracked ref.
> +			 */
> +			*tail = ref->next;
> +			free(ref->peer_ref);
> +			free(ref);
> +		} else {
> +			tail = &ref->next;
> +		}
> +	}
> +

Not being overly familiar with the "git remote show" code, this implementation looks very reasonable to me. If we see a negative refspec, we remove it from the fetch_map list and append it to the skipped list. Otherwise, we increment our pointer, and continue along until we reach the end of the list.

Show 30 quoted lines

> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
> index fff14e13ed43..e19b8d666c73 100755
> --- a/t/t5505-remote.sh
> +++ b/t/t5505-remote.sh
> @@ -302,6 +302,33 @@ test_expect_success 'show' '
>  	)
>  '
>
> +cat >test/expect <<EOF
> +* remote origin
> +  Fetch URL: $(pwd)/one
> +  Push  URL: $(pwd)/one
> +  HEAD branch: main
> +  Remote branches:
> +    main     skipped
> +    side     tracked
> +    upstream stale (use 'git remote prune' to remove)
> +  Local branches configured for 'git pull':
> +    ahead merges with remote main
> +    main  merges with remote main
> +  Local refs configured for 'git push':
> +    main pushes to main     (local out of date)
> +    main pushes to upstream (create)
> +EOF
> +
> +test_expect_success 'show with negative refspecs' '
> +	test_when_finished "git -C test config --fixed-value --unset remote.origin.fetch ^refs/heads/main" &&
> +	(
> +		cd test &&
> +		git config --add remote.origin.fetch ^refs/heads/main &&

Doing "git config --unset" outside of the subshell could be avoided by ditching the subshell altogether, perhaps with something like:

    test_config -C test remote.origin.fetch ^refs/heads/main &&
    git -C test remote show origin >actual &&
    test_cmp test/expect actual

Thanks, Taylor