"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: fix cloning of repositories with dangling head reference
To:
Stefan Sperling <stsp@stsp.name>
Cc:
gameoftrees@openbsd.org
Date:
Tue, 08 Nov 2022 17:01:35 +0100

Download raw body.

Thread
On 2022/11/07 20:53:52 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> Make 'got clone -b' work for repositories which lack a HEAD reference
> that points to an existing branch.
> 
> Regression test which reproduces the problem is included.
> 
> Patch fixes this error:
>   got-fetch-pack: could not find any branches to fetch
> 
> Also ensures that default config files get created in this case.
> 
> ok?

ok op@;  some comments below however.

> diff ca7cfae029f0cbca1f65f326025744f627b69d92 593af7bd3baeb8314b7ef9a11fe0559a6ba9557d
> commit - ca7cfae029f0cbca1f65f326025744f627b69d92
> commit + 593af7bd3baeb8314b7ef9a11fe0559a6ba9557d
> blob - c0c38ea47dfbd657ee0c14d0e8829767ad691ae6
> blob + 1e29e64a3d1e97b5787efdf1754a70fe1615d190
> --- got/got.c
> +++ got/got.c
> @@ -1856,6 +1856,7 @@ cmd_clone(int argc, char *argv[])
>  			goto done;
>  	}
>  	if (pe == NULL) {
> +		struct got_reference *target_ref = NULL;

I'd leave an empty line here

>  		/*
>  		 * We failed to set the HEAD reference. If we asked for
>  		 * a set of wanted branches use the first of one of those
> @@ -1863,12 +1864,12 @@ cmd_clone(int argc, char *argv[])
>  		 */
>  		TAILQ_FOREACH(pe, &wanted_branches, entry) {
>  			const char *target = pe->path;
> -			struct got_reference *target_ref;
>  
>  			error = got_ref_open(&target_ref, repo, target, 0);
>  			if (error) {
>  				if (error->code == GOT_ERR_NOT_REF) {
>  					error = NULL;
> +					target_ref = NULL;
>  					continue;
>  				}
>  				goto done;
> @@ -1881,6 +1882,20 @@ cmd_clone(int argc, char *argv[])
>  				goto done;
>  			break;
>  		}
> +
> +		if (!fpa.configs_created && target_ref) {

Reached this point target_ref is either NULL or a free'd pointer.
It's fine; the integral number doesn't change after passing it to
got_ref_close, but I just felt to pointing it out.

> +			error = create_config_files(fpa.config_info.proto,
> +			    fpa.config_info.host, fpa.config_info.port,
> +			    fpa.config_info.remote_repo_path,
> +			    fpa.config_info.git_url,
> +			    fpa.config_info.fetch_all_branches,
> +			    fpa.config_info.mirror_references,
> +			    fpa.config_info.symrefs,
> +			    fpa.config_info.wanted_branches,
> +			    fpa.config_info.wanted_refs, fpa.repo);
> +			if (error)
> +				goto done;
> +		}
>  	}
> [...]
> --- regress/cmdline/clone.sh
> +++ regress/cmdline/clone.sh
> [...]
> +	got clone -q -b foo $testurl/repo $testroot/repo-clone
> +	ret=$?
> +	if [ $ret -ne 0 ]; then
> +		echo "got clone command failed unexpectedly" >&2
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
> +	got ref -l -r $testroot/repo-clone > $testroot/stdout
> +
> +	echo "HEAD: refs/heads/master" > $testroot/stdout.expected
> +	echo "refs/heads/foo: $commit_id" >> $testroot/stdout.expected
> +	echo "refs/remotes/origin/foo: $commit_id" >> $testroot/stdout.expected

forgot to cmp stdout and stdout.expected

> +	cat > $testroot/got.conf.expected <<EOF
> +remote "origin" {
> +	server 127.0.0.1
> +	protocol ssh
> +	repository "$testroot/repo"
> +	branch { "foo" }
> +}
> [...]