Re: debconf and debconf/frontend
Hello again,
So, after some work, I have a couple of patches. However, I would very
much like some feedback on it before pushing them.
The first one reworks completely the frontend loading to be more robust,
and better handle situations where debconf/frontend is changed,
including not crashing if selecting one which is not available.
One side effect of this is that cdebconf can load the debconf templates
file without crashing. Therefore, all templates can be loaded and the
values from the debconf database be loaded. I have a very ugly local
script to do it, which will need more work to make sure it actually
handles all transition cases possible.
The second patch reads debconf/frontend which takes precedence over the
value in the config file. Debconf does it the other way around, but this
one sounds more sensible, so I'm not sure in which order to apply it.
Then there's the issue of the debconf/frontend values not being
compatible with cdebconf, but at least now they are simply ignored.
Obviously I'm very keen on my patches being reviewed before I push them,
as it's a rather intrusive change that I don't want to get wrong.
Regis
On Thu, 2011-07-21 at 23:38 +0100, Regis Boudin wrote:
> Hi all,
>
> So, with the small patch applied to debconf, I have now switched my
> laptop to use cdebconf with DEBCONF_USE_CDEBCONF. As a result, I can see
> some real life issues.
>
> The first one is the use of debconf/frontend. At every GO call, cdebconf
> checks its value, and if it's different from the current frontend, tries
> to load it. Since the default returned value is Dialog, cdebconf crashes
> after loading debconf.templates. Is the value actually set and used by
> d-i, is this a bit of unused code (couldn't find any template for it
> other than in debconf), should it be changed to "cdebconf/frontend" ?
> I'm seriously considering the later, but would like feedback first. I'm
> also considering reworking it to be more robbust ; so that when cdebconf
> fails to load a new frontend, it reverts to to previous one instead of
> crashing. I'm also wondering if, when calling frontend_new(), the value
> from the database should be checked and take precedence over the default
> from the config file.
>
> Any comment on this ?
>
> Regis
>
>
>From f8db54347ccb9a55a9a280bd736ca240781d7a01 Mon Sep 17 00:00:00 2001
From: Regis Boudin <regis@boudin.name>
Date: Wed, 20 Jul 2011 23:50:22 +0100
Subject: [PATCH 1/2] Rework frontend_new and frontend reloading
frontend_new() now tries to load modules according to
the following precedence: * DEBIAN_FRONTEND
environment variable. * The value passed as command
line parameter. * The default frontend from the
configuration file. In case of failure to load, it
falls back to the next one.
Frontend reloading when modified is handled differently:
When debconf/frontent is set, DEBIAN_FRONTEND is set accordingly, to take
precedence over the value that was passed at the launch.
If loading and initialising the new module fails, reinitialise the previously
running one.
---
src/commands.c | 54 ++++++++++++++++++++++++++-------------
src/frontend.c | 76 +++++++++++++++++++++++++++++++++++--------------------
2 files changed, 84 insertions(+), 46 deletions(-)
diff --git a/src/commands.c b/src/commands.c
index ad3e2b2..491ffd0 100644
--- a/src/commands.c
+++ b/src/commands.c
@@ -195,28 +195,41 @@ command_go(struct confmodule *mod, char *arg)
char *out;
const char *running_frontend = NULL;
const char *requested_frontend = NULL;
- struct question *q;
int ret;
argc = strcmdsplit(arg, argv, DIM(argv) - 1);
CHECKARGC(== 0);
- q = mod->questions->methods.get(mod->questions, "debconf/frontend");
- if (q)
- requested_frontend = question_getvalue(q, "C");
- question_deref(q);
+ requested_frontend = getenv("DEBIAN_FRONTEND");
- running_frontend = getenv("DEBIAN_FRONTEND");
+ if (requested_frontend)
+ {
+ if (mod != NULL && mod->frontend != NULL)
+ running_frontend = mod->frontend->name;
- if (requested_frontend && strcmp(running_frontend, requested_frontend) != 0) {
- q = mod->frontend->questions;
- mod->frontend->methods.shutdown(mod->frontend);
- if (mod->frontend->handle != NULL)
- dlclose(mod->frontend->handle);
- DELETE(mod->frontend);
- setenv("DEBIAN_FRONTEND",requested_frontend,1);
- mod->frontend = frontend_new(mod->config, mod->templates, mod->questions);
- mod->frontend->questions = q;
+ if (requested_frontend && running_frontend &&
+ strcmp(running_frontend, requested_frontend) != 0)
+ {
+ struct frontend *old_frontend;
+ struct frontend *new_frontend;
+
+ /* Shut down the current frontend, but do not close unless opening
+ the new one actually succeeds */
+ mod->frontend->methods.shutdown(mod->frontend);
+
+ new_frontend = frontend_new(mod->config, mod->templates, mod->questions);
+ if (new_frontend)
+ {
+ old_frontend = mod->frontend;
+ mod->frontend = new_frontend;
+ frontend_delete(old_frontend);
+ }
+ else
+ {
+ /* If the new frontend failed, reinitialise the old one */
+ mod->frontend->methods.initialize(mod->frontend, mod->config);
+ }
+ }
}
mod->frontend->methods.go_noninteractive(mod->frontend);
@@ -295,7 +308,7 @@ command_set(struct confmodule *mod, char *arg)
asprintf(&out, "%u value set", CMDSTATUS_SUCCESS);
if (0 == strcmp("debconf/language", argv[0]))
{ /* Pass the value on to getlanguage() in templates.c */
- debug_printf(0, "Setting debconf/language to %s", argv[1]);
+ debug_printf(0, "Setting %s to %s", argv[0], argv[1]);
setenv("LANGUAGE", argv[1], 1);
/* Reloading takes a little while, so only do it when it's
* really necessary.
@@ -307,9 +320,14 @@ command_set(struct confmodule *mod, char *arg)
}
else if (strcmp(argv[0], "debconf/priority") == 0)
{
- debug_printf(0, "Setting debconf/priority to %s", argv[1]);
+ debug_printf(0, "Setting %s to %s", argv[0], argv[1]);
setenv("DEBIAN_PRIORITY", argv[1], 1);
}
+ else if (strcmp(argv[0], "debconf/frontend") == 0)
+ {
+ debug_printf(0, "Setting %s to %s", argv[0], argv[1]);
+ setenv("DEBIAN_FRONTEND", argv[1], 1);
+ }
}
else
asprintf(&out, "%u cannot set value", CMDSTATUS_INTERNALERROR);
diff --git a/src/frontend.c b/src/frontend.c
index ec242aa..18b37e0 100644
--- a/src/frontend.c
+++ b/src/frontend.c
@@ -230,19 +230,56 @@ static void frontend_progress_stop(struct frontend *ui)
{
}
+static struct frontend_module *frontend_load_module(struct configuration *cfg, const char *modpath, const char *modname, void **dlh)
+{
+ char tmp[256];
+ *dlh = NULL;
+ struct frontend_module *mod;
+
+ if ((modname == NULL) || (modpath == NULL))
+ return NULL;
+
+ snprintf(tmp, sizeof(tmp), "%s/%s.so", modpath, modname);
+ //Frontend switching works when dlopening with RTLD_LAZY
+ //The real reason why it segfaultes with RTLD_NOW has yet to be found
+ dlh = dlopen(tmp, RTLD_NOW | RTLD_GLOBAL);
+ if (dlh == NULL)
+ {
+ INFO(INFO_DEBUG, "Cannot load frontend module %s: %s", tmp, dlerror());
+ return NULL;
+ }
+
+ mod = (struct frontend_module *)dlsym(dlh, "debconf_frontend_module");
+ if (mod == NULL)
+ {
+ dlclose(dlh);
+ INFO(INFO_DEBUG, "Malformed frontend module %s", modname);
+ return NULL;
+ }
+
+ return mod;
+}
+
struct frontend *frontend_new(struct configuration *cfg, struct template_db *tdb, struct question_db *qdb)
{
struct frontend *obj = NULL;
void *dlh = NULL;
- struct frontend_module *mod;
+ struct frontend_module *mod = NULL;
char tmp[256];
const char *modpath, *modname;
- struct question *q;
+
+ modpath = cfg->get(cfg, "global::module_path::frontend", 0);
+ if (modpath == NULL)
+ return NULL;
modname = getenv("DEBIAN_FRONTEND");
- if (modname == NULL)
+ mod = frontend_load_module(cfg, modpath, modname, &dlh);
+ if (mod == NULL)
+ {
modname = cfg->get(cfg, "_cmdline::frontend", 0);
- if (modname == NULL)
+ mod = frontend_load_module(cfg, modpath, modname, &dlh);
+ }
+ if (mod == NULL)
{
modname = cfg->get(cfg, "global::default::frontend", 0);
if (modname == NULL)
@@ -251,35 +288,17 @@ struct frontend *frontend_new(struct configuration *cfg, struct template_db *tdb
snprintf(tmp, sizeof(tmp), "frontend::instance::%s::driver",
modname);
modname = cfg->get(cfg, tmp, 0);
+ mod = frontend_load_module(cfg, modpath, modname, &dlh);
+ }
+ if (mod == NULL && strcmp(modname, "none") != 0 && strcmp(modname, "noninteractive") != 0)
+ {
+ return NULL;
}
- if (modname == NULL)
- DIE("Frontend instance driver not defined (%s)", tmp);
- setenv("DEBIAN_FRONTEND", modname, 1);
obj = NEW(struct frontend);
memset(obj, 0, sizeof(struct frontend));
+ memcpy(&obj->methods, mod, sizeof(struct frontend_module));
- modpath = cfg->get(cfg, "global::module_path::frontend", 0);
- if (modpath == NULL)
- DIE("Frontend module path not defined (global::module_path::frontend)");
-
- if (strcmp(modname, "none") != 0 && strcmp(modname, "noninteractive") != 0)
- {
- q = qdb->methods.get(qdb, "debconf/frontend");
- if (q)
- question_setvalue(q, modname);
- question_deref(q);
- snprintf(tmp, sizeof(tmp), "%s/%s.so", modpath, modname);
- //Frontend switching works when dlopening with RTLD_LAZY
- //The real reason why it segfaultes with RTLD_NOW has yet to be found
- if ((dlh = dlopen(tmp, RTLD_NOW | RTLD_GLOBAL)) == NULL)
- DIE("Cannot load frontend module %s: %s", tmp, dlerror());
-
- if ((mod = (struct frontend_module *)dlsym(dlh, "debconf_frontend_module")) == NULL)
- DIE("Malformed frontend module %s", modname);
-
- memcpy(&obj->methods, mod, sizeof(struct frontend_module));
- }
obj->name = strdup(modname);
obj->handle = dlh;
obj->config = cfg;
@@ -289,6 +308,7 @@ struct frontend *frontend_new(struct configuration *cfg, struct template_db *tdb
snprintf(obj->configpath, sizeof(obj->configpath),
"frontend::instance::%s", modname);
+ modpath = cfg->get(cfg, "global::module_path::frontend", 0);
if (asprintf(&obj->plugin_path, "%s/%s", modpath, modname) == -1) {
frontend_delete(obj);
return NULL;
--
1.7.6
>From e937d043205d20251046f2e75eea5f6980b69dfe Mon Sep 17 00:00:00 2001
From: Regis Boudin <regis@boudin.name>
Date: Wed, 27 Jul 2011 23:49:54 +0100
Subject: [PATCH 2/2] Check debconf/frontend before the configuration file
Conflicts:
src/frontend.c
---
src/frontend.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/src/frontend.c b/src/frontend.c
index 18b37e0..3b9ccd1 100644
--- a/src/frontend.c
+++ b/src/frontend.c
@@ -267,6 +267,7 @@ struct frontend *frontend_new(struct configuration *cfg, struct template_db *tdb
struct frontend_module *mod = NULL;
char tmp[256];
const char *modpath, *modname;
+ struct question *q;
modpath = cfg->get(cfg, "global::module_path::frontend", 0);
if (modpath == NULL)
@@ -279,6 +280,14 @@ struct frontend *frontend_new(struct configuration *cfg, struct template_db *tdb
modname = cfg->get(cfg, "_cmdline::frontend", 0);
mod = frontend_load_module(cfg, modpath, modname, &dlh);
}
+ if (mod == NULL)
+ {
+ q = qdb->methods.get(qdb, "debconf/frontend");
+ if (q)
+ modname = question_getvalue(q, "C");
+ question_deref(q);
+ mod = frontend_load_module(cfg, modpath, modname, &dlh);
+ }
if (mod == NULL)
{
modname = cfg->get(cfg, "global::default::frontend", 0);
--
1.7.6
Reply to: