Bug#222500: The state engine has (i mean.. had :) some problems.
Package: anna
Version: 0.044
Severity: normal
Tags: d-i, patch
I'm still hacking anna... hope it's ok. The attached patch fixes some
problems. I couldn't resist revamping the code a bit further than
strictly necessary. Here are the new changelog entries :
- status, packages and packages_allocator have been made global.
- No longer use debconf to store the retriever to use. Instead, the global
variable retriever_path is used. This way the question doesn't ever
exist when a retriever is provided on command line.
- New shiny question mechanism. Going back from the module selection
no longer show the retreiver selection when one was given on the
command line. Going back from errors now work properly.
This patch is to be applied after the one in #219888 (the full selection
thing.) Please also add the correct Closes: statement in the changelog.
About the question mechanism, each state now has its own structure,
pointing to two optional functions (to be called before and after the
question is input), errors have been turned into states, error templates
and codes are now stored directly into the states. They're not really
"states" any longer, however, since a stack of them is now used via
recursive calls of step_go().
--
Jeremie Koenig <sprite@sprite.fr.eu.org>
diff -ur anna.219888/anna.c anna/anna.c
--- anna.219888/anna.c Tue Nov 25 21:19:27 2003
+++ anna/anna.c Sat Nov 29 17:57:13 2003
@@ -6,13 +6,46 @@
#include <sys/utsname.h>
#include "anna.h"
+/* steps for debconf questions asking */
+struct step {
+ char *priority;
+ char *question;
+ int error;
+ int (*prepare)(char **question);
+ int (*finish)(char *value, struct step **next);
+ struct step *next;
+};
+int step_go(struct step *step);
+
+/* Steps for errors */
+struct step step_no_modules =
+ {"critical", "anna/no_modules", 4, NULL, NULL, NULL};
+struct step step_retrieve_failed =
+ {"critical", "anna/retreive_failed", 6, NULL, NULL, NULL};
+struct step step_md5sum_failed =
+ {"critical", "anna/md5sum_failed", 7, NULL, NULL, NULL};
+struct step step_install_failed =
+ {"critical", "anna/install_failed", 8, NULL, NULL, NULL};
+
struct debconfclient *debconf = NULL;
+di_packages *status;
+di_packages *packages = NULL;
+di_packages_allocator *packages_allocator = NULL;
static int
-choose_retriever(di_packages *status, di_packages **packages __attribute__((unused)), di_packages_allocator **packages_allocator __attribute__((unused)))
+choose_retriever(char **question)
{
char *choices;
- di_package **retrievers;
+ di_package **retrievers;
+
+ if (packages) {
+ di_packages_free(packages);
+ packages = NULL;
+ }
+ if (packages_allocator) {
+ di_packages_allocator_free(packages_allocator);
+ packages_allocator = NULL;
+ }
retrievers = get_retriever_packages(status);
if (!retrievers)
@@ -27,12 +60,11 @@
if (retriever && (p = di_packages_get_package(status, retriever, 0))) {
package_to_choice(p, buf, 200);
- set_retriever(buf);
+ debconf_set(debconf, anna_retriever, buf);
}
}
-
debconf_subst(debconf, anna_retriever, "CHOICES", choices);
- debconf_input(debconf, "medium", anna_retriever);
+ *question = anna_retriever;
di_free(retrievers);
di_free(choices);
@@ -40,9 +72,28 @@
return 0;
}
+static int
+setup_retriever(char *value, struct step **next_step)
+{
+ char *c;
+
+ if ((c = strchr(value, ':')))
+ *c = '\0';
+
+ set_retriever(value);
+ config_retriever();
+
+ packages_allocator = di_system_packages_allocator_alloc();
+ packages = get_packages(packages_allocator);
+
+ if (packages == NULL)
+ *next_step = &step_no_modules;
+
+ return 0;
+}
static int
-choose_modules(di_packages *status, di_packages **packages, di_packages_allocator **packages_allocator)
+choose_modules(char **question)
{
char *choices, *choices_dfl, *package_kernel, *running_kernel = NULL;
int package_count = 0, package_count_dfl, i;
@@ -50,26 +101,15 @@
di_slist_node *node, *node1;
struct utsname uts;
- config_retriever();
if (uname(&uts) == 0)
running_kernel = uts.release;
- *packages_allocator = di_system_packages_allocator_alloc();
- *packages = get_packages(*packages_allocator);
-
- if (*packages == NULL) {
- debconf_fset(debconf, ANNA_NO_MODULES, "seen", "false");
- debconf_input(debconf, "critical", ANNA_NO_MODULES);
- debconf_go(debconf);
- return 4;
- }
-
/* XXX enhances is not a legal field for udebs, so why is this here?
* -- JEH */
for (node = status->list.head; node; node = node->next) {
status_package = node->data;
if (status_package->status == di_package_status_unpacked || status_package->status == di_package_status_installed) {
- package = di_packages_get_package(*packages, status_package->package, 0);
+ package = di_packages_get_package(packages, status_package->package, 0);
if (!package)
continue;
for (node1 = package->depends.head; node1; node1 = node1->next) {
@@ -90,7 +130,7 @@
subarchitecture = strdup(debconf->value);
#endif
- for (node = (*packages)->list.head; node; node = node->next) {
+ for (node = packages->list.head; node; node = node->next) {
package = node->data;
package_kernel = udeb_kernel_version(package);
@@ -127,21 +167,21 @@
}
/* Include packages in udeb_include */
- take_includes(*packages);
+ take_includes(packages);
/* Drop packages in udeb_exclude */
- drop_excludes(*packages);
+ drop_excludes(packages);
- di_packages_resolve_dependencies_mark(*packages);
+ di_packages_resolve_dependencies_mark(packages);
/* Now build the asklist. If debconf/priority == low, show all packages
* and preselect the good ones. Otherwise we only offer extra things. */
/* Slight over-allocation, but who cares */
- package_array = di_new0(di_package *, di_hash_table_size((*packages)->table));
+ package_array = di_new0(di_package *, di_hash_table_size(packages->table));
/* Modules selected by default, listed first. */
- if(list_all_mods) {
- for (node = (*packages)->list.head; node; node = node->next) {
+ if (list_all_mods) {
+ for (node = packages->list.head; node; node = node->next) {
package = node->data;
if (package->status_want == di_package_status_want_install)
package_array[package_count++] = package;
@@ -152,7 +192,7 @@
package_count_dfl = package_count;
choices_dfl = list_to_choices(package_array);
/* Now, extra things. */
- for (node = (*packages)->list.head; node; node = node->next) {
+ for (node = packages->list.head; node; node = node->next) {
package = node->data;
if (package->status_want == di_package_status_want_unknown)
package_array[package_count++] = package;
@@ -162,14 +202,14 @@
choices = list_to_choices(package_array);
- if(list_all_mods)
+ if (list_all_mods)
for(i = 0 ; i < package_count_dfl ; i++)
package_array[i]->status_want = di_package_status_want_unknown;
debconf_subst(debconf, anna_choose_modules, "CHOICES", choices);
- if(!seen(debconf, anna_choose_modules))
+ if (!seen(debconf, anna_choose_modules))
debconf_set(debconf, anna_choose_modules, choices_dfl);
- debconf_input(debconf, "medium", anna_choose_modules);
+ *question = anna_choose_modules;
di_free(choices);
di_free(choices_dfl);
@@ -179,23 +219,18 @@
}
static int
-install_modules(di_packages *status, di_packages *packages, di_packages_allocator *status_allocator __attribute__ ((unused)))
+install_modules(char *value, struct step **next_step)
{
di_slist_node *node;
di_package *package;
char *f, *fp, *dest_file;
int ret = 0, pkg_count = 0;
- debconf_get(debconf, anna_choose_modules);
- if (debconf->value != NULL) {
- char *choices = debconf->value;
-
- for (node = packages->list.head; node; node = node->next) {
- package = node->data;
- /* Not very safe, but at least easy ;) */
- if (strstr(choices, package->package) != NULL)
- package->status_want = di_package_status_want_install;
- }
+ for (node = packages->list.head; node; node = node->next) {
+ package = node->data;
+ /* Not very safe, but at least easy ;) */
+ if (strstr(value, package->package) != NULL)
+ package->status_want = di_package_status_want_install;
}
di_packages_resolve_dependencies_mark(packages);
@@ -229,22 +264,14 @@
debconf_subst(debconf, "anna/progress_step_inst", "PACKAGE", package->package);
debconf_progress_info(debconf, "anna/progress_step_retr");
if (get_package(package, dest_file)) {
- debconf_progress_stop(debconf);
- debconf_subst(debconf, "anna/retrieve_failed", "PACKAGE", package->package);
- debconf_input(debconf, "critical", "anna/retrieve_failed");
- debconf_go(debconf);
free(dest_file);
- ret = 6;
+ *next_step = &step_retrieve_failed;
break;
}
if (!md5sum(package->md5sum, dest_file)) {
- debconf_progress_stop(debconf);
- debconf_subst(debconf, "anna/md5sum_failed", "PACKAGE", package->package);
- debconf_input(debconf, "critical", "anna/md5sum_failed");
- debconf_go(debconf);
unlink(dest_file);
free(dest_file);
- ret = 7;
+ *next_step = &step_md5sum_failed;
break;
}
debconf_progress_step(debconf, 1);
@@ -254,13 +281,9 @@
#else
if (!unpack_package(dest_file)) {
#endif
- debconf_progress_stop(debconf);
- debconf_subst(debconf, "anna/install_failed", "PACKAGE", package->package);
- debconf_input(debconf, "critical", "anna/install_failed");
- debconf_go(debconf);
unlink(dest_file);
free(dest_file);
- ret = 8;
+ *next_step = &step_install_failed;
break;
}
unlink(dest_file);
@@ -274,18 +297,58 @@
return ret;
}
+
+/* May seem a bit overkill for two questions, but my true intention is to get
+ * this included in libdebconfclient someday. Calls itself recursively to
+ * handle backup (there's no such stack as the stack.) */
+int step_go(struct step *step)
+{
+ char *question;
+ struct step *next;
+ int ret;
+
+ do {
+ /* Prepare the question */
+ question = step->question;
+ if (step->prepare && (ret = step->prepare(&question)) != 0)
+ return ret;
+ if (!question)
+ return 0;
+ if (step->error)
+ debconf_fset(debconf, question, "seen", "false");
+
+ /* Ask */
+ debconf_input(debconf, step->priority, question);
+ if ( (ret = debconf_go(debconf) ) != 0
+ || (ret = debconf_get(debconf, question) ) != 0)
+ return ret;
+
+ /* What's next ? */
+ next = step->next ? step->next : step + 1;
+ if (step->finish && (ret = step->finish(debconf->value, &next)) != 0)
+ return ret;
+ if (step->error)
+ return step->error;
+ if (!next)
+ return 0;
+ } while((ret = step_go(next)) == CMD_GOBACK);
+
+ return ret;
+}
+
+
#ifndef TEST
int
main(int argc, char **argv)
{
- int ret, state = 0;
- int (*states[])(di_packages *status, di_packages **packages, di_packages_allocator **packages_allocator) = {
- choose_retriever,
- choose_modules,
- NULL,
+ struct step steps[] = {
+ {"medium", NULL, 0, choose_retriever, setup_retriever, NULL},
+ {"medium", NULL, 0, choose_modules, install_modules, NULL},
+ {NULL, NULL, 0, NULL, NULL, NULL}
};
- di_packages *packages = NULL, *status;
- di_packages_allocator *packages_allocator = NULL, *status_allocator;
+ struct step *first_step;
+ int ret;
+ di_packages_allocator *status_allocator;
debconf = debconfclient_new();
debconf_capb(debconf, "backup");
@@ -301,39 +364,22 @@
* should be separate, and menu stuff should provide their names for
* use in question names instead. */
if (argc > 1) {
- prepare_questions(argv[1]);
- set_retriever(argv[1]);
- state=1; /* skip manual setting and use the supplied retriever */
- } else
- prepare_questions(NULL);
-
- while (state >= 0 && states[state] != NULL) {
- ret = states[state](status, &packages, &packages_allocator);
- if (ret != 0)
- state = -1;
- else if (debconf_go(debconf) == 0)
- state++;
- else
- {
- if (packages)
- {
- di_packages_free(packages);
- di_packages_allocator_free(packages_allocator);
- packages = NULL;
- packages_allocator = NULL;
- }
- state--;
- }
+ prepare_questions(argv[1], 0);
+ /* skip manual setting and use the supplied retriever */
+ first_step = steps + 1;
+ setup_retriever(argv[1], &first_step);
+ } else {
+ prepare_questions(NULL, 1);
+ first_step = steps;
}
- if (state < 0)
- ret = 10; /* back to the menu */
- else
- {
- ret = install_modules(status, packages, status_allocator);
+
+ ret = step_go(first_step);
+
#ifdef LIBDI_SYSTEM_DPKG
+ if (ret == 0)
di_system_packages_status_write_file(status, DI_SYSTEM_DPKG_STATUSFILE);
#endif
- }
+
if (!ret && argc > 1) {
di_package **retrievers_after = get_retriever_packages(status);
if (new_retrievers(retrievers_before, retrievers_after))
diff -ur anna.219888/anna.h anna/anna.h
--- anna.219888/anna.h Mon Nov 24 10:41:45 2003
+++ anna/anna.h Sat Nov 29 15:47:24 2003
@@ -26,12 +26,13 @@
#undef LIBDI_SYSTEM_DPKG
extern struct debconfclient *debconf;
+extern char *retriever_path;
#define seen(debconf, question) \
((debconf_fget(debconf, question, "seen") == CMD_SUCCESS) \
&& (strcmp(debconf->value, "true") == 0))
-void prepare_questions(const char *retriever);
+void prepare_questions(const char *retriever, int use_retriever);
di_package **get_retriever_packages(di_packages *status);
const char *get_default_retriever(const char *choices);
void set_retriever(const char *retriever);
diff -ur anna.219888/debian/changelog anna/debian/changelog
--- anna.219888/debian/changelog Tue Nov 25 21:35:45 2003
+++ anna/debian/changelog Wed Nov 26 20:42:33 2003
@@ -14,6 +14,13 @@
- Get rid of unconditionnaly resetting the seen flag. Instead,
instanciate multiple questions from the templates, depending on the
retriever passed on the command line.
+ - status, packages and packages_allocator have been made global.
+ - No longer use debconf to store the retriever to use. Instead, the global
+ variable retriever_path is used. This way the question doesn't ever
+ exist when a retriever is provided on command line.
+ - New shiny question mechanism. Go back no longer show the retreiver
+ selection when one was given on the command line. "Go back" on errors
+ now goes back.
-- Bart Cornelis <cobaco@linux.be> Mon, 17 Nov 2003 16:44:25 +0100
diff -ur anna.219888/util.c anna/util.c
--- anna.219888/util.c Tue Nov 25 21:10:37 2003
+++ anna/util.c Sat Nov 29 15:47:13 2003
@@ -6,19 +6,20 @@
#include <sys/utsname.h>
#include "anna.h"
-/* Choose which questions to ask. Instanciate templates if necessary */
-
char *anna_choose_modules;
char *anna_retriever;
int list_all_mods;
+char *retriever_path = NULL;
+
+/* Choose which questions to ask. Instanciate templates if necessary */
static char *instanciate(const char *template, const char *suffix)
{
char *question;
- if(!suffix) {
+ if (!suffix)
question = strdup(template);
- } else {
+ else {
asprintf(&question, "%s.%s", template, suffix);
debconf_register(debconf, template, question);
}
@@ -26,16 +27,16 @@
return question;
}
-void prepare_questions(const char *suffix)
+void prepare_questions(const char *suffix, int use_retr)
{
char *anna_list_all_mods;
anna_list_all_mods = instanciate(ANNA_LIST_ALL_MODS, suffix);
- if(debconf_get(debconf, "debconf/priority") != CMD_SUCCESS)
+ if (debconf_get(debconf, "debconf/priority") != CMD_SUCCESS)
list_all_mods = 0;
- else if(strcmp(debconf->value, "low") == 0)
+ else if (strcmp(debconf->value, "low") == 0)
list_all_mods = 1;
- else if(debconf_get(debconf, anna_list_all_mods) != CMD_SUCCESS)
+ else if (debconf_get(debconf, anna_list_all_mods) != CMD_SUCCESS)
list_all_mods = 0;
else
list_all_mods = (strcmp(debconf->value, "true") == 0);
@@ -44,7 +45,7 @@
anna_choose_modules = instanciate(list_all_mods ? ANNA_CHOOSE_MODULES_ALL
: ANNA_CHOOSE_MODULES, suffix);
- anna_retriever = instanciate(ANNA_RETRIEVER, suffix);
+ anna_retriever = use_retr ? instanciate(ANNA_RETRIEVER, suffix) : NULL;
}
/* Construct a list of all the retriever packages */
@@ -97,59 +98,45 @@
/* Force use of a given retriever. */
void
-set_retriever(const char *retriever) {
- debconf_set(debconf, anna_retriever, retriever);
+set_retriever(const char *retriever)
+{
+ if (retriever_path)
+ free(retriever_path);
+ asprintf(&retriever_path, "%s/%s", RETRIEVER_DIR, retriever);
}
/*
* Helper functions for choose_modules
*/
-char *
-get_retriever(void)
-{
- char *retriever = NULL, *colon_p = NULL;
-
- debconf_get(debconf, anna_retriever);
- if (debconf->value != NULL)
- colon_p = strchr(debconf->value, ':');
- if (colon_p != NULL)
- *colon_p = '\0';
- if (asprintf(&retriever, "%s/%s", RETRIEVER_DIR, debconf->value) == -1)
- retriever = NULL;
- if (!retriever)
- di_log (DI_LOG_LEVEL_ERROR, "can't find retriever in %s, got %s", __PRETTY_FUNCTION__, debconf->value);
- return retriever;
-}
-
-/* Corresponds to the 'config' retriever command */
int
-config_retriever(void)
+retriever_command(const char *cmd, const char *arg1, const char *arg2)
{
- char *retriever, *command;
+ const char *fmt;
+ char *command;
int ret;
- retriever = get_retriever();
- if (asprintf(&command, "%s config", retriever) == -1)
+ fmt = arg1 ? (arg2 ? "%s %s %s %s" : "%s %s %s") : "%s %s";
+ if (asprintf(&command, fmt, retriever_path, cmd, arg1, arg2) == -1)
return 1;
ret = di_exec_shell_log(command);
free(command);
return ret;
}
+/* Corresponds to the 'config' retriever command */
+int
+config_retriever(void)
+{
+ return retriever_command("config", NULL, NULL);
+}
+
di_packages *
get_packages(di_packages_allocator *allocator)
{
di_packages *packages;
- char *retriever, *command;
- int ret;
- retriever = get_retriever();
- if (asprintf(&command, "%s packages " DOWNLOAD_PACKAGES, retriever) == -1)
- return NULL;
- ret = di_exec_shell_log(command);
- free(command);
- if (ret != 0)
+ if (retriever_command("packages", DOWNLOAD_PACKAGES, NULL) != 0)
return NULL;
packages = di_system_packages_read_file(DOWNLOAD_PACKAGES, allocator);
unlink(DOWNLOAD_PACKAGES);
@@ -239,17 +226,7 @@
int
get_package (di_package *package, char *dest)
{
- int ret;
- char *retriever;
- char *command;
-
- retriever = get_retriever();
- if (asprintf(&command, "%s retrieve %s %s", retriever, package->filename, dest) == -1)
- return 1;
- ret = di_exec_shell_log(command);
- free(retriever);
- free(command);
- return ret;
+ return retriever_command("retrieve", package->filename, dest);
}
/* Calls udpkg to unpack a package. */
@@ -298,15 +275,7 @@
void
cleanup(void)
{
- char *retriever;
- char *command;
-
- retriever = get_retriever();
- if (asprintf(&command, "%s cleanup", retriever) != -1) {
- di_exec_shell_log(command);
- free(command);
- }
- free(retriever);
+ retriever_command("cleanup", NULL, NULL);
}
/*
Reply to: