From 6afe3a8b72977c7d9cc1fedde960e3f2bf20e3a5 Mon Sep 17 00:00:00 2001
From: J. Ali Harlow <ali@juiblex.co.uk>
Date: Thu, 9 Feb 2012 20:45:27 +0000
Subject: [PATCH] Add transaction barriers
 These allow packages to be installed and removed which have scripts
 that depend on each other when atomic transactions are involved.
 Note that yum supports pre, but not other requires flags. post will
 need similar support to the post scripts themselves pulling in the
 requires flags from the rpms. Likewise preun and postun will need
 similar handling to those scrips since the requires flags will need
 to be stored in the razor database.

---
 configure.ac     |    4 +-
 librazor/razor.c |  218 +++++++++++++++++++++++++++++++++++++++++++++-
 librazor/razor.h |   17 ++++-
 src/import-yum.c |    6 +-
 src/main.c       |  254 +++++++++++++++++++++++-------------------------------
 test/zip.spec    |    1 +
 test/zsh.spec    |    1 +
 7 files changed, 344 insertions(+), 157 deletions(-)

diff --git a/configure.ac b/configure.ac
index 88911de..edf7b96 100644
--- a/configure.ac
+++ b/configure.ac
@@ -10,9 +10,9 @@ AM_MAINTAINER_MODE
 #
 # See http://sources.redhat.com/autobook/autobook/autobook_91.html#SEC91 for details
 #
-LT_CURRENT=2
+LT_CURRENT=3
 LT_REVISION=0
-LT_AGE=0
+LT_AGE=1
 AC_SUBST(LT_CURRENT)
 AC_SUBST(LT_REVISION)
 AC_SUBST(LT_AGE)
diff --git a/librazor/razor.c b/librazor/razor.c
index d7684b4..45c9fef 100644
--- a/librazor/razor.c
+++ b/librazor/razor.c
@@ -1,7 +1,7 @@
 /*
  * Copyright (C) 2008  Kristian Høgsberg <krh@redhat.com>
  * Copyright (C) 2008  Red Hat, Inc
- * Copyright (C) 2009-2011  J. Ali Harlow <ali@juiblex.co.uk>
+ * Copyright (C) 2009-2012  J. Ali Harlow <ali@juiblex.co.uk>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -992,6 +992,32 @@ add_action(enum razor_diff_action action,
 	}
 }
 
+/*
+ * Does <package> have a requirement for <script> which is
+ * satisfied by <provider> ?
+ * Note: We already know that <provider> is to be added as part of this install
+ * so there is no need to check the relation.
+ */
+static int
+package_script_requires(struct razor_set *set, struct razor_package *package,
+			enum razor_property_flags script,
+			struct razor_package *provider)
+{
+	struct list *link;
+	struct razor_property *prop;
+
+	link = list_first(&package->properties, &set->property_pool);
+	for(; link; link = list_next(link)) {
+		prop = set->properties.data;
+		prop += link->data;
+		if ((prop->flags & RAZOR_PROPERTY_SCRIPT_MASK) & script &&
+		    (prop->flags & RAZOR_PROPERTY_TYPE_MASK) == RAZOR_PROPERTY_REQUIRES &&
+		    provider->name == prop->name)
+			return 1;
+	}
+	return 0;
+}
+
 RAZOR_EXPORT struct razor_install_iterator *
 razor_set_create_install_iterator(struct razor_set *set,
 				  struct razor_set *next)
@@ -1002,10 +1028,12 @@ razor_set_create_install_iterator(struct razor_set *set,
 	 * A->B means action A should follow action B.
 	 */
 	struct graph follows;
-	struct install_action *actions, *ai, *aj;
+	struct install_action *actions, *ai, *aj, *an;
 	int i, j, count, vertex_added;
 	struct list *link;
 	struct razor_set *rs;
+	struct deque *order;
+	int barrier_needed;
 
 	assert (set != NULL);
 	assert (next != NULL);
@@ -1066,7 +1094,73 @@ razor_set_create_install_iterator(struct razor_set *set,
 			graph_add_edge(&follows, i, i);
 	}
 
-	ii->order = graph_sort(&follows);
+	/*
+	 * Because files are installed and removed using razor_atomic,
+	 * but scripts are run with no regard for these, we need some
+	 * means for synchronisation between the two. We support this
+	 * via transaction barriers which are points where
+	 * razor_atomic_commit() should be called so that scripts can
+	 * rely on the files they need being present.
+	 *
+	 * Rules:
+	 *   1)	Transaction barriers never occur within a dependency
+	 *	loop. Since we can't guarantee any ordering of actions
+	 *	within such a loop, they make no sense.
+	 *   2) Otherwise the following table applies:
+	 *	Action I    Action J	Barrier between I and J iff
+	 *	ADD P	    ADD Q	%pre(Q) requires P
+	 *	ADD P	    REMOVE Q	%preun(Q) requires P
+	 *	REMOVE P    ADD Q	never
+	 *	REMOVE P    REMOVE Q	%postun(P) requires Q
+	 *
+	 * FIXME:
+	 *	This should take account of conflicts somehow.
+	 */
+	order = graph_sort(&follows);
+	ii->order = deque_new(0);
+	if (!deque_empty(order)) {
+		j = deque_pop(order);
+		deque_unshift(ii->order, j);
+	}
+	while (!deque_empty(order)) {
+		i = j;
+		j = deque_pop(order);
+		ai = actions + i;
+		aj = actions + j;
+		if (graph_is_connected(&follows, i, j) &&
+		    graph_is_connected(&follows, j, i))
+			barrier_needed = 0;
+		else if (ai->action == RAZOR_INSTALL_ACTION_ADD &&
+			 aj->action == RAZOR_INSTALL_ACTION_ADD &&
+			 package_script_requires(next, aj->package,
+						 RAZOR_PROPERTY_PRE,
+						 ai->package))
+			barrier_needed = 1;
+		else if (ai->action == RAZOR_INSTALL_ACTION_ADD &&
+			 aj->action == RAZOR_INSTALL_ACTION_REMOVE &&
+			 package_script_requires(set, aj->package,
+						 RAZOR_PROPERTY_PREUN,
+						 ai->package))
+			barrier_needed = 1;
+		else if (ai->action == RAZOR_INSTALL_ACTION_REMOVE &&
+			 aj->action == RAZOR_INSTALL_ACTION_REMOVE &&
+			 package_script_requires(set, ai->package,
+						 RAZOR_PROPERTY_POSTUN,
+						 aj->package))
+			barrier_needed = 1;
+		else
+			barrier_needed = 0;
+		if (barrier_needed) {
+			an = array_add(&ii->actions, sizeof *an);
+			actions = ii->actions.data;
+			an->package = NULL;
+			an->action = RAZOR_INSTALL_ACTION_COMMIT;
+			deque_unshift(ii->order, an - actions);
+		}
+		deque_unshift(ii->order, j);
+	}
+	deque_free(order);
+
 	ii->left = deque_dup(ii->order);
 	graph_release(&follows);
 
@@ -1105,12 +1199,99 @@ razor_install_iterator_next(struct razor_install_iterator *ii,
 				(*count)++;
 		}
 		razor_package_iterator_destroy(pi);
-	} else
+	} else if (a->action == RAZOR_INSTALL_ACTION_ADD)
 		*count = 1;
 
 	return 1;
 }
 
+static int
+action_is_included(struct razor_install_iterator *ii, struct deque *done,
+		   struct razor_package *package,
+		   enum razor_install_action action)
+{
+	struct deque *t;
+	struct install_action *a;
+	int retval;
+
+	t = deque_dup(done);
+
+	while(!deque_empty(t)) {
+		a = (struct install_action *)ii->actions.data + deque_pop(t);
+		if (a->package == package && a->action == action)
+			break;
+	}
+	retval = !deque_empty(t);
+
+	deque_free(t);
+
+	return retval;
+}
+
+RAZOR_EXPORT struct razor_set *
+razor_install_iterator_commit_set(struct razor_install_iterator *ii)
+{
+	struct razor_merger *merger;
+	struct razor_set *set;
+	struct razor_package *n, *nend, *npkgs, *s, *send, *spkgs;
+	struct deque *done;
+	size_t pos;
+	char *npool, *spool;
+	int cmp;
+
+	done = deque_dup(ii->order);
+	pos = razor_install_iterator_tell(ii);
+	while(deque_length(done) > pos)
+		deque_shift(done);
+
+	s = ii->set->packages.data;
+	spkgs = ii->set->packages.data;
+	send = ii->set->packages.data + ii->set->packages.size;
+	spool = ii->set->string_pool.data;
+
+	n = ii->next->packages.data;
+	npkgs = ii->next->packages.data;
+	nend = ii->next->packages.data + ii->next->packages.size;
+	npool = ii->next->string_pool.data;
+
+	merger = razor_merger_create(ii->set, ii->next);
+	while (s < send || n < nend) {
+		if (s < send && n < nend)
+			cmp = strcmp(&spool[s->name], &npool[n->name]);
+		else if (s < send)
+			cmp = -1;
+		else
+			cmp = 1;
+
+		if (cmp < 0) {
+			if (!action_is_included(ii, done, s,
+						RAZOR_INSTALL_ACTION_REMOVE))
+				razor_merger_add_package(merger, s);
+			s++;
+		} else if (cmp == 0) {
+			if (!action_is_included(ii, done, s,
+						RAZOR_INSTALL_ACTION_REMOVE))
+				razor_merger_add_package(merger, s);
+			if (action_is_included(ii, done, n,
+					       RAZOR_INSTALL_ACTION_ADD))
+				razor_merger_add_package(merger, n);
+
+			s++;
+			n++;
+		} else {
+			if (action_is_included(ii, done, n,
+					       RAZOR_INSTALL_ACTION_ADD))
+				razor_merger_add_package(merger, n);
+			n++;
+		}
+	}
+
+	set = razor_merger_commit(merger);
+	razor_merger_destroy(merger);
+
+	return set;
+}
+
 RAZOR_EXPORT void
 razor_install_iterator_rewind(struct razor_install_iterator *ii)
 {
@@ -1118,6 +1299,35 @@ razor_install_iterator_rewind(struct razor_install_iterator *ii)
 	ii->left=deque_dup(ii->order);
 }
 
+RAZOR_EXPORT size_t
+razor_install_iterator_tell(struct razor_install_iterator *ii)
+{
+	return deque_length(ii->order) - deque_length(ii->left);
+}
+
+RAZOR_EXPORT size_t
+razor_install_iterator_seek(struct razor_install_iterator *ii, size_t pos)
+{
+	size_t current_pos;
+
+	if (pos > deque_length(ii->order))
+		pos = deque_length(ii->order);
+
+	current_pos = razor_install_iterator_tell(ii);
+
+	if (pos < current_pos) {
+		razor_install_iterator_rewind(ii);
+		current_pos = 0;
+	}
+
+	while(current_pos < pos) {
+		(void) deque_pop(ii->left);
+		current_pos++;
+	}
+
+	return current_pos;
+}
+
 RAZOR_EXPORT void
 razor_install_iterator_destroy(struct razor_install_iterator *ii)
 {
diff --git a/librazor/razor.h b/librazor/razor.h
index 52cac4a..8aeebf4 100644
--- a/librazor/razor.h
+++ b/librazor/razor.h
@@ -1,7 +1,7 @@
 /*
  * Copyright (C) 2008  Kristian Høgsberg <krh@redhat.com>
  * Copyright (C) 2008  Red Hat, Inc
- * Copyright (C) 2009, 2011  J. Ali Harlow <ali@juiblex.co.uk>
+ * Copyright (C) 2009, 2011, 2012  J. Ali Harlow <ali@juiblex.co.uk>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -349,7 +349,22 @@ int razor_install_iterator_next(struct razor_install_iterator *ii,
 				enum razor_install_action *action,
 				int *count);
 
+/**
+ * razor_install_iterator_commit_set
+ *
+ * Immediately after razor_install_iterator_next() returns
+ * RAZOR_INSTALL_ACTION_COMMIT, this function will return the razor_set
+ * which should be committed.
+ *
+ * Returns: a new #razor_set object.
+ **/
+struct razor_set *
+razor_install_iterator_commit_set(struct razor_install_iterator *ii);
+
 void razor_install_iterator_rewind(struct razor_install_iterator *ii);
+size_t razor_install_iterator_tell(struct razor_install_iterator *ii);
+size_t razor_install_iterator_seek(struct razor_install_iterator *ii,
+				   size_t pos);
 void razor_install_iterator_destroy(struct razor_install_iterator *ii);
 
 /**
diff --git a/src/import-yum.c b/src/import-yum.c
index f2520f6..017754b 100644
--- a/src/import-yum.c
+++ b/src/import-yum.c
@@ -170,11 +170,7 @@ yum_primary_start_element(void *data, const char *name, const char **atts)
 			else if (strcmp(atts[i], "flags") == 0)
 				relation = yum_to_razor_relation(atts[i + 1]);
 			else if (strcmp(atts[i], "pre") == 0)
-				pre = 
-					RAZOR_PROPERTY_PRE |
-					RAZOR_PROPERTY_POST |
-					RAZOR_PROPERTY_PREUN |
-					RAZOR_PROPERTY_POSTUN;
+				pre = RAZOR_PROPERTY_PRE;
 		}
 
 		if (n == NULL) {
diff --git a/src/main.c b/src/main.c
index 65c5319..2259596 100644
--- a/src/main.c
+++ b/src/main.c
@@ -1,7 +1,7 @@
 /*
  * Copyright (C) 2008  Kristian Høgsberg <krh@redhat.com>
  * Copyright (C) 2008  Red Hat, Inc
- * Copyright (C) 2009, 2011  J. Ali Harlow <ali@juiblex.co.uk>
+ * Copyright (C) 2009, 2011-2012  J. Ali Harlow <ali@juiblex.co.uk>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -48,18 +48,16 @@ static const char *yum_url;
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
 
-enum update_pass_type {
-    UPDATE_PASS_PRE_REMOVE,
-    UPDATE_PASS_MAIN,
-    UPDATE_PASS_POST_INSTALL,
-};
-
 static int
 update_packages(struct razor_transaction *trans,
 		struct razor_install_iterator *ii, struct razor_set *system,
 		struct razor_set *next, struct razor_atomic *atomic,
 		struct razor_relocations *relocations,
-		enum update_pass_type stage);
+		enum razor_stage_type stage);
+static int
+update_system(const char *install_root, struct razor_relocations *relocations,
+	      struct razor_transaction *trans, struct razor_set *system,
+	      struct razor_set *next, const char *verb);
 
 static struct razor_package_iterator *
 create_iterator_from_argv(struct razor_set *set, int argc, const char *argv[])
@@ -619,24 +617,22 @@ mark_packages_for_removal(struct razor_transaction *trans,
 static int
 command_remove(int argc, const char *argv[])
 {
-	struct razor_root *root;
 	struct razor_set *system, *upstream, *next;
 	struct razor_transaction *trans;
 	struct razor_atomic *atomic;
-	struct razor_install_iterator *ii;
 	int i, retval;
 
 	atomic = razor_atomic_open("Remove packages");
 
-	root = razor_root_open(install_root, atomic);
-	system = razor_set_ref(razor_root_get_system_set(root));
+	system = razor_root_open_read_only(install_root, atomic);
 	if (system == NULL) {
 		fprintf(stderr, "%s\n", razor_atomic_get_error_msg(atomic));
-		razor_root_close(root);
 		razor_atomic_destroy(atomic);
 		return 1;
 	}
 
+	razor_atomic_destroy(atomic);
+
 	upstream = razor_set_create_without_root();
 	trans = razor_transaction_create(system, upstream);
 	razor_set_unref(upstream);
@@ -645,8 +641,6 @@ command_remove(int argc, const char *argv[])
 			fprintf(stderr, "no match for %s\n", argv[i]);
 			razor_transaction_destroy(trans);
 			razor_set_unref(system);
-			razor_root_close(root);
-			razor_atomic_destroy(atomic);
 			return 1;
 		}
 	}
@@ -656,33 +650,15 @@ command_remove(int argc, const char *argv[])
 	if (retval) {
 		razor_transaction_destroy(trans);
 		razor_set_unref(system);
-		razor_root_close(root);
-		razor_atomic_destroy(atomic);
 		return 1;
 	}
 
 	next = razor_transaction_commit(trans);
-	ii = razor_set_create_install_iterator(system, next);
-	retval = update_packages(trans, ii, system, next, atomic, NULL,
-				 UPDATE_PASS_PRE_REMOVE);
-	if (retval)
-		fprintf(stderr, "Remove aborted\n");
-	else {
-		update_packages(trans, ii, system, next, atomic, NULL,
-				UPDATE_PASS_MAIN);
 
-		razor_root_update(root, next);
+	retval = update_system(install_root, NULL, trans, system, next,
+			       "Remove");
 
-		(void)razor_root_commit(root);
-		retval = razor_atomic_commit(atomic);
-		if (retval)
-			fprintf(stderr, "%s\n",
-				razor_atomic_get_error_msg(atomic));
-	}
-
-	razor_install_iterator_destroy(ii);
 	razor_transaction_destroy(trans);
-	razor_atomic_destroy(atomic);
 	razor_set_unref(system);
 	razor_set_unref(next);
 
@@ -826,7 +802,7 @@ download_packages(struct razor_set *system, struct razor_set *next)
 
 	ii = razor_set_create_install_iterator(system, next);
 	while (razor_install_iterator_next(ii, &package, &action, &count)) {
-		if (action == RAZOR_INSTALL_ACTION_REMOVE)
+		if (action != RAZOR_INSTALL_ACTION_ADD)
 			continue;
 
 		razor_package_get_details(next, package,
@@ -989,108 +965,118 @@ install_package(struct razor_transaction *trans, struct razor_set *set,
 }
 
 /*
- * In the most general case, there should be three passes:
- *
- * 1) For each package to be removed, run %preun
- *
- * 2) For each package:
- *	If the package is to be installed, run %pre
- *	Update the files on disk
- *	If the package has been removed, run %postun
- *
- * 3) For each packge installed, run %post
- *
- * This guarantees that:
- *   a)	Save where dependency loops make it impossible, at the time
- *	%pre is run, all required packages are installed (although
- *	their %post script may not have been run). We should support
- *	Requires(%pre) to allow packagers to describe their requirements
- *	more accurately and avoid unnecessary dependency loops.
- *   b)	At the time %preun is run, all required packages are installed.
- *	Supporting Requires(%preun) would make this more complicated
- *	since we might have to install a package in order to remove
- *	another one. For now, treating Requires(%preun) as Requires
- *	seems more sensible.
- *   c)	At the time %post is run, all required packages are installed.
- *	Supporting Requires(%post) would allow us to remove a package
- *	that was only needed to install another, but there seems no
- *	obvious advantage.
- *   d)	Save where dependency loops make it impossible, at the time
- *	%postun is run, all required packages are installed. Again,
- *	we should support Requires(%postun) to avoid unnecessary
- *	dependency loops.
- *
- * Notes:
- *	rpm treats %pre and %preun script failures as fatal errors
- *	and %post and %postun failures as warnings.
+ * Returns 0 on success, -1 on failure and 1 if a RAZOR_INSTALL_ACTION_COMMIT
+ * is met (in which case the action is consumed).
  */
 static int
 update_packages(struct razor_transaction *trans,
 		struct razor_install_iterator *ii, struct razor_set *system,
 		struct razor_set *next, struct razor_atomic *atomic,
 		struct razor_relocations *relocations,
-		enum update_pass_type pass)
+		enum razor_stage_type stage)
 {
 	struct razor_package *package;
 	enum razor_install_action action;
 	int retval = 0, count;
-	enum razor_stage_type remove_stage, add_stage;
-
-	switch (pass) {
-	case UPDATE_PASS_PRE_REMOVE:
-		add_stage = 0;
-		remove_stage = RAZOR_STAGE_SCRIPTS_PRE;
-		break;
-	case UPDATE_PASS_MAIN:
-		add_stage = RAZOR_STAGE_SCRIPTS_PRE | RAZOR_STAGE_FILES;
-		remove_stage = RAZOR_STAGE_FILES | RAZOR_STAGE_SCRIPTS_POST;
-		break;
-	case UPDATE_PASS_POST_INSTALL:
-		add_stage = RAZOR_STAGE_SCRIPTS_POST;
-		remove_stage = 0;
-		break;
-	}
-
-	razor_install_iterator_rewind(ii);
 
 	while (!retval && razor_install_iterator_next(ii, &package, &action,
 						      &count)) {
-		if (action == RAZOR_INSTALL_ACTION_ADD && add_stage)
-			retval = install_package(trans, next, atomic, package,
-						 relocations, count, add_stage);
-		else if (action == RAZOR_INSTALL_ACTION_REMOVE && remove_stage)
-			retval = razor_package_remove(system, next, atomic,
-						      package, install_root,
-						      count, remove_stage);
+		if (action == RAZOR_INSTALL_ACTION_ADD) {
+			if (install_package(trans, next, atomic, package,
+					    relocations, count, stage))
+				retval = -1;
+		} else if (action == RAZOR_INSTALL_ACTION_REMOVE) {
+			if (razor_package_remove(system, next, atomic, package,
+						 install_root, count, stage))
+				retval = -1;
+		} else if (action == RAZOR_INSTALL_ACTION_COMMIT)
+				retval = 1;
 	}
 
 	return retval;
 }
 
 static int
