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

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

Download raw body.

Thread
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'.
> 
> Thanks,
> 
> Omar Polo
> 
> 
> % echo 'hello there' > regular-file
> % got add regular-file
> A  regular-file
> % got ci -m 'added a regular file' regular-file
> A  regular-file
> Created commit dcc021340d50824cc5fb99e9a87b2dc7b269e671
> % ln -s regular-file symlink
> % got add symlink
> A  symlink
> % got ci -m 'added a symlink' symlink
> A  symlink
> Created commit 3018b22349d1547665ce292f2e543f9055a8f394
> % got send
> Connecting to "origin" git@omarpolo.com
> packing 1 reference; 5 objects; deltify: 100%; uploading pack:    562B 100%fatal: missing blob object 'b844002558b5c724cf8a667b4b8c52366e04766a'
> fatal: missing blob object 'b844002558b5c724cf8a667b4b8c52366e04766a'
> got-send-pack: unexpected message from server
> 
> got: bad packet received
> %
> 
> 

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?

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