Stefan Sperling <stsp@stsp.name> wrote:
> gotadmin cleanup currently errors out if the HEAD reference does not
> point at an existing branch. The most usual case looks like this:
> 
>   $ got ref -l
>   HEAD: refs/heads/main
>   refs/heads/master: de66c9099a70943844a8350b63e14b42d7cc7a7d
>   $ gotadmin cleanup
>   gotadmin: reference refs/heads/main not found
> 
> This can happen when people neglect to set a remote repository's HEAD
> and never send a branch called 'main' to the server.
> 
> I've seen this happen on gothub.org which defaults to 'main' when
> repositories are initially created. If users never send a main branch
> then the HEAD will be left dangling. Users don't necesarily realize that
> there is an issue because 'git clone' and 'got clone' have workarounds
> for this situation. Cloning the repository will succeed anyway.
> 
> However, cleanup triggered from cronjobs will always fail, which results
> in performance degrading over time as more and more pack files accumulate.
> 
> This patch makes gotadmin cleanup succeed in this case by ignoring
> symbolic references which cannot be resolved.
> 
> ok?

ok op@, just one question and a nit below, but they're not important I
think.

> --- lib/reference.c
> +++ lib/reference.c
> @@ -678,8 +678,11 @@ get_committer_time(struct got_reference *ref, struct g
>  	struct got_object_id *id = NULL;
>  
>  	err = got_ref_resolve(&id, repo, ref);
> -	if (err)
> +	if (err) {
> +		if (err->code == GOT_ERR_NOT_REF && got_ref_is_symbolic(ref))
> +			return NULL;
>  		return err;
> +	}

I was a bit worried about this on a first read, but it seems it's only
used for sorting so it's okay I guess.

>  	err = got_object_get_type(&obj_type, repo, id);
>  	if (err)
> [...]
> --- regress/cmdline/cleanup.sh
> +++ regress/cmdline/cleanup.sh
> @@ -694,6 +694,41 @@ EOF
>  	test_done "$testroot" "$ret"
>  }
>  
> +test_cleanup_no_head_branch() {
> +	local testroot=`test_init cleanup_no_head_branch`
> +	local commit_id=`git_show_head $testroot/repo`
> +
> +	# create a dangling HEAD reference
> +	echo 'ref: refs/heads/main' > $testroot/repo/.git/HEAD

just in case we'll switch the default branch to main in the future (iirc
git 3.0 will default to main as well), can we call this with another
name?

"foo" maybe?

so that we have one less thing to change in the future maybe

> +	# list references
> +	got ref -r $testroot/repo -l > $testroot/stdout
> +	cat > $testroot/stdout.expected <<EOF
> +HEAD: refs/heads/main
> +refs/heads/master: $commit_id
> +EOF