-command_install_or_update(int argc, const char *argv[], int do_update)
+update_system(const char *install_root, struct razor_relocations *relocations,
+	      struct razor_transaction *trans, struct razor_set *system,
+	      struct razor_set *next, const char *verb)
 {
 	struct razor_root *root;
+	struct razor_set *set;
+	struct razor_atomic *atomic;
+	struct razor_install_iterator *ii;
+	int r, retval = 0;
+	char *description;
+	size_t pos;
+
+	description = razor_concat(verb, " packages", NULL);
+
+	ii = razor_set_create_install_iterator(system, next);
+
+	do {
+		pos = razor_install_iterator_tell(ii);
+
+		atomic = razor_atomic_open(description);
+
+		root = razor_root_open(install_root, atomic);
+		if (root == NULL) {
+			fprintf(stderr, "%s\n",
+				razor_atomic_get_error_msg(atomic));
+			razor_atomic_destroy(atomic);
+			retval = 1;
+			break;
+		}
+
+		r = update_packages(trans, ii, system, next, atomic,
+				    relocations, RAZOR_STAGE_SCRIPTS_PRE);
+		if (r < 0) {
+			fprintf(stderr, "%s aborted\n", verb);
+			razor_atomic_destroy(atomic);
+			retval = r;
+		} else {
+			razor_install_iterator_seek(ii, pos);
+			r = update_packages(trans, ii, system, next, atomic,
+					    relocations, RAZOR_STAGE_FILES);
+
+			if (r == 1) {
+				set = razor_install_iterator_commit_set(ii);
+				razor_root_update(root, set);
+				razor_set_unref(set);
+			} else if (r == 0)
+				razor_root_update(root, next);
+
+			(void)razor_root_commit(root);
+			retval = razor_atomic_commit(atomic);
+			if (retval) {
+				fprintf(stderr, "%s\n",
+					razor_atomic_get_error_msg(atomic));
+				razor_atomic_destroy(atomic);
+			} else {
+				razor_install_iterator_seek(ii, pos);
+				update_packages(trans, ii, system, next,
+						atomic, relocations,
+						RAZOR_STAGE_SCRIPTS_POST);
+			}
+		}
+
+		razor_atomic_destroy(atomic);
+	} while(!retval && r == 1);
+
+	free(description);
+
+	return retval;
+}
+
+static int
+command_install_or_update(int argc, const char *argv[], int do_update)
+{
 	struct razor_relocations *relocations=NULL;
 	struct razor_set *system, *upstream, *next, *set;
 	struct razor_transaction *trans;
 	struct razor_atomic *atomic;
-	struct razor_install_iterator *ii;
 	int i, retval, len, dependencies = 1;
 	char *oldpath;
 
-	if (do_update)
-		atomic = razor_atomic_open("Update packages");
-	else
-		atomic = razor_atomic_open("Install packages");
-
-	root = razor_root_open(install_root, atomic);
-	if (root == NULL) {
-		fprintf(stderr, "%s\n", razor_atomic_get_error_msg(atomic));
-		razor_atomic_destroy(atomic);
-		return 1;
-	}
-
 	for (i = 0; i < argc; i++) {
 		if (strcmp(argv[i], "--no-dependencies") == 0)
 			dependencies = 0;
@@ -1119,10 +1105,14 @@ command_install_or_update(int argc, const char *argv[], int do_update)
 			break;
 	}
 
+	if (do_update)
+		atomic = razor_atomic_open("Update packages");
+	else
+		atomic = razor_atomic_open("Install packages");
+
 	upstream = razor_set_open(rawhide_repo_filename, atomic);
 	if (upstream == NULL) {
 		fprintf(stderr, "%s\n", razor_atomic_get_error_msg(atomic));
-		razor_root_close(root);
 		razor_atomic_destroy(atomic);
 		return 1;
 	}
@@ -1132,7 +1122,6 @@ command_install_or_update(int argc, const char *argv[], int do_update)
 		if (set == NULL) {
 			fprintf(stderr, "%s\n",
 				razor_atomic_get_error_msg(atomic));
-			razor_root_close(root);
 			razor_atomic_destroy(atomic);
 			razor_set_unref(upstream);
 			return 1;
@@ -1141,7 +1130,7 @@ command_install_or_update(int argc, const char *argv[], int do_update)
 		upstream = set;
 	}
 
-	system = razor_set_ref(razor_root_get_system_set(root));
+	system = razor_root_open_read_only(install_root, atomic);
 
 	trans = razor_transaction_create(system, upstream);
 
@@ -1156,7 +1145,6 @@ command_install_or_update(int argc, const char *argv[], int do_update)
 			razor_transaction_destroy(trans);
 			razor_set_unref(upstream);
 			razor_set_unref(system);
-			razor_root_close(root);
 			razor_atomic_destroy(atomic);
 			return 1;
 		}
@@ -1168,23 +1156,24 @@ command_install_or_update(int argc, const char *argv[], int do_update)
 			razor_transaction_destroy(trans);
 			razor_set_unref(upstream);
 			razor_set_unref(system);
-			razor_root_close(root);
 			razor_atomic_destroy(atomic);
 			return 1;
 		}
 	}
 
 	if (razor_atomic_create_dir(atomic, "rpms",
-				    S_IRWXU | S_IRWXG | S_IRWXO)) {
+				    S_IRWXU | S_IRWXG | S_IRWXO) ||
+	    razor_atomic_commit(atomic)) {
 		fprintf(stderr, "%s\n", razor_atomic_get_error_msg(atomic));
 		razor_transaction_destroy(trans);
 		razor_set_unref(upstream);
 		razor_set_unref(system);
-		razor_root_close(root);
 		razor_atomic_destroy(atomic);
 		return 1;
 	}
 
+	razor_atomic_destroy(atomic);
+
 	next = razor_transaction_commit(trans);
 
 	if (download_packages(system, next) < 0) {
@@ -1192,43 +1181,18 @@ command_install_or_update(int argc, const char *argv[], int do_update)
 		razor_transaction_destroy(trans);
 		razor_set_unref(upstream);
 		razor_set_unref(system);
-		razor_root_close(root);
 		razor_atomic_destroy(atomic);
                 return 1;
         }
 
