From: Omar Polo <op@omarpolo.com>
Subject: Re: got send vs symlinks
To: Stefan Sperling <stsp@stsp.name>
Cc: gameoftrees@openbsd.org
Date: Wed, 22 Sep 2021 15:20:07 +0200


Stefan Sperling <stsp@stsp.name> writes:

> On Wed, Sep 22, 2021 at 11:40:06AM +0200, Omar Polo wrote:
>> Hello,
>> 
>> I've come across another instance where `got send' doesn't seem to
>> assemble the packfile correctly, this time using symlink.
>> 
>> The full reproducible example follows, but the gist is that `got send'
>> fails to upload a commit that includes an added symlink to a file
>> already in the repository.  `git push' works with the commit produced by
>> `got ci'.
>> 
>>[..]
>
> Indeed we should be including "symlinks" in the pack file.
> Which are really just blobs internally that contain the symlink target path.
> The dirent mode bits which make it a symlink are stored in the tree object
> which points at this blob.
>
> Turns out we were simply skipping symlinks when creating the pack.
> This updates the corresponding test to reproduce the issue and fixes it.
>
> ok?

How fast :)

Thanks, it fixes the example I found, and the regression suite still
passes

Cheers!

> diff ad324bf53a11587dc227a2c00b65020595aa18dc /home/stsp/src/got
> blob - 6f64f97e5ab0a743040b77f12b907d6537a7f7f5
> file + lib/pack_create.c
> --- lib/pack_create.c
> +++ lib/pack_create.c
> @@ -372,8 +372,7 @@ load_tree_entries(struct got_object_id_queue *ids, str
>  				break;
>  		}
>  
> -		if (got_object_tree_entry_is_symlink(e) ||
> -		    got_object_tree_entry_is_submodule(e) ||
> +		if (got_object_tree_entry_is_submodule(e) ||
>  		    got_object_idset_contains(idset, id))
>  			continue;
>  		
> @@ -389,7 +388,7 @@ load_tree_entries(struct got_object_id_queue *ids, str
>  			if (err)
>  				break;
>  			STAILQ_INSERT_TAIL(ids, qid, entry);
> -		} else if (S_ISREG(mode)) {
> +		} else if (S_ISREG(mode) || S_ISLNK(mode)) {
>  			err = add_meta(v, idset, id, p, GOT_OBJ_TYPE_BLOB,
>  			    mtime, loose_obj_only, repo);
>  			if (err)
> blob - 5fdd09a2185f5b0ec3129d8c2ff58780671c2b0a
> file + regress/cmdline/send.sh
> --- regress/cmdline/send.sh
> +++ regress/cmdline/send.sh
> @@ -40,6 +40,10 @@ EOF
>  		| tr -d ' ' | cut -d: -f2`
>  
>  	echo "modified alpha" > $testroot/repo/alpha
> +	(cd $testroot/repo && git rm -q beta)
> +	(cd $testroot/repo && ln -s epsilon/zeta symlink && git add symlink)
> +	echo "new file alpha" > $testroot/repo/new
> +	(cd $testroot/repo && git add new)
>  	git_commit $testroot/repo -m "modified alpha"
>  	local commit_id2=`git_show_head $testroot/repo`
>  
> @@ -103,6 +107,18 @@ EOF
>  		return 1
>  	fi
>  
> +	got tree -r $testroot/repo-clone -c $commit_id2 -i -R \
> +		> $testroot/stdout
> +	got tree -r $testroot/repo -c $commit_id2 -i -R \
> +		> $testroot/stdout.expected
> +	cmp -s $testroot/stdout $testroot/stdout.expected
> +	ret="$?"
> +	if [ "$ret" != "0" ]; then
> +		diff -u $testroot/stdout.expected $testroot/stdout
> +		test_done "$testroot" "$ret"
> +		return 1
> +	fi
> +
>  	got send -r $testroot/repo > $testroot/stdout 2> $testroot/stderr
>  	ret="$?"
>  	if [ "$ret" != "0" ]; then