On Wed, Sep 11, 2024 at 03:18:34PM +0000, Phillip Wood via GitGitGadget wrote:
> From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
>
> To replace the list of branches to be fetched "git remote set-branches"
> first removes the fetch refspecs for the remote and then creates a new
> set of fetch refspecs based and the branches passed on the commandline.
s/and/on/
> When deleting the existing refspecs git_config_set_multivar_gently()
> will return a non-zero result if there was nothing to delete.
> Unfortunately the calling code treats that as an error and bails out
> rather than setting up the new branches. Fix this by not treating a
> return value of CONFIG_NOTHING_SET as an error.
>
> Reported-by: Han Jiang <jhcarl0814@xxxxxxxxx>
> Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
> ---
> builtin/remote.c | 8 ++++++--
> t/t5505-remote.sh | 14 +++++++++++---
> 2 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/remote.c b/builtin/remote.c
> index d1f9292ed2b..794396ba02f 100644
> --- a/builtin/remote.c
> +++ b/builtin/remote.c
> @@ -1567,8 +1567,12 @@ static int update(int argc, const char **argv, const char *prefix)
>
> static int remove_all_fetch_refspecs(const char *key)
> {
> - return git_config_set_multivar_gently(key, NULL, NULL,
> - CONFIG_FLAGS_MULTI_REPLACE);
> + int res = git_config_set_multivar_gently(key, NULL, NULL,
> + CONFIG_FLAGS_MULTI_REPLACE);
> + if (res == CONFIG_NOTHING_SET)
> + res = 0;
> +
> + return res;
> }
Makes sense.
> static void add_branches(struct remote *remote, const char **branches,
> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
> index 08424e878e1..cfbd6139e00 100755
> --- a/t/t5505-remote.sh
> +++ b/t/t5505-remote.sh
> @@ -1131,7 +1131,9 @@ test_expect_success 'remote set-branches' '
> +refs/heads/next:refs/remotes/scratch/next
> +refs/heads/seen:refs/remotes/scratch/seen
> EOF
> -
> + cat <<-\EOF >expect.replace-missing &&
s/ / /
Also, the redirect typically comes before the heredoc marker.
> + +refs/heads/topic:refs/remotes/scratch/topic
> + EOF
> git clone .git/ setbranches &&
> (
> cd setbranches &&
> @@ -1161,14 +1163,20 @@ test_expect_success 'remote set-branches' '
>
> git remote set-branches --add scratch seen &&
> git config --get-all remote.scratch.fetch >config-result &&
> - sort <config-result >../actual.respect-ffonly
> + sort <config-result >../actual.respect-ffonly &&
> +
> + git config --unset-all remote.scratch.fetch &&
> + git remote set-branches scratch topic &&
> + git config --get-all remote.scratch.fetch \
> + >../actual.replace-missing
I wonder whether we'd rather wnat to wire this up in a new test instead
of altering an existing one.
Patrick
