Release v3.4.0

This release adds rename detection support which can be enabled via the
--enable-rename-detection option during configure time.

Signed-off-by: Tudor Marcu <tudor.marcu@intel.com>

Makefile.am

@@ -114,10 +114,17 @@ dist_check_SCRIPTS = \
 	test/functional/full-run/test.bats \
 	test/functional/fullfiles/test.bats \
 	test/functional/include-version-bump/test.bats \
+	test/functional/includes-deduplicate/test.bats \
 	test/functional/no-delta/test.bats \
 	test/functional/pack/test.bats \
 	test/functional/state-file/test.bats \
+	test/functional/subtract-delete/test.bats \
 	test/functional/update/test.bats
+
+if RENAMES
+dist_check_SCRIPTS += \
+	test/functional/renames/test.bats
+endif
 endif
 
 if COVERAGE

configure.ac

@@ -2,7 +2,7 @@
 # Process this file with autoconf to produce a configure script.
 
 AC_PREREQ([2.66])
-AC_INIT(swupd-server, 3.3.3, tudor.marcu@intel.com)
+AC_INIT(swupd-server, 3.4.0, tudor.marcu@intel.com)
 AM_INIT_AUTOMAKE([foreign -Wall -W subdir-objects])
 AM_SILENT_RULES([yes])
 AC_PROG_CC
@@ -82,6 +82,11 @@ AS_IF([test "$enable_lzma" != "no"], [
 ])
 AM_CONDITIONAL([ENABLE_LZMA], [test "$enable_lzma" != "no"])
 
+AC_ARG_ENABLE([rename-detection], [AS_HELP_STRING([--enable-rename-detection], [enable rename detection feature])])
+
+AS_IF([test "$enable_rename_detection" = "yes"], [AC_DEFINE(RENAMES,1,[Use rename detection])])
+AM_CONDITIONAL([RENAMES], [test "$enable_rename_detection" = "yes"])
+
 AC_CONFIG_FILES([Makefile])
 AC_REQUIRE_AUX_FILE([tap-driver.sh])
 AC_OUTPUT

include/swupd.h

@@ -65,6 +65,13 @@
 #include <lzma.h>
 #endif
 
+/* Approximatly the smallest size of a pair of input files which
+ * differ by a single bit that bsdiff can produce a more compact
+ * deltafile. Files smaller than this are always marked as different.
+ * See the magic 200 value in the bsdiff/src/diff.c code.
+ */
+#define BSDIFFSIZE 200
+
 struct manifest {
 	unsigned long long int format;
 	int version;
@@ -122,7 +129,7 @@ struct file {
 	double rename_score;
 	struct file *rename_peer;
 	char *alpha_only_filename; /* filename minus all numerics/etc */
-	char *filetype;
+	char *filetype;		   /* The output of 'file', truncated */
 	char *basename;
 	char *dirname;
 	/* end of rename detection fields */
@@ -218,8 +225,6 @@ extern void chroot_create_full(int newversion);
 
 extern void read_group_file(char *filename);
 extern void release_group_file(void);
-extern char *group_groups(char *group);
-extern char *group_packages(char *group);
 extern char *group_status(char *group);
 extern char *next_group(void);
 
@@ -242,8 +247,8 @@ extern int previous_version_manifest(struct manifest *mom, char *name);
 
 extern void type_change_detection(struct manifest *manifest);
 
-extern void rename_detection(struct manifest *manifest, int last_change, GList *last_versions_list);
-extern void link_renames(GList *newfiles, struct manifest *from_manifest);
+extern void rename_detection(struct manifest *manifest);
+extern void link_renames(GList *newfiles, int to_version);
 extern void __create_delta(struct file *file, int from_version, char *from_hash);
 
 extern void account_delta_hit(void);

src/analyze_fs.c

@@ -42,6 +42,10 @@
 
 static GThreadPool *threadpool;
 
+/* Why not strcpy? Looks like the hash was going to be stored in
+ * binary at one stage. Should use g_string_chunk_insert_const to
+ * change hash_compare to a pointer compare
+ */
 void hash_assign(char *src, char *dst)
 {
 	memcpy(dst, src, SWUPD_HASH_LEN - 1);
@@ -196,7 +200,7 @@ int compute_hash(struct file *file, char *filename)
 		return 0;
 	}
 
-	hash_set_zeros(key);
+	hash_set_zeros(key); /* Set to 64 '0' (not '\0') characters */
 
 	if (file->is_link) {
 		char link[PATH_MAX];

src/create_update.c

@@ -332,8 +332,6 @@ int main(int argc, char **argv)
 	}
 
 	apply_heuristics(new_full);
-#warning disabled rename detection for some simplicity
-	// rename_detection(new_full);
 
 	print_elapsed_time("full manifest creation", &previous_time, &current_time);
 
@@ -365,13 +363,12 @@ int main(int argc, char **argv)
 		apply_heuristics(new_core);
 		/* Step 3c: ... else save the manifest */
 		type_change_detection(new_core);
-
-#warning disabled rename detection for some simplicity
-		/* Detect renamed files specifically for each pack */
-		// rename_detection(...);
-
+#ifdef RENAMES
+		/* Detect renamed files specifically for os-core */
+		rename_detection(new_core);
+#endif
 		old_deleted = remove_old_deleted_files(old_core, new_core);
-		sort_manifest_by_version(new_core);
+		sort_manifest_by_version(new_core); /* sorts by filename */
 		newfiles = prune_manifest(new_core);
 		if (newfiles <= 0) {
 			LOG(NULL, "", "Core component has not changed (after pruning), exiting");
@@ -406,25 +403,40 @@ int main(int argc, char **argv)
 		GList *name_includes;
 		char *group = next_group();
 		struct manifest *manifest;
+		struct manifest *current = NULL;
 
 		if (!group) {
 			break;
 		}
 		manifest = g_hash_table_lookup(new_manifests, group);
 		name_includes = manifest->includes;
+
 		while (name_includes) {
 			char *name = name_includes->data;
 			name_includes = g_list_next(name_includes);
-			manifest_includes = g_list_prepend(manifest_includes, g_hash_table_lookup(new_manifests, name));
+
+			current = g_hash_table_lookup(new_manifests, name);
+
+			// Avoid adding duplicate includes to the list
+			if (g_list_find(manifest_includes, current) == NULL) {
+				manifest_includes = g_list_prepend(manifest_includes, current);
+			}
 		}
 		manifest->includes = manifest_includes;
 		manifest_includes = NULL;
 		manifest = g_hash_table_lookup(old_manifests, group);
 		name_includes = manifest->includes;
+
 		while (name_includes) {
 			char *name = name_includes->data;
 			name_includes = g_list_next(name_includes);
-			manifest_includes = g_list_prepend(manifest_includes, g_hash_table_lookup(old_manifests, name));
+
+			current = g_hash_table_lookup(old_manifests, name);
+
+			// Avoid adding duplicate includes to the list
+			if (g_list_find(manifest_includes, current) == NULL) {
+				manifest_includes = g_list_prepend(manifest_includes, current);
+			}
 		}
 		manifest->includes = manifest_includes;
 	}
@@ -471,7 +483,10 @@ int main(int argc, char **argv)
 			newm->version = oldm->version;
 		} else {
 			apply_heuristics(newm);
-#warning missing rename_detection here
+#ifdef RENAMES
+			/* Detect renamed files specifically for this bundle */
+			rename_detection(newm);
+#endif
 			/* Step 6b: otherwise, write out the manifest */
 			old_deleted = remove_old_deleted_files(oldm, newm);
 			sort_manifest_by_version(newm);

src/fullfiles.c

@@ -249,22 +249,20 @@ static GList *get_deduplicated_fullfile_list(struct manifest *manifest)
 	struct file *tmp;
 
 	// presort by hash for easy deduplication
-	manifest->files = g_list_sort(manifest->files, file_sort_hash);
+	list = manifest->files = g_list_sort(manifest->files, file_sort_hash);
 
-	list = g_list_first(manifest->files);
-	while (prev == NULL && list != NULL) {
+	for (; list; list = g_list_next(list)) {
 		tmp = list->data;
-		list = g_list_next(list);
 
 		// find first new file
 		if (tmp->last_change == manifest->version) {
 			prev = tmp;
 			outfiles = g_list_prepend(outfiles, tmp);
+			break;
 		}
 	}
-	while (list) {
+	for (; list; list = g_list_next(list)) {
 		file = list->data;
-		list = g_list_next(list);
 
 		// add any new file having a unique hash
 		//FIXME: rename logic will be needed here

src/groups.c

@@ -37,6 +37,7 @@ static char **groups = NULL;
 static gsize groupcount = 0;
 static unsigned int groupcursor = 0;
 
+#if 0
 char *group_packages(char *group)
 {
 	assert(groupfile != NULL);
@@ -50,6 +51,7 @@ char *group_groups(char *group)
 
 	return g_key_file_get_value(groupfile, group, "groups", NULL);
 }
+#endif
 
 char *group_status(char *group)
 {

src/make_packs.c

@@ -25,7 +25,6 @@
 #include <dirent.h>
 #include <errno.h>
 #include <getopt.h>
-#include <getopt.h>
 #include <glib.h>
 #include <locale.h>
 #include <stdio.h>

src/manifest.c

@@ -51,6 +51,10 @@ int file_sort_hash(gconstpointer a, gconstpointer b)
 	return memcmp(A->hash, B->hash, SWUPD_HASH_LEN - 1);
 }
 
+/* Standard sort compare function which sorts
+ * first on the version where the file changed and then
+ * on the name
+ */
 int file_sort_version(gconstpointer a, gconstpointer b)
 {
 	struct file *A, *B;
@@ -582,6 +586,16 @@ void subtract_manifests(struct manifest *m1, struct manifest *m2)
 			list1 = g_list_next(list1);
 			list2 = g_list_next(list2);
 
+			/* When both files are marked deleted, skip
+			 * subtraction. Preserving the deleted entries in both
+			 * manifests is required for 'swupd update' to know
+			 * when to delete the file, because the m2 bundle may
+			 * be installed with or without the m1 bundle.
+			 */
+			if (file1->is_deleted && file2->is_deleted) {
+				continue;
+			}
+
 			if (file1->is_deleted == file2->is_deleted && file1->is_file == file2->is_file) {
 				m1->files = g_list_delete_link(m1->files, todel);
 				m1->count--;

src/pack.c

@@ -144,7 +144,7 @@ static void prepare_pack(struct packdata *pack)
 
 	match_manifests(manifest, pack->end_manifest);
 
-	link_renames(pack->end_manifest->files, manifest);
+	link_renames(pack->end_manifest->files, pack->to);
 }
 
 static void make_pack_full_files(struct packdata *pack)

src/rename.c

@@ -21,6 +21,9 @@
  *
  */
 
+/* Rename detection and support.
+ */
+
 #define _GNU_SOURCE
 #include <assert.h>
 #include <ctype.h>
@@ -37,8 +40,55 @@
 
 #include <magic.h>
 
-static magic_t mcookie;
+static bool samefiletype(char *t1, char *t2)
+{
+	if (t1 && (t1 == t2) && *t1) {
+		return true;
+	}
+	return false;
+}
+/* For an elf binary, we get the BuildID in it, which
+ * is essentially a hash of the original loaded sections
+ * and hence is unique. This hardly makes for a 'type'
+ */
+static char *getmagic(char *filename)
+{
+	static magic_t mcookie;
+	static GStringChunk *typestore;
+	char *c2;
+	char *c1;
+	if (mcookie == NULL) {
+		mcookie = magic_open(MAGIC_NO_CHECK_COMPRESS);
+		magic_load(mcookie, NULL);
+		typestore = g_string_chunk_new(200);
+	}
 
+	c1 = (char *)magic_file(mcookie, filename);
+	if (!c1) {
+		LOG(NULL, "Cannot find file type", "%s", filename);
+		c1 = "";
+	}
+	c1 = strdup(c1);
+	c2 = strstr(c1, ", BuildID[");
+	if (c2) {
+		*c2 = 0;
+	}
+	c2 = strstr(c1, "not stripped");
+	if (c2) {
+		*c2 = 0;
+	}
+	c2 = strstr(c1, "stripped");
+	if (c2) {
+		*c2 = 0;
+	}
+	c2 = g_string_chunk_insert_const(typestore, c1);
+	free(c1);
+	return c2;
+}
+
+/* Assign a score roughly in the range -100 to 1000 to express how similar
+ * two files are.
+ */
 double rename_score(struct file *old, struct file *new)
 {
 	double score = 0.0;
@@ -50,6 +100,16 @@ double rename_score(struct file *old, struct file *new)
 		score += 400;
 	}
 
+	/* If the files are smaller than about 200 bytes then even a
+	 * single byte change using bsdiff is going to work out as
+	 * bigger than just shipping the new file, so stop if they are
+	 * not the same. No point in running up bsdiff just for the
+	 * sake of it.
+	 */
+	if (new->stat.st_size < BSDIFFSIZE) {
+		return -99.0;
+	}
+
 	/* points for being in the same directory */
 	if (strcmp(old->dirname, new->dirname) == 0) {
 		score += 10;
@@ -86,12 +146,14 @@ double rename_score(struct file *old, struct file *new)
 		in--;
 	}
 
+#if 0
 	/* if both start with /boot/vmlinuz give it a boost; this is a local hack due to vmlinuz being very short */
 	if (strncmp(old->filename, "/boot/vmlinuz", 13) == 0 && strncmp(new->filename, "/boot/vmlinuz", 13) == 0) {
 		score += 80;
 	}
 
 	/* if ELF, points for sharing the same soname to the first dot */
+#endif
 
 	/* negative points for not being within 25%+/-1Kb of the same file size */
 	if (old->stat.st_size > ((new->stat.st_size * 1.25) + 1024)) {
@@ -115,66 +177,40 @@ double rename_score(struct file *old, struct file *new)
 	}
 	/* negative points for not having the same 'file' type */
 
-	if (old->filetype && new->filetype && strcmp(old->filetype, new->filetype) != 0) {
+	if (!samefiletype(old->filetype, new->filetype)) {
 		score -= 60;
 	}
 
 	return score;
 }
 
-static void precompute_file_data(struct manifest *manifest, struct file *file, int old_rename, GList *last_versions_list)
+static void precompute_file_data(int version, const char *component, struct file *file, bool fast)
 {
-	GList *item;
 	char *c1, *c2;
 	char *filename = NULL;
-	int last_change;
-	struct stat buf;
 
+	assert(file);
 	/* fill in the filename-minus-the-numbers field */
 	file->alpha_only_filename = calloc(strlen(file->filename) + 1, sizeof(char));
 
 	c1 = file->filename;
 	c2 = file->alpha_only_filename;
-	while (*c1) {
-		while (*c1 && !isalpha(*c1))
-			c1++;
-		if (!*c1) {
-			break;
-		}
-		if (c2 == NULL) {
-			break;
-		}
-		*c2 = *c1;
-		c1++;
-		c2++;
-	}
-
-	if (manifest) {
-		string_or_die(&filename, "%s/%i/%s/%s", image_dir, manifest->version, manifest->component, file->filename);
-	} else if (old_rename) {
-		item = g_list_first(last_versions_list);
-		while (item) {
-			last_change = GPOINTER_TO_INT(item->data);
-			item = g_list_next(item);
-
-			free(filename);
-			string_or_die(&filename, "%s/%i/full/%s", image_dir, last_change, file->filename);
-			if (!lstat(filename, &buf)) {
-				break;
+	if (c2) {
+		for (; *c1; c1++) {
+			if (isalpha(*c1)) { /* Only copy letters */
+				*c2++ = *c1;
 			}
 		}
-	} else {
-		string_or_die(&filename, "%s/%i/full/%s", image_dir, file->last_change, file->filename);
+		/* alpha_only_filename is NUL terminated by calloc */
 	}
+	string_or_die(&filename, "%s/%i/%s/%s", image_dir, version, component, file->filename);
 
 	/* make sure file->stat.st_size is valid */
 	if (file->stat.st_size == 0) {
 		int ret;
+		struct stat buf;
 
-		if (filename == NULL) {
-			printf("filename is null...impossible to stat\n");
-			assert(0);
-		}
+		assert(filename);
 		ret = lstat(filename, &buf);
 		if (!ret) {
 			file->stat.st_size = buf.st_size;
@@ -182,27 +218,19 @@ static void precompute_file_data(struct manifest *manifest, struct file *file, i
 			printf("Stat failure on %s\n", filename);
 		}
 	}
-
-	c1 = (char *)magic_file(mcookie, filename);
-	if (c1) {
-		char *c2;
-		file->filetype = strdup(c1);
-		c2 = strstr(file->filetype, "not stripped");
-		if (c2) {
-			*c2 = 0;
-		}
-		c2 = strstr(file->filetype, "stripped");
-		if (c2) {
-			*c2 = 0;
-		}
+	if (file->stat.st_size < BSDIFFSIZE) {
+		/* thing is too small, always will be regenerated
+		 * so no point in trying to figure out what kind
+		 * of file it is
+		 */
 	} else {
-		LOG(file, "Cannot find file type", "%s", filename);
+		file->filetype = getmagic(filename);
 	}
 
 	free(filename);
 
-	file->basename = strdup(basename(file->filename));
-	file->dirname = strdup(dirname(file->filename));
+	file->basename = g_path_get_basename(file->filename);
+	file->dirname = g_path_get_dirname(file->filename);
 }
 
 int file_sort_score(gconstpointer a, gconstpointer b)
@@ -222,6 +250,10 @@ int file_sort_score(gconstpointer a, gconstpointer b)
 	return 0;
 }
 
+/* compare file to each deleted file.
+ * set file->rename_peer to best matched deleted file
+ * set file->rename_score to the score
+ */
 static void score_file(GList *deleted_files, struct file *file)
 {
 	GList *list2;
@@ -246,61 +278,107 @@ static void score_file(GList *deleted_files, struct file *file)
 	}
 }
 
-void rename_detection(struct manifest *manifest, int last_change, GList *last_versions_list)
+/* delete the first element of the list and return the new head */
+static GList *del_first(GList *list)
 {
-	GList *new_files = NULL;
-	GList *deleted_files = NULL;
+	/* The first list is the pointer to the list, the second is
+	 * the pointer to what to delete */
+	return g_list_delete_link(list, list);
+}
+
+/* Take a list, return a new list where the filter function returns true */
+static GList *new_filtered_list(GList *list, int version, int (*f)(struct file *file, int version))
+{
+	/* make a list of new files, no peer */
+	GList *newlist = NULL;
+	list = g_list_first(list);
+	for (; list; list = g_list_next(list)) {
+		struct file *file = list->data;
+		if (f(file, version)) {
+			newlist = g_list_prepend(newlist, file);
+		}
+	}
+	return newlist;
+}
+
+static int renamed_file_p(struct file *file, int unused)
+{
+	return file->is_rename;
+}
+/* Return a new list of renamed files */
+static GList *new_list_renamed_files(GList *infiles)
+{
+	return new_filtered_list(infiles, 0, renamed_file_p);
+}
+
+/* Predicate that returns true if this is a new file in the stated version */
+static int new_file_p(struct file *file, int version)
+{
+	if ((file->last_change != version) ||
+	    (file->is_deleted) ||
+	    (!file->is_file) ||
+	    (file->peer)) {
+		return 0;
+	}
+	return 1;
+}
+
+/* return a new list of the new files */
+static GList *list_new_files(struct manifest *manifest)
+{
+	GList *list = new_filtered_list(manifest->files, manifest->version, new_file_p);
+	/* call precompute_file_data for each file on list, return the list */
+	GList *ret = list;
+	for (list = g_list_first(list); list; list = g_list_next(list)) {
+		struct file *file = list->data;
+		precompute_file_data(manifest->version, manifest->component, file, true);
+	}
+	return ret;
+}
+
+static int deleted_p(struct file *file, int version)
+{
+	if ((!file->is_deleted) ||
+	    (!file->peer) ||
+	    (file->last_change != version) ||
+	    (file->peer->is_dir || file->peer->is_link)) {
+		return 0;
+	}
+	return 1;
+}
+
+static GList *list_deleted_files(struct manifest *manifest)
+{
+	GList *list = new_filtered_list(manifest->files, manifest->version, deleted_p);
+	GList *ret = list;
+	/* call precompute_file_data for each peer of file on list */
+	for (list = g_list_first(list); list; list = g_list_next(list)) {
+		struct file *file = list->data;
+		struct file *peer = file->peer;
+		/* Need to get things from the /full/ as we do not know
+		 * which  component may be coming from? */
+		precompute_file_data(peer->last_change, "full", peer, false);
+	}
+	return ret;
+}
+
+void rename_detection(struct manifest *manifest)
+{
+	GList *new_files;
+	GList *deleted_files;
 
 	GList *list;
 	struct file *file;
-	int old_rename = 0;
 
-	if (last_change != manifest->version) {
-		old_rename = 1;
-	}
+	new_files = list_new_files(manifest);
 
-	if (mcookie == NULL) {
-		mcookie = magic_open(MAGIC_NO_CHECK_COMPRESS);
-		magic_load(mcookie, NULL);
-	}
-
-	/* make a list of new files, no peer */
-	list = g_list_first(manifest->files);
-	while (list) {
-		file = list->data;
-		list = g_list_next(list);
-		if ((file->last_change != manifest->version) ||
-		    (file->is_deleted) ||
-		    (!file->is_file) ||
-		    (file->peer)) {
-			continue;
-		}
-
-		new_files = g_list_prepend(new_files, file);
-		precompute_file_data(manifest, file, old_rename, last_versions_list);
-	}
-
-	/* if there are no new files, we're not having any renames -- early exit */
+	/* no new files --> no renames -- early exit */
 	if (!new_files) {
 		LOG(NULL, "No new files, no rename detection", "%s", manifest->component);
 		return;
 	}
-	/* make a list of newly deleted files that have a peer */
 
-	list = g_list_first(manifest->files);
-	while (list) {
-		file = list->data;
-		list = g_list_next(list);
-		if ((!file->is_deleted) ||
-		    (!file->peer) ||
-		    (file->last_change != last_change) ||
-		    (file->peer->is_dir || file->peer->is_link)) {
-			continue;
-		}
-
-		deleted_files = g_list_prepend(deleted_files, file);
-		precompute_file_data(NULL, file->peer, old_rename, last_versions_list);
-	}
+	deleted_files = list_deleted_files(manifest);
 
 	/* nothing got deleted --> no renames --> early exit */
 	if (!deleted_files) {
@@ -309,7 +387,8 @@ void rename_detection(struct manifest *manifest, int last_change, GList *last_ve
 		return;
 	}
 
-	/* for each new file, find the deleted file with the highest score, and store the score */
+	/* for each new file, find the deleted file with the highest score,
+	 * store it in file->rename_peer and store the score */
 	list = g_list_first(new_files);
 	while (list) {
 		file = list->data;
@@ -317,20 +396,24 @@ void rename_detection(struct manifest *manifest, int last_change, GList *last_ve
 		score_file(deleted_files, file);
 	}
 
+redo:
 	/* sort all new files by score */
-
+	/* walk the sorted score list.
+	 * pick the top score,
+	 * check if the score is still valid,
+	 * if not, recompute the score and resort
+	 * This is probably an O(n^3).
+	 */
 	new_files = g_list_sort(new_files, file_sort_score);
 
-	/* pick the top score, check if the score is still valid, if not, recompute the score and resort */
-
-	while (new_files) {
+	for (; new_files; new_files = del_first(new_files)) {
 		file = new_files->data;
+		if (file->rename_peer == NULL) {
+			continue;
+		}
 
-		if (file->rename_score < 15.0 || file->rename_peer == NULL) {
-			new_files = g_list_delete_link(new_files, new_files);
-			if (file->rename_peer) {
-				LOG(NULL, "Rename not done due to insufficient high score", "%s -> %s   score %4.1f", file->rename_peer->filename, file->filename, file->rename_score);
-			}
+		if (file->rename_score < 15.0) {
+			LOG(NULL, "Rename not done due to insufficient high score", "%s -> %s   score %4.1f", file->rename_peer->filename, file->filename, file->rename_score);
 			continue;
 		}
 
@@ -338,10 +421,8 @@ void rename_detection(struct manifest *manifest, int last_change, GList *last_ve
 			/* the candidate peer got already taken by another file! */
 			LOG(NULL, "Rename not done due to target already taken", "%s -> %s   score %4.1f", file->rename_peer->filename, file->filename, file->rename_score);
 			file->rename_peer = NULL;
-			file->rename_score = -100;
 			score_file(deleted_files, file);
-			new_files = g_list_sort(new_files, file_sort_score);
-			continue;
+			goto redo;
 		}
 
 		/* if valid and score is high enough, make the link by setting the flag and storing the hash */
@@ -349,13 +430,14 @@ void rename_detection(struct manifest *manifest, int last_change, GList *last_ve
 		LOG(NULL, "Rename detected!", "%s -> %s   score %4.1f", file->rename_peer->filename, file->filename, file->rename_score);
 
 		file->rename_peer->rename_peer = file;
-		/* must delete the file from the deleted list */
+		/* must remove the file from the deleted list */
 		deleted_files = g_list_remove(deleted_files, file->rename_peer);
 		hash_assign(file->hash, file->rename_peer->hash);
 		file->is_rename = 1;
 		file->rename_peer->is_rename = 1;
-
-		new_files = g_list_delete_link(new_files, new_files);
+		if (!deleted_files) {
+			break; /* No more deleted files to rename */
+		}
 
 	} /* lather, rinse, repeat until all files have a target */
 
@@ -364,65 +446,47 @@ void rename_detection(struct manifest *manifest, int last_change, GList *last_ve
 	g_list_free(deleted_files);
 }
 
-static int file_found_in_older_manifest(struct manifest *from_manifest, struct file *searched_file)
-{
-	GList *list;
-	struct file *file;
-
-	list = g_list_first(from_manifest->files);
-	while (list) {
-		file = list->data;
-		list = g_list_next(list);
-
-		if (file->is_deleted) {
-			continue;
-		}
-		if (!strcmp(file->filename, searched_file->filename)) {
-			return 1;
-		}
-	}
-
-	return 0;
-}
-
-void link_renames(GList *newfiles, struct manifest *from_manifest)
+/* What do we need this for?
+ *
+ * rename_detection has already set up the links in the manifest it
+ * was given.
+ *
+ */
+void link_renames(GList *newfiles, int to_version)
 {
 	GList *list1, *list2;
 	GList *targets;
 	struct file *file1, *file2;
 
-	targets = newfiles = g_list_sort(newfiles, file_sort_version);
+	targets = new_list_renamed_files(newfiles);
+	/* TODO: Check that g_list_sort is reasonable speed */
+	targets = newfiles = g_list_sort(targets, file_sort_version);
 
-	list1 = g_list_first(newfiles);
-
-	/* todo: sort newfiles and targets by hash */
-
-	while (list1) {
+	for (list1 = newfiles; list1; list1 = g_list_next(list1)) {
 		file1 = list1->data;
-		list1 = g_list_next(list1);
 
-		if ((file1->peer || !file1->is_rename) ||
-		    (file1->is_deleted)) {
+		if (file1->peer || file1->is_deleted) {
 			continue;
 		}
-		/* now, file1 is the new file that got renamed. time to search the rename targets */
+		/* now, file1 is the new file that got renamed.
+		 * time to search the rename targets */
 		list2 = g_list_first(targets);
-		while (list2) {
+		for (; list2; list2 = g_list_next(list2)) {
 			file2 = list2->data;
-			list2 = g_list_next(list2);
-
-			if ((!file2->peer || !file2->is_rename) ||
+			/* This is like deleted_p but not quite */
+			/* deleted_p returns false for directories and links */
+			if (!file2->peer ||
 			    (!file2->is_deleted) ||
-			    (!file_found_in_older_manifest(from_manifest, file2))) {
+			    (file2->last_change != to_version)) {
 				continue;
 			}
 			if (hash_compare(file2->hash, file1->hash)) {
 				file1->rename_peer = file2->peer;
 				file1->peer = file2->peer;
 				file2->peer->rename_peer = file1;
-				list2 = NULL;
+				break;
 			}
 		}
 	}
-	free(from_manifest);
+	g_list_free(targets);
 }

src/xattrs.c

@@ -60,8 +60,7 @@ static int xattr_get_value(const char *path, const char *name, char **blob,
 
 	/* realloc needed len + 1 in case we need to add final zero
 	 * to ensure consistent blob */
-	value = realloc(*blob, *blob_len + len +
-				   (action == XATTRS_ACTION_GET_BLOB ? 1 : 0));
+	value = realloc(*blob, *blob_len + len + (action == XATTRS_ACTION_GET_BLOB ? 1 : 0));
 	assert(value);
 
 	*blob = value;

test/functional/includes-deduplicate/test.bats

@@ -0,0 +1,42 @@
+#!/usr/bin/env bats
+
+# common functions
+load "../swupdlib"
+
+setup() {
+  clean_test_dir
+  init_test_dir
+
+  init_server_ini
+  set_latest_ver 0
+  init_groups_ini os-core test-bundle1 test-bundle2
+
+  set_os_release 10 os-core
+  track_bundle 10 os-core
+  track_bundle 10 test-bundle1
+  track_bundle 10 test-bundle2
+
+  set_os_release 20 os-core
+  track_bundle 20 os-core
+  track_bundle 20 test-bundle1
+  track_bundle 20 test-bundle2
+
+  gen_file_plain 10 os-core test0
+  gen_file_plain 10 test-bundle1 test1
+  gen_file_plain 10 test-bundle2 test2
+
+  gen_includes_file test-bundle2 10 test-bundle1 test-bundle1
+  gen_includes_file test-bundle2 20 test-bundle1 test-bundle1
+}
+
+@test "deduplicate bundle includes" {
+  sudo $CREATE_UPDATE --osversion 10 --statedir $DIR --format 3
+  set_latest_ver 10
+  sudo $CREATE_UPDATE --osversion 20 --statedir $DIR --format 3
+
+  # includes list should be deduplicated in both the old and new manifests
+  [[ 1 -eq $(grep '^includes:	test-bundle1$' $DIR/www/10/Manifest.test-bundle2 | wc -l) ]]
+  [[ 1 -eq $(grep '^includes:	test-bundle1$' $DIR/www/20/Manifest.test-bundle2 | wc -l) ]]
+}
+
+# vi: ft=sh ts=8 sw=2 sts=2 et tw=80

test/functional/renames/job

@@ -0,0 +1,13 @@
+#!/bin/bash
+
+export BATS_TEST_DIRNAME=$PWD
+export TIMEFORMAT
+TIMEFORMAT="time %R %P%% (%U+%S)"
+source ../swupdlib.bash
+exec 3< rundata
+
+while read -r -u 3 
+do
+    read -i "$REPLY" -e CMD
+    eval "set -x ; time ( $CMD ) ; set +x "
+done

test/functional/renames/rundata

@@ -0,0 +1,22 @@
+sudo rm -r logs/ web-dir/
+tar xf ~/data/test.tar 
+init_test_dir
+init_server_ini
+set_latest-ver 0
+init_groups_ini os-core test-bundle
+set_os_release 10 os-core
+track_bundle 10 os-core
+track_bundle 10 test-bundle
+set_os_release 20 os-core
+track_bundle 20 os-core
+track_bundle 20 test-bundle
+sudo $CREATE_UPDATE --osversion 10 --statedir $DIR --format 3
+sudo $MAKE_FULLFILES --statedir $DIR 10
+sudo $MAKE_PACK --statedir $DIR 0 10 os-core
+sudo $MAKE_PACK --statedir $DIR 0 10 test-bundle
+time  sudo $MAKE_PACK --statedir $DIR 0 10 test-bundle
+set_latest-ver 10
+sudo $CREATE_UPDATE --osversion 20 --statedir $DIR --format 3
+sudo $MAKE_FULLFILES --statedir $DIR 20
+sudo $MAKE_PACK --statedir $DIR  10 20  os-core
+sudo $MAKE_PACK --statedir $DIR  10 20  test-bundle

test/functional/renames/test.bats

@@ -0,0 +1,246 @@
+#!/usr/bin/env bats
+
+# common functions
+load "../swupdlib"
+
+setup() {
+  maybeskip
+  clean_test_dir
+  init_test_dir
+
+  init_server_ini
+  set_latest_ver 0
+  init_groups_ini os-core test-bundle
+
+  set_os_release 10 os-core
+  track_bundle 10 os-core
+  track_bundle 10 test-bundle
+  set_os_release 20 os-core
+  track_bundle 20 os-core
+  track_bundle 20 test-bundle
+}
+
+# Generate data files of 3 types in test-bundle
+# All files are big enough that they get rename detection
+gendataA() {
+  gen_file_plain_with_content "$1" test-bundle "$2" "$(seq 100)"
+}
+gendataB() {
+  gen_file_plain_with_content "$1" test-bundle "$2" "$(seq 50) $(seq 52 101)"
+}
+gendataC() {
+  # cache string
+  [ -z "$dataC" ] || dataC="$(seq 1000 | gzip | uuencode wombat)"
+  gen_file_plain_with_content "$1" test-bundle "$2" "$dataC"
+}
+# Generate small data files
+gendataAs() {
+  gen_file_plain_with_content "$1" test-bundle "$2" "$(seq 50)"
+}
+gendataBs() {
+  gen_file_plain_with_content "$1" test-bundle "$2" "$(seq 24) $(seq 26 49)"
+}
+gendataCs() {
+  # cache string
+  [ -z "$dataCs" ] || dataC="$(seq 50 | gzip | uuencode wombat)"
+  gen_file_plain_with_content "$1" test-bundle "$2" "$dataCs"
+}
+
+checkrenamed(){
+    local flags sh1 ver name fromsha1="bad" tosha1
+    # Check that $1 is renamed to $2
+    exec 9< $DIR/www/20/Manifest.test-bundle
+    # skip the header
+    while read -u9
+    do
+	[ -z "$REPLY" ] && break
+    done
+    while read -r -u9 flags sha1 ver name
+    do
+	case "$flags" in
+	    (?"dr"?) [ "$name" = "$1" ] && fromsha1=$sha1 ;;
+	    (?".r"?) [ "$name" = "$2" ] && tosha1=$sha1 ;;
+	esac
+    done
+    if [ "$fromsh1" = "$tosha1" ] ; then return 0 ; else return 1 ; fi
+}
+
+# Guts of doing an update
+do_an_update() {
+  sudo $CREATE_UPDATE --osversion 10 --statedir $DIR --format 3
+  sudo $MAKE_FULLFILES --statedir $DIR 10
+  sudo $MAKE_PACK --statedir $DIR 0 10 os-core
+  sudo $MAKE_PACK --statedir $DIR 0 10 test-bundle
+
+  set_latest_ver 10
+
+  sudo $CREATE_UPDATE --osversion 20 --statedir $DIR --format 3
+  sudo $MAKE_FULLFILES --statedir $DIR 20
+  sudo $MAKE_PACK --statedir $DIR 0 20 os-core
+  sudo $MAKE_PACK --statedir $DIR 0 20 test-bundle
+  sudo $MAKE_PACK --statedir $DIR 10 20 os-core
+  sudo $MAKE_PACK --statedir $DIR 10 20 test-bundle
+}
+
+
+@test "basic rename detection support" {
+  gendataA 10 foo
+  gendataA 20 bar
+  do_an_update
+  checkrenamed foo bar
+}
+
+@test "ignore rename detection for small files" {
+  gendataAs 10 foo
+  gendataAs 20 bar
+  do_an_update
+  # A renamed file comprises a new file and a deleted file
+  [[ 0 -eq $(grep '^F\.\.r.*/bar$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+  [[ 0 -eq $(grep '^\.d\.r.*/foo$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+}
+@test "ignore rename detection for large to small files" {
+  gendataA 10 foo
+  gendataAs 20 bar
+  do_an_update
+  # A renamed file comprises a new file and a deleted file
+  [[ 0 -eq $(grep '^F\.\.r.*/bar$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+  [[ 0 -eq $(grep '^\.d\.r.*/foo$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+}
+@test "ignore rename detection for small to large files" {
+  gendataAs 10 foo
+  gendataA 20 bar
+  do_an_update
+  # A renamed file comprises a new file and a deleted file
+  [[ 0 -eq $(grep '^F\.\.r.*/bar$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+  [[ 0 -eq $(grep '^\.d\.r.*/foo$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+}
+
+@test "rename one file to two" {
+  gendataA 10 foo
+  gendataA 20 bar
+  gendataA 20 baz
+  do_an_update  
+
+  # A renamed file comprises a new file and a deleted file
+  [[ 1 -eq $(grep '^F\.\.r.*/ba[rz]$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+  [[ 1 -eq $(grep '^\.d\.r.*/foo$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+}
+@test "rename two file to one" {
+  gendataA 10 foo
+  gendataA 10 foz
+  gendataA 20 baz
+  do_an_update  
+
+  # A renamed file comprises a new file and a deleted file
+  [[ 1 -eq $(grep '^F\.\.r.*/baz$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+  [[ 1 -eq $(grep '^\.d\.\..*/fo[oz]$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+  [[ 1 -eq $(grep '^\.d\.r.*/fo[oz]$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+}
+@test "rename two files to two" {
+  gendataA 10 foo
+  gendataA 10 foz
+  gendataA 20 bar
+  gendataA 20 baz  
+  do_an_update
+  checkrenamed foo bar
+  checkrenamed foz baz
+}
+
+@test "rename two files to two, one slightly different" {
+  gendataA 10 foo
+  gendataA 10 foz
+  gendataA 20 bar
+  gendataB 20 baz  
+  do_an_update
+  # we don't actually know how the client we do this rename, but don't care
+  checkrenamed foo bar
+  checkrenamed foz baz
+}
+
+@test "rename two files to two, each pair slightly different" {
+  gendataA 10 foo
+  gendataB 10 foz
+  gendataA 20 bar
+  gendataB 20 baz
+  do_an_update
+  checkrenamed foo bar
+  checkrenamed foz baz
+}
+
+@test "rename two files to two, one very different" {
+  gendataA 10 foo
+  gendataA 10 foz
+  gendataA 20 bar
+  gendataC 20 baz  
+  do_an_update
+  # A renamed file comprises a new file and a deleted file
+  [[ 1 -eq $(grep '^F\.\.r.*/ba[rz]$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+  [[ 1 -eq $(grep '^\.d\.r.*/fo[oz]$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+}
+@test "rename two files to two, one small" {
+  gendataA 10 foo
+  gendataA 10 foz
+  gendataA 20 bar
+  gendataCs 20 baz  
+  do_an_update
+  run checkrenamed foo bar
+  if [ $status -eq 1 ] ; then
+    checkrenamed foz bar
+  fi
+}
+
+@test "directory name changes" {
+  gendataA 10 dir1/foo
+  gendataA 20 dir2/foo
+  do_an_update
+  checkrenamed /dir1/foo /dir2/foo
+}
+
+@test "directory name changes small files" {
+  gendataAs 10 dir1/foo
+  gendataAs 20 dir2/foo
+  do_an_update
+  if ! checkrenamed /dir1/foo /dir2/foo ; then false ; fi
+}
+
+@test "directory name and small data changes" {
+  gendataA 10 dir1/foo
+  gendataB 20 dir2/foz
+  do_an_update
+  checkrenamed /dir1/foo /dir2/foz
+}
+
+@test "directory name and small data changes, choose same name" {
+  gendataA 10 dir1/foo
+  gendataA 10 dir1/foz
+  gendataB 20 dir2/foz
+  do_an_update
+  checkrenamed /dir1/foz /dir2/foz
+}
+
+@test "same basename test" {
+    gendataA 10 dir1/foo.so.1
+    gendataA 10 dir1/foz.so.1
+    gendataB 20 dir2/foo.so.2
+    do_an_update
+    checkrenamed /dir1/foo.so.1 /dir2/foo.so.2
+}
+
+@test "rename file to dir/file" {
+    gendataA 10 foo
+    gendataA 20 foo/bar
+    do_an_update
+    checkrenamed /foo /foo/bar
+}
+# @test "rename foo/foo to foo" {
+#     gendataA 10 foo/foo
+#     gendataA 20 foo
+#     do_an_update
+#     checkrenamed /foo/foo /foo
+# }
+
+# Emacs and vi support
+# Local variables:
+# sh-indentation: 2
+# End:
+# vi: ft=sh ts=8 sw=2 sts=2 et tw=80

test/functional/renames2/job

@@ -0,0 +1,13 @@
+#!/bin/bash
+
+export BATS_TEST_DIRNAME=$PWD
+export TIMEFORMAT
+TIMEFORMAT="time %R %P%% (%U+%S)"
+source ../swupdlib.bash
+exec 3< rundata
+
+while read -r -u 3 
+do
+    read -i "$REPLY" -e CMD
+    eval "set -x ; time ( $CMD ) ; set +x "
+done

test/functional/renames2/rundata

@@ -0,0 +1,22 @@
+sudo rm -r logs/ web-dir/
+tar xf ~/data/test.tar 
+init_test_dir
+init_server_ini
+set_latest_ver 0
+init_groups_ini os-core test-bundle
+set_os_release 10 os-core
+track_bundle 10 os-core
+track_bundle 10 test-bundle
+set_os_release 20 os-core
+track_bundle 20 os-core
+track_bundle 20 test-bundle
+sudo $CREATE_UPDATE --osversion 10 --statedir $DIR --format 3
+sudo $MAKE_FULLFILES --statedir $DIR 10
+sudo $MAKE_PACK --statedir $DIR 0 10 os-core
+sudo $MAKE_PACK --statedir $DIR 0 10 test-bundle
+time  sudo $MAKE_PACK --statedir $DIR 0 10 test-bundle
+set_latest_ver 10
+sudo $CREATE_UPDATE --osversion 20 --statedir $DIR --format 3
+sudo $MAKE_FULLFILES --statedir $DIR 20
+sudo $MAKE_PACK --statedir $DIR  10 20  os-core
+sudo $MAKE_PACK --statedir $DIR  10 20  test-bundle

test/functional/renames2/test.bats

@@ -0,0 +1,144 @@
+#!/usr/bin/env bats
+
+# common functions
+load "../swupdlib"
+
+setup() {
+  clean_test_dir
+  init_test_dir
+  tar x -C $DIR/.. -f ~/data/test.tar 
+  mv $DIR/image/20/test-bundle/usr/share/bash-completion/completions{,.old}
+
+  init_server_ini
+  set_latest_ver 0
+  init_groups_ini os-core test-bundle
+
+  set_os_release 10 os-core
+  track_bundle 10 os-core
+  track_bundle 10 test-bundle
+  set_os_release 20 os-core
+  track_bundle 20 os-core
+  track_bundle 20 test-bundle
+
+  # Files have different names ("foo" vs "bar"), but have the same content
+  gen_file_plain_with_content 10 test-bundle "foo" "data"
+  gen_file_plain_with_content 20 test-bundle "bar" "data"
+}
+
+@test "rename detection support" {
+  sudo $CREATE_UPDATE --osversion 10 --statedir $DIR --format 3
+  sudo $MAKE_FULLFILES --statedir $DIR 10
+  sudo $MAKE_PACK --statedir $DIR 0 10 os-core
+  sudo $MAKE_PACK --statedir $DIR 0 10 test-bundle
+
+  set_latest_ver 10
+
+  sudo $CREATE_UPDATE --osversion 20 --statedir $DIR --format 3
+  sudo $MAKE_FULLFILES --statedir $DIR 20
+  sudo $MAKE_PACK --statedir $DIR 0 20 os-core
+  sudo $MAKE_PACK --statedir $DIR 0 20 test-bundle
+  sudo $MAKE_PACK --statedir $DIR 10 20 os-core
+  sudo $MAKE_PACK --statedir $DIR 10 20 test-bundle
+
+  # A renamed file comprises a new file and a deleted file
+  [[ 1 -eq $(grep '^F\.\.r.*/bar$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+  [[ 1 -eq $(grep '^\.d\.r.*/foo$' $DIR/www/20/Manifest.test-bundle | wc -l) ]]
+}
+
+# @test "rename detection support 1 to 2" {
+#   sudo $CREATE_UPDATE --osversion 10 --statedir $DIR --format 3
+#   sudo $MAKE_FULLFILES --statedir $DIR 10
+#   sudo $MAKE_PACK --statedir $DIR 0 10 os-core
+#   sudo $MAKE_PACK --statedir $DIR 0 10 test-bundle
+
+#   set_latest_ver 10
+
+#   gen_file_plain_with_content 20 test-bundle "baz" "data"
+#   sudo $CREATE_UPDATE --osversion 20 --statedir $DIR --format 3
+#   sudo $MAKE_FULLFILES --statedir $DIR 20
+#   sudo $MAKE_PACK --statedir $DIR 0 20 os-core
+#   sudo $MAKE_PACK --statedir $DIR 0 20 test-bundle
+#   sudo $MAKE_PACK --statedir $DIR 10 20 os-core
+#   sudo $MAKE_PACK --statedir $DIR 10 20 test-bundle
+
+#   # A renamed file comprises a new file and a deleted file.
+#   # Should be one file that is not renamed
+#   # Need to figure out what happens to hard links
+#   [[ 1 -eq $(grep -c '^F\.\.r.*/ba[rz]$' $DIR/www/20/Manifest.test-bundle) ]]
+#   [[ 1 -eq $(grep -c '^F\.\.\..*/ba[rz]$' $DIR/www/20/Manifest.test-bundle) ]]
+#   [[ 1 -eq $(grep -c '^\.d\.r.*/foo$' $DIR/www/20/Manifest.test-bundle) ]]
+# }
+
+# @test "rename detection support 2 to 2" {
+#   gen_file_plain_with_content 10 test-bundle "fee" "data"
+#   sudo $CREATE_UPDATE --osversion 10 --statedir $DIR --format 3
+#   sudo $MAKE_FULLFILES --statedir $DIR 10
+#   sudo $MAKE_PACK --statedir $DIR 0 10 os-core
+#   sudo $MAKE_PACK --statedir $DIR 0 10 test-bundle
+
+#   set_latest_ver 10
+
+#   gen_file_plain_with_content 20 test-bundle "foo~" "data"
+#   sudo $CREATE_UPDATE --osversion 20 --statedir $DIR --format 3
+#   sudo $MAKE_FULLFILES --statedir $DIR 20
+#   sudo $MAKE_PACK --statedir $DIR 0 20 os-core
+#   sudo $MAKE_PACK --statedir $DIR 0 20 test-bundle
+#   sudo $MAKE_PACK --statedir $DIR 10 20 os-core
+#   sudo $MAKE_PACK --statedir $DIR 10 20 test-bundle
+
+#   # A renamed file comprises a new file and a deleted file.
+#   # Should be one file that is not renamed
+#   # Need to figure out what happens to hard links
+#   [[ 1 -eq $(grep -c '^F\.\.r.*/bar$' $DIR/www/20/Manifest.test-bundle) ]]
+#   [[ 1 -eq $(grep -c '^F\.\.r.*/foo~$' $DIR/www/20/Manifest.test-bundle) ]]
+#   [[ 2 -eq $(grep -c '^\.d\.r.*/f..$' $DIR/www/20/Manifest.test-bundle) ]]
+# }
+
+# @test "rename detection support binary files" {
+#   sudo mkdir -p  $DIR/image/10/test-bundle/bin/
+#   sudo cp /bin/gcc-ar $DIR/image/10/test-bundle/bin/gcc-ar.1
+#   sudo cp /bin/gcc-ar $DIR/image/20/test-bundle/gcc-ar.1
+#   strip $DIR/image/20/test-bundle/gcc-ar.1
+#   sudo $CREATE_UPDATE --osversion 10 --statedir $DIR --format 3
+#   sudo $MAKE_FULLFILES --statedir $DIR 10
+#   sudo $MAKE_PACK --statedir $DIR 0 10 os-core
+#   sudo $MAKE_PACK --statedir $DIR 0 10 test-bundle
+
+#   set_latest_ver 10
+
+#   gen_file_plain_with_content 20 test-bundle "foo~" "And now not the same"
+#   sudo $CREATE_UPDATE --osversion 20 --statedir $DIR --format 3
+#   sudo $MAKE_FULLFILES --statedir $DIR 20
+#   sudo $MAKE_PACK --statedir $DIR 0 20 os-core
+#   sudo $MAKE_PACK --statedir $DIR 0 20 test-bundle
+#   sudo $MAKE_PACK --statedir $DIR 10 20 os-core
+#   sudo $MAKE_PACK --statedir $DIR 10 20 test-bundle
+
+#   # A renamed file comprises a new file and a deleted file.
+#   # Should be one file that is not renamed
+#   # Need to figure out what happens to hard links
+# }
+
+# @test "rename detection support binary files 2" {
+#   sudo $CREATE_UPDATE --osversion 10 --statedir $DIR --format 3
+#   sudo $MAKE_FULLFILES --statedir $DIR 10
+#   sudo $MAKE_PACK --statedir $DIR 0 10 os-core
+#   sudo $MAKE_PACK --statedir $DIR 0 10 test-bundle
+
+#   set_latest_ver 10
+
+#   gen_file_plain_with_content 20 test-bundle "foo" "And now not the same"
+#   gen_file_plain_with_content 20 test-bundle "foo.1" "data"
+#   sudo $CREATE_UPDATE --osversion 20 --statedir $DIR --format 3
+#   sudo $MAKE_FULLFILES --statedir $DIR 20
+#   sudo $MAKE_PACK --statedir $DIR 0 20 os-core
+#   sudo $MAKE_PACK --statedir $DIR 0 20 test-bundle
+#   sudo $MAKE_PACK --statedir $DIR 10 20 os-core
+#   sudo $MAKE_PACK --statedir $DIR 10 20 test-bundle
+
+#   # A renamed file comprises a new file and a deleted file.
+#   # Should be one file that is not renamed
+#   # Need to figure out what happens to hard links
+# }
+
+# # vi: ft=sh ts=8 sw=2 sts=2 et tw=80

test/functional/subtract-delete/test.bats

@@ -0,0 +1,56 @@
+#!/usr/bin/env bats
+
+# common functions
+load "../swupdlib"
+
+setup() {
+  clean_test_dir
+  init_test_dir
+
+  init_server_ini
+  set_latest_ver 0
+  init_groups_ini os-core test-bundle
+
+  # start with both bundles containing a file "foo"
+  set_os_release 10 os-core
+  track_bundle 10 os-core
+  track_bundle 10 test-bundle
+  gen_file_plain 10 os-core foo
+  gen_file_plain 10 test-bundle foo
+
+  # delete "foo" from os-core (the included bundle)
+  set_os_release 20 os-core
+  track_bundle 20 os-core
+  track_bundle 20 test-bundle
+  gen_file_plain 20 test-bundle foo
+
+  # delete "foo" from test-bundle
+  set_os_release 30 os-core
+  track_bundle 30 os-core
+  track_bundle 30 test-bundle
+
+  # make modification (add new file) to test-bundle
+  set_os_release 40 os-core
+  track_bundle 40 os-core
+  track_bundle 40 test-bundle
+  gen_file_plain 40 test-bundle foobar
+}
+
+@test "no subtraction for two deleted files" {
+  sudo $CREATE_UPDATE --osversion 10 --statedir $DIR --format 3
+  set_latest_ver 10
+  sudo $CREATE_UPDATE --osversion 20 --statedir $DIR --format 3
+  set_latest_ver 20
+  sudo $CREATE_UPDATE --osversion 30 --statedir $DIR --format 3
+  set_latest_ver 30
+  sudo $CREATE_UPDATE --osversion 40 --statedir $DIR --format 3
+  set_latest_ver 40
+
+  # If the file is absent in test-bundle, this means it was subtracted. If
+  # present, subtraction was not performed.
+  hash1=$(hash_for 40 test-bundle "/foo")
+  [ -n "$hash1" ]
+}
+
+
+# vi: ft=sh ts=8 sw=2 sts=2 et tw=80

test/functional/swupdlib.bash

@@ -40,6 +40,11 @@ EOF
   done
 }
 
+# If the variable RUN_JUST_ONE is set then only run that test
+maybeskip() {
+    [ -z "$RUN_JUST_ONE" ] || [ "$RUN_JUST_ONE" -eq "$BATS_TEST_NUMBER" ] || skip
+}
+
 set_os_release() {
   local ver=$1
   local bundle=$2
@@ -92,7 +97,10 @@ gen_file_plain() {
   local name="$3"
 
   # Add plain text file into a bundle
-  mkdir -p $DIR/image/$ver/$bundle/$(dirname "$name")
+  case "$name" in
+      (*"/"*)	mkdir -p "$DIR/image/$ver/$bundle/${name%/*}" ;;
+      (*)	mkdir -p $DIR/image/$ver/$bundle ;;
+  esac
   echo "$name" > $DIR/image/$ver/$bundle/"$name"
 }
 
@@ -102,7 +110,10 @@ gen_file_plain_change() {
   local name="$3"
 
   # Add plain text file into a bundle
-  mkdir -p $DIR/image/$ver/$bundle/$(dirname "$name")
+  case "$name" in
+      (*"/"*)	mkdir -p "$DIR/image/$ver/$bundle/${name%/*}" ;;
+      (*)	mkdir -p $DIR/image/$ver/$bundle ;;
+  esac
   echo "$ver $name" > $DIR/image/$ver/$bundle/"$name"
 }
 
@@ -137,4 +148,16 @@ hash_for() {
   awk -F'\t' -v NAME="$name" 'NF == 4 && $4 == NAME { print $2 }' $DIR/www/$ver/Manifest.$bundle
 }
 
+gen_file_plain_with_content() {
+  local ver=$1
+  local bundle=$2
+  local name="$3"
+  local content="$4"
+  case "$name" in
+      (*"/"*)	mkdir -p "$DIR/image/$ver/$bundle/${name%/*}" ;;
+      (*)	mkdir -p $DIR/image/$ver/$bundle ;;
+  esac
+  echo "$content" > $DIR/image/$ver/$bundle/"$name"
+}
+
 # vi: ft=sh ts=8 sw=2 sts=2 et tw=80