-	ii = razor_set_create_install_iterator(system, next);
-
-	retval = update_packages(trans, ii, system, next, atomic, relocations,
-				 UPDATE_PASS_PRE_REMOVE);
-	if (retval)
-		fprintf(stderr, "%s aborted\n",
-			do_update ? "Update" : "Install");
-	else {
-		update_packages(trans, ii, system, next, atomic, relocations,
-				UPDATE_PASS_MAIN);
-
-		razor_root_update(root, next);
-
-		razor_set_unref(upstream);
+	retval = update_system(install_root, relocations, trans, system, next,
+			       do_update ? "Update" : "Install");
 
-		(void)razor_root_commit(root);
-		retval = razor_atomic_commit(atomic);
-		if (retval)
-			fprintf(stderr, "%s\n",
-				razor_atomic_get_error_msg(atomic));
-		else
-			(void)update_packages(trans, ii, system, next, atomic,
-					      relocations,
-					      UPDATE_PASS_POST_INSTALL);
-	}
+	razor_set_unref(upstream);
 
 	razor_transaction_destroy(trans);
 	if (relocations)
 		razor_relocations_destroy(relocations);
-	razor_install_iterator_destroy(ii);
-
-	razor_atomic_destroy(atomic);
 
 	razor_set_unref(next);
 	razor_set_unref(system);
diff --git a/test/zip.spec b/test/zip.spec
index c9fd04e..8a25430 100644
--- a/test/zip.spec
+++ b/test/zip.spec
@@ -12,6 +12,7 @@ BuildArch: noarch
 BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 Prefix:    /usr
 Requires:  zap
+Requires(pre,postun):  zap
 
 %description
 Test package
diff --git a/test/zsh.spec b/test/zsh.spec
index a2e674b..c51fdfe 100644
--- a/test/zsh.spec
+++ b/test/zsh.spec
@@ -12,6 +12,7 @@ BuildArch: noarch
 BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 Prefix:    /usr
 Requires:  zip
+Requires(pre,postun):  zip
 
 %description
 Test package
-- 
1.7.1