commit eee114b41cf7ab91bbbd07d50d4d496161c901b7
from: Stefan Sperling <stsp@stsp.name>
via: Thomas Adam <thomas@xteddy.org>
date: Tue May 31 23:14:47 2022 UTC

fix paths stored in pack meta data, improving file deltification

The old code was broken and stored an empty path or filenames, instead
of a repository-relative path. Which means we didn't sort files for
deltification as was intended.
Fixing this provides much better deltas in large pack files written by
gotadmin pack -a. In my test case, pack size changed from 2GB to 1.5GB.

ok op@

commit - 36832a8e24b87436ab3bb16483022e6e6d458699
commit + eee114b41cf7ab91bbbd07d50d4d496161c901b7
blob - 844cd590c40792437ed4d3a04c3bda1896d7d69c
blob + 72ca2f999a2f5bb3eda4f7075e6c1d7959be3faa
--- lib/pack_create.c
+++ lib/pack_create.c
@@ -953,6 +953,8 @@ load_tree_entries(struct got_object_id_queue *ids, int
 			err = got_object_qid_alloc(&qid, id);
 			if (err)
 				break;
+			qid->data = p;
+			p = NULL;
 			STAILQ_INSERT_TAIL(ids, qid, entry);
 		} else if (S_ISREG(mode) || S_ISLNK(mode)) {
 			err = add_object(want_meta,
@@ -962,9 +964,12 @@ load_tree_entries(struct got_object_id_queue *ids, int
 			    progress_cb, progress_arg, rl);
 			if (err)
 				break;
+			free(p);
+			p = NULL;
+		} else {
+			free(p);
+			p = NULL;
 		}
-		free(p);
-		p = NULL;
 	}
 
 	got_object_tree_close(tree);
@@ -991,12 +996,19 @@ load_tree(int want_meta, struct got_object_idset *idse
 
 	err = got_object_qid_alloc(&qid, tree_id);
 	if (err)
+		return err;
+	qid->data = strdup(dpath);
+	if (qid->data == NULL) {
+		err = got_error_from_errno("strdup");
+		got_object_qid_free(qid);
 		return err;
+	}
 
 	STAILQ_INIT(&tree_ids);
 	STAILQ_INSERT_TAIL(&tree_ids, qid, entry);
 
 	while (!STAILQ_EMPTY(&tree_ids)) {
+		const char *path;
 		if (cancel_cb) {
 			err = (*cancel_cb)(cancel_arg);
 			if (err)
@@ -1005,32 +1017,38 @@ load_tree(int want_meta, struct got_object_idset *idse
 
 		qid = STAILQ_FIRST(&tree_ids);
 		STAILQ_REMOVE_HEAD(&tree_ids, entry);
+		path = qid->data;
 
 		if (got_object_idset_contains(idset, &qid->id) ||
 		    got_object_idset_contains(idset_exclude, &qid->id)) {
+			free(qid->data);
 			got_object_qid_free(qid);
 			continue;
 		}
 
 		err = add_object(want_meta, want_meta ? idset : idset_exclude,
-		    &qid->id, dpath, GOT_OBJ_TYPE_TREE,
+		    &qid->id, path, GOT_OBJ_TYPE_TREE,
 		    mtime, loose_obj_only, repo,
 		    ncolored, nfound, ntrees, progress_cb, progress_arg, rl);
 		if (err) {
+			free(qid->data);
 			got_object_qid_free(qid);
 			break;
 		}
 
 		err = load_tree_entries(&tree_ids, want_meta, idset,
 		    idset_exclude, &qid->id,
-		    dpath, mtime, repo, loose_obj_only, ncolored, nfound,
+		    path, mtime, repo, loose_obj_only, ncolored, nfound,
 		    ntrees, progress_cb, progress_arg, rl,
 		    cancel_cb, cancel_arg);
+		free(qid->data);
 		got_object_qid_free(qid);
 		if (err)
 			break;
 	}
 
+	STAILQ_FOREACH(qid, &tree_ids, entry)
+		free(qid->data);
 	got_object_id_queue_free(&tree_ids);
 	return err;
 }