Release v3.3.4

This release fixes subtracting files from manifests when both versions are marked deleted. Server subtracting of files when both are deleted
proves problematic for client updates, because the version at which the
files were deleted will differ, and the client uses the versions to
determine when it should delete files for an update.

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

Makefile.am

@@ -109,14 +109,16 @@ TESTS = $(dist_check_SCRIPTS)
 dist_check_SCRIPTS = \
 	test/functional/basic/test.bats \
 	test/functional/delete-no-version-bump/test.bats \
-	test/functional/include-version-bump/test.bats \
-	test/functional/update/test.bats \
-	test/functional/fullfiles/test.bats \
-	test/functional/pack/test.bats \
-	test/functional/full-run/test.bats \
-	test/functional/full-run-delta/test.bats \
 	test/functional/file-name-blacklisted/test.bats \
-	test/functional/state-file/test.bats
+	test/functional/full-run-delta/test.bats \
+	test/functional/full-run/test.bats \
+	test/functional/fullfiles/test.bats \
+	test/functional/include-version-bump/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
 endif
 
 if COVERAGE
@@ -134,6 +136,27 @@ endif
 distclean-local:
 	rm -rf aclocal.m4 autogen.sh autom4te.cache config.h.in config.h.in~ configure depcomp install-sh Makefile.in missing compile
 
+compliant:
+	@git diff --quiet --exit-code include src; ret=$$?; \
+	if [ $$ret -eq 1 ]; then \
+		echo "Error: can only check code style when include/ and src/ are clean."; \
+		echo "Stash or commit your changes and try again."; \
+		exit $$ret; \
+	elif [ $$ret -gt 1 ]; then \
+		exit $$ret; \
+	fi; \
+	clang-format -i -style=file include/*.h src/*.c; ret=$$?; \
+	if [ $$ret -ne 0 ]; then \
+		exit $$ret; \
+	fi; \
+	git diff --quiet --exit-code include src; ret=$$?; \
+	if [ $$ret -eq 1 ]; then \
+		echo "Code style issues found. Run 'git diff' to view issues."; \
+	elif [ $$ret -eq 0 ]; then \
+		echo "No code style issues found."; \
+	fi; \
+	exit $$ret
+
 release:
 	@git rev-parse v$(PACKAGE_VERSION) &> /dev/null; \
 	if [ "$$?" -eq 0 ]; then \

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.2, tudor.marcu@intel.com)
+AC_INIT(swupd-server, 3.3.4, tudor.marcu@intel.com)
 AM_INIT_AUTOMAKE([foreign -Wall -W subdir-objects])
 AM_SILENT_RULES([yes])
 AC_PROG_CC

src/analyze_fs.c

@@ -302,10 +302,10 @@ static bool illegal_characters(const char *filename)
 }
 
 static struct file *add_file(struct manifest *manifest,
-                             const char *entry_name,
-                             char *sub_filename,
-                             char *fullname,
-                             bool do_hash)
+			     const char *entry_name,
+			     char *sub_filename,
+			     char *fullname,
+			     bool do_hash)
 {
 	GError *err = NULL;
 	struct file *file;
@@ -334,7 +334,6 @@ static struct file *add_file(struct manifest *manifest,
 		assert(0);
 	}
 
-
 	/* if for some reason there is a file in the official build
 	 * which should not be included in the Manifest, then open a bug
 	 * to get it removed, and work around its presence by
@@ -356,10 +355,9 @@ static struct file *add_file(struct manifest *manifest,
 	}
 	manifest->files = g_list_prepend(manifest->files, file);
 	manifest->count++;
-        return file;
+	return file;
 }
 
-
 static void iterate_directory(struct manifest *manifest, char *pathprefix,
 			      char *subpath, bool do_hash)
 {
@@ -445,7 +443,7 @@ static void iterate_directory(struct manifest *manifest, char *pathprefix,
 	while (dir) {
 		char *sub_filename;
 		char *fullname;
-                struct file *file;
+		struct file *file;
 
 		entry = readdir(dir);
 		if (!entry) {
@@ -463,8 +461,8 @@ static void iterate_directory(struct manifest *manifest, char *pathprefix,
 		/* takes ownership of the strings, so we don't need to free it */
 		file = add_file(manifest, entry->d_name, sub_filename, fullname, do_hash);
 
-                if (file && file->is_dir) {
-                    iterate_directory(manifest, pathprefix, file->filename, do_hash);
+		if (file && file->is_dir) {
+			iterate_directory(manifest, pathprefix, file->filename, do_hash);
 		}
 	}
 	closedir(dir);

src/chroot.c

@@ -39,7 +39,7 @@ void chroot_create_full(int newversion)
 	char *full_dir;
 
 	string_or_die(&full_dir, "%s/%i/full/", image_dir, newversion);
-	if (!access(full_dir, R_OK|X_OK)) {
+	if (!access(full_dir, R_OK | X_OK)) {
 		free(full_dir);
 		return;
 	}

src/delta.c

@@ -40,8 +40,8 @@ void __create_delta(struct file *file, int from_version, char *from_hash)
 	char *original, *newfile, *outfile, *dotfile, *testnewfile, *conf;
 	int ret;
 
-	if (file->is_link) {
-		return;
+	if (!file->is_file || !file->peer->is_file) {
+		return; /* only support deltas between two regular files right now */
 	}
 
 	if (file->is_deleted) {

src/manifest.c

@@ -582,6 +582,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--;

test/functional/full-run-delta/test.bats

@@ -21,7 +21,7 @@ setup() {
   track_bundle 20 included-two
   track_bundle 20 included-nested
 
-  gen_file_to_delta 10 4096 20 4 test-bundle
+  gen_file_to_delta 10 4096 20 4 test-bundle randomfile
 
   gen_file_plain 10 test-bundle foo
   gen_file_plain 10 test-bundle foobarbaz

test/functional/no-delta/test.bats

@@ -0,0 +1,73 @@
+#!/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
+
+  set_os_release 10 os-core
+  track_bundle 10 os-core
+  set_os_release 20 os-core
+  track_bundle 20 os-core
+
+  # symlink -> regular file type change (L -> F)
+  gen_file_to_delta 10 4096 20 4 os-core testfile1
+  gen_symlink_to_file 10 os-core testsym1 testfile1
+  copy_file 10 os-core testfile1 20 os-core testsym1
+
+  # regular file -> symlink type change (F -> L)
+  gen_file_to_delta 10 4096 20 4 os-core testfile2
+  copy_file 10 os-core testfile2 10 os-core testsym2
+  gen_symlink_to_file 20 os-core testsym2 testfile2
+
+  # symlink change + symlink target change; delta should be created for
+  # testfile3, but not for the dereferenced testsym3
+  gen_file_to_delta 10 4096 20 4 os-core testfile3
+  copy_file 20 os-core testfile3 20 os-core testfile4
+  gen_symlink_to_file 10 os-core testsym3 testfile3
+  gen_symlink_to_file 20 os-core testsym3 testfile4
+}
+
+@test "no deltas created for type changes or dereferenced symlinks" {
+  sudo $CREATE_UPDATE --osversion 10 --statedir $DIR --format 3
+  set_latest_ver 10
+  sudo $CREATE_UPDATE --osversion 20 --statedir $DIR --format 3
+
+  # attempt to create some deltas
+  sudo $MAKE_PACK --statedir $DIR 10 20 os-core
+
+  # F -> F deltas should exist
+  hash1=$(hash_for 10 os-core "/testfile1")
+  hash2=$(hash_for 20 os-core "/testfile1")
+  [ -f $DIR/www/20/delta/10-20-$hash1-$hash2 ]
+
+  hash1=$(hash_for 10 os-core "/testfile2")
+  hash2=$(hash_for 20 os-core "/testfile2")
+  [ -f $DIR/www/20/delta/10-20-$hash1-$hash2 ]
+
+  hash1=$(hash_for 10 os-core "/testfile3")
+  hash2=$(hash_for 20 os-core "/testfile3")
+  [ -f $DIR/www/20/delta/10-20-$hash1-$hash2 ]
+
+  # deltas for symlink type changes should not be created
+  hash1=$(hash_for 10 os-core "/testsym1")
+  hash2=$(hash_for 20 os-core "/testsym1")
+  [ ! -f $DIR/www/20/delta/10-20-$hash1-$hash2 ]
+
+  hash1=$(hash_for 10 os-core "/testsym2")
+  hash2=$(hash_for 20 os-core "/testsym2")
+  [ ! -f $DIR/www/20/delta/10-20-$hash1-$hash2 ]
+
+  hash1=$(hash_for 10 os-core "/testsym3")
+  hash2=$(hash_for 20 os-core "/testsym3")
+  [ ! -f $DIR/www/20/delta/10-20-$hash1-$hash2 ]
+}
+
+
+# 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

@@ -72,16 +72,17 @@ gen_file_to_delta() {
   local newver=$3
   local newbytes=$4
   local bundle=$5
+  local name=$6
 
   # create some random data for the original version
   mkdir -p $DIR/image/$origver/$bundle
-  dd if=/dev/urandom of=$DIR/image/$origver/$bundle/randomfile bs=1 count=$origsize
+  dd if=/dev/urandom of=$DIR/image/$origver/$bundle/$name bs=1 count=$origsize
 
   # append more random data to the end of the file in the new version
   TMP=$(mktemp foo.XXXXXX)
   mkdir -p $DIR/image/$newver/$bundle
   dd if=/dev/urandom of=$TMP bs=1 count=$newbytes
-  cat $DIR/image/$origver/$bundle/randomfile $TMP > $DIR/image/$newver/$bundle/randomfile
+  cat $DIR/image/$origver/$bundle/$name $TMP > $DIR/image/$newver/$bundle/$name
   rm $TMP
 }
 
@@ -105,4 +106,35 @@ gen_file_plain_change() {
   echo "$ver $name" > $DIR/image/$ver/$bundle/"$name"
 }
 
+gen_symlink_to_file() {
+  local ver=$1
+  local bundle=$2
+  local symname="$3"
+  local filename="$4"
+
+  mkdir -p $DIR/image/$ver/$bundle/$(dirname "$symname")
+  ln -s "$filename" $DIR/image/$ver/$bundle/"$symname"
+}
+
+copy_file() {
+  local origver=$1
+  local origbundle=$2
+  local origname="$3"
+  local newver=$4
+  local newbundle=$5
+  local newname="$6"
+
+  mkdir -p $DIR/image/$newver/$newbundle/$(dirname "$newname")
+  cp -a $DIR/image/$origver/$origbundle/"$origname" $DIR/image/$newver/$newbundle/"$newname"
+}
+
+# Gets the hash for file NAME in BUNDLE manifest for VER
+hash_for() {
+  local ver=$1
+  local bundle=$2
+  local name="$3"
+
+  awk -F'\t' -v NAME="$name" 'NF == 4 && $4 == NAME { print $2 }' $DIR/www/$ver/Manifest.$bundle
+}
+
 # vi: ft=sh ts=8 sw=2 sts=2 et tw=80