Fix bug causing scripts to be run at the wrong time

diff --git a/src/main.c b/src/main.c
index 42330de..65c5319 100644 (file)
--- a/src/main.c
+++ b/src/main.c
@@ -48,12 +48,18 @@ 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 razor_stage_type stage);
+               enum update_pass_type stage);
 
 static struct razor_package_iterator *
 create_iterator_from_argv(struct razor_set *set, int argc, const char *argv[])
@@ -657,22 +663,24 @@ command_remove(int argc, const char *argv[])
 
        next = razor_transaction_commit(trans);
        ii = razor_set_create_install_iterator(system, next);
-       update_packages(trans, ii, system, next, atomic, NULL,
-                       RAZOR_STAGE_SCRIPTS_PRE);
-       update_packages(trans, ii, system, next, atomic, NULL,
-                       RAZOR_STAGE_FILES);
-
-       razor_root_update(root, next);
-
-       (void)razor_root_commit(root);
-       retval = razor_atomic_commit(atomic);
+       retval = update_packages(trans, ii, system, next, atomic, NULL,
+                                UPDATE_PASS_PRE_REMOVE);
        if (retval)
-               fprintf(stderr, "%s\n", razor_atomic_get_error_msg(atomic));
-       else
+               fprintf(stderr, "Remove aborted\n");
+       else {
                update_packages(trans, ii, system, next, atomic, NULL,
-                               RAZOR_STAGE_SCRIPTS_POST);
-       razor_install_iterator_destroy(ii);
+                               UPDATE_PASS_MAIN);
+
+               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_install_iterator_destroy(ii);
        razor_transaction_destroy(trans);
        razor_atomic_destroy(atomic);
        razor_set_unref(system);
@@ -980,28 +988,80 @@ install_package(struct razor_transaction *trans, struct razor_set *set,
        return retval;
 }
 
+/*
+ * 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.
+ */
 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 razor_stage_type stage)
+               enum update_pass_type pass)
 {
        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)
+       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, stage);
-               else if (action == RAZOR_INSTALL_ACTION_REMOVE)
+                                                relocations, count, add_stage);
+               else if (action == RAZOR_INSTALL_ACTION_REMOVE && remove_stage)
                        retval = razor_package_remove(system, next, atomic,
                                                      package, install_root,
-                                                     count, stage);
+                                                     count, remove_stage);
        }
 
        return retval;
@@ -1139,22 +1199,29 @@ command_install_or_update(int argc, const char *argv[], int do_update)
 
        ii = razor_set_create_install_iterator(system, next);
 
-       update_packages(trans, ii, system, next, atomic, relocations,
-                       RAZOR_STAGE_SCRIPTS_PRE);
-       update_packages(trans, ii, system, next, atomic, relocations,
-                       RAZOR_STAGE_FILES);
+       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_root_update(root, next);
 
-       razor_set_unref(upstream);
+               razor_set_unref(upstream);
 
-       (void)razor_root_commit(root);
-       retval = razor_atomic_commit(atomic);
-       if (retval)
-               fprintf(stderr, "%s\n", razor_atomic_get_error_msg(atomic));
-       else
-               update_packages(trans, ii, system, next, atomic, relocations,
-                               RAZOR_STAGE_SCRIPTS_POST);
+               (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_transaction_destroy(trans);
        if (relocations)