[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

[RFC] choose-mirror rewrite - please review/test



I've spent a couple of days working on choose mirror. The new version has 
quite a few improvements over the current one:

* solid support for installing oldstable
* support for incomplete mirrors (e.g. missing suite symlinks) [1];
  this also means the new version has better support for archive.d.o
* checks validity of *all* suites on the mirror and only display those
  that are supported and valid
* more extensive checks for broken mirrors
* display of both codename and (translated) suite during release selection:
  http://people.debian.org/~fjp/tmp/d-i/choose-mirror/choose-mirror.png
* warning if the default suite is not supported by the mirror (current
  version will happily (silently!) install testing if the stable installer
  is used, but the mirror only has testing and unstable)
* improved dialogs

I'm very happy with the result myself and intend to backport these changes 
for Lenny because of the better support for oldstable and archived 
releases. This should be possible without any template changes.
Possibly they should even be backported for Etch.

I have done fairly extensive testing, but would welcome others to test too. 
A netboot image for i386 is available for testing at:
   http://people.debian.org/~fjp/tmp/d-i/choose-mirror/
Below some suggestions for testing.

One thing still on my ToDo is to test this version with CD installs.

I would also very much appreciate if someone could carefully review the 
attached patch [2] as my C foo is extremely limited. Things to check are:
- memory allocation freeing of variables
- string manipulation
- things that can be done smarter

Please also review the template changes.
Question: is a rewrite of the mirror/suite template needed, because of
- oldstable not mentioned
- dialog now normally lists both codenames and suites, and can sometimes
  list *only* codenames
- not sure about the term "flavors"...
?

Cheers,
FJP

[1] Current version can make install choke on incomplete mirrors.
[2] Note that the attached patch does not include all the changes I've got 
queued up. The attachment includes only the most structural changes (4 out 
of 22 commits; most of the rest is cleanup and minor fixes).

Suggestions for testing. Tests can be abandoned after mirror selection.
1) - boot with 'priority=medium'
   - install as normal; note release selection dialog
2) - boot with 'priority=medium suite?=lenny' (!!! note the "?=")
   - install as normal; note oldstable is now listed
3) - boot with 'priority=medium suite?=sarge' (!!! note the "?=")
   - select a normal mirror
   - note the warning that sarge is not available; answer "yes"
     (if you answer "no", stable/testing/unstable will be listed)
   - select manual mirror entry and enter 'archive.debian.org'
   - note that sarge is now correctly listed sarge (codename only)

diff --git a/packages/choose-mirror/choose-mirror.c b/packages/choose-mirror/choose-mirror.c
index 5ef00c5..f9421a8 100644
--- a/packages/choose-mirror/choose-mirror.c
+++ b/packages/choose-mirror/choose-mirror.c
@@ -27,6 +27,11 @@ int show_progress = 1;
 /* Are we installing from a CD that includes base system packages? */
 static int base_on_cd = 0;
 
+/* Available releases (suite/codename) on the mirror. */
+static struct release_t *releases;
+
+static struct release_t get_release(char *name);
+
 /*
  * Returns a string on the form "DEBCONF_BASE/protocol/supplied". The
  * calling function is responsible for freeing the string afterwards.
@@ -148,6 +153,212 @@ static char *get_default_suite(void) {
 	return suite;
 }
 
+static int validate_codename(char *codename, char *suite) {
+	struct release_t release;
+
+	release = get_release(codename);
+	if (strcmp(release.suite, suite) != 0)
+		release.status &= ~IS_VALID;
+	
+	if (release.name)
+		return release.status;
+	else
+		return -1;
+}
+
+void log_invalid_release(char *name, char *field) {
+	di_log(DI_LOG_LEVEL_WARNING,
+		"broken mirror: invalid %s in Release file for %s", field, name);
+}
+
+/*
+ * Fetch a Release file and extract its Suite and Codename.
+ */
+static struct release_t get_release(char *name) {
+	char *command;
+	FILE *f = NULL;
+	char *hostname, *directory;
+	char line[80];
+	char buf[SUITE_LENGTH];
+	struct release_t release;
+	int cn_status;
+
+	hostname = add_protocol("hostname");
+	debconf_get(debconf, hostname);
+	free(hostname);
+	hostname = strdup(debconf->value);
+	directory = add_protocol("directory");
+	debconf_get(debconf, directory);
+	free(directory);
+	directory = strdup(debconf->value);
+
+	asprintf(&command, "wget -q %s://%s%s/dists/%s/Release -O - | grep -E '^(Suite|Codename):'",
+		 protocol, hostname, directory, name);
+	di_log(DI_LOG_LEVEL_DEBUG, "command: %s", command);
+	f = popen(command, "r");
+	free(command);
+	free(hostname);
+	free(directory);
+
+	release.name = NULL;
+	release.suite = NULL;
+	release.status = 0;
+	if (f != NULL) {
+		while (fgets(line, sizeof(line), f) != NULL) {
+			char *value;
+
+			if (line[strlen(line) - 1] == '\n')
+				line[strlen(line) - 1] = '\0';
+			value = strstr(line, ": ");
+			if (value) {
+				strncpy(buf, value + 2, strlen(value) -2 + 1);
+				buf[strlen(value) - 2 + 1] = '\0';
+				if (strncmp(line, "Codename:", 9) == 0)
+					release.name = strdup(buf);
+				if (strncmp(line, "Suite:", 6) == 0)
+					release.suite = strdup(buf); 
+			}
+		}
+		if (release.name && strcmp(release.name, name) == 0)
+			release.status |= IS_VALID | GET_CODENAME;
+		if (release.suite && strcmp(release.suite, name) == 0)
+			release.status |= IS_VALID | GET_SUITE;
+
+		if ((release.name || release.suite) && !(release.status & IS_VALID))
+			log_invalid_release(name, "Suite or Codename");
+
+		/* Check if release can also be gotten using codename */
+		if ((release.status & IS_VALID) &&
+		    release.name && !(release.status & GET_CODENAME)) {
+			cn_status = validate_codename(release.name, name);
+			if (cn_status >= 0) {
+				if (!(cn_status & IS_VALID)) {
+					release.status &= ~IS_VALID;
+					log_invalid_release(name, "Codename");
+				} else {
+					release.status |= (cn_status & GET_CODENAME);
+				}
+			}
+		}
+
+		/* In case there is no Codename field */
+		if ((release.status & IS_VALID) && ! release.name)
+			release.name = strdup(name);
+
+		// di_log(DI_LOG_LEVEL_DEBUG, "get_release(): %s -> %s:%s (0x%x)",
+		//	name, release.suite, release.name, release.status);
+	}
+
+	pclose(f);
+
+	return release;
+}
+
+static int find_releases(void) {
+	int nbr_suites = sizeof(suites)/SUITE_LENGTH;
+	int i, r = 0;
+	int bad_mirror = 0, have_default = 0;
+	struct release_t release;
+	char *default_suite;
+
+	default_suite = get_default_suite();
+	if (default_suite == NULL)
+		di_log(DI_LOG_LEVEL_ERROR, "no default release specified");
+
+	if (show_progress) {
+		debconf_progress_start(debconf, 0, nbr_suites,
+				       DEBCONF_BASE "checking_title");
+		debconf_progress_info(debconf,
+				      DEBCONF_BASE "checking_download");
+	}
+
+	if (releases)
+		free(releases);
+	releases = malloc(MAXRELEASES * sizeof(struct release_t));
+
+	/* Get releases for all suites */
+	if (! base_on_cd) {
+		for (i=0; i < nbr_suites; i++) {
+			char *suite;
+
+			suite = strdup(suites[i]);
+			release = get_release(suite);
+
+			if (release.status & IS_VALID) {
+				if (strcmp(release.name, default_suite) == 0 ||
+				    strcmp(release.suite, default_suite) == 0) {
+					release.status |= IS_DEFAULT;
+					have_default = 1;
+				}
+				/* Only list oldstable if it's the default */
+				if (strcmp(suite, "oldstable") != 0 ||
+				    (release.status & IS_DEFAULT))
+					releases[r++] = release;
+			} else if (release.name) {
+				bad_mirror = 1;
+				free(suite);
+				break;
+			}
+
+			free(suite);
+			if (show_progress)
+				debconf_progress_step(debconf, 1);
+		}
+		if (! bad_mirror && r == 0)
+			di_log(DI_LOG_LEVEL_INFO, "mirror does not have any suite symlinks");
+	}
+
+	/* Get release using default suite */
+	if (! bad_mirror && (base_on_cd || r == 0)) {
+		release = get_release(default_suite);
+		if (release.status & IS_VALID) {
+			release.status |= IS_DEFAULT;
+			releases[r++] = release;
+		} else {
+			di_log(DI_LOG_LEVEL_WARNING,
+				"mirror does not support the specified release (%s)",
+				default_suite);
+		}
+	}
+
+	/* Mark end of list */
+	releases[r].name = NULL;
+
+	if (show_progress) {
+		debconf_progress_step(debconf, nbr_suites);
+		debconf_progress_stop(debconf);
+	}
+
+	if (r == 0 || bad_mirror) {
+		free(default_suite);
+
+		debconf_input(debconf, "critical", DEBCONF_BASE "bad");
+		if (debconf_go(debconf) == 30)
+			exit(10); /* back up to menu */
+		else
+			return 1; /* back to beginning of questions */
+	}
+
+	if (! base_on_cd && ! have_default) {
+		debconf_subst(debconf, DEBCONF_BASE "no-default",
+			"RELEASE", default_suite);
+		free(default_suite);
+
+		debconf_input(debconf, "critical", DEBCONF_BASE "no-default");
+		if (debconf_go(debconf) == 30) {
+			exit(10); /* back up to menu */
+		} else {
+			debconf_get(debconf, DEBCONF_BASE "no-default");
+			if (strcmp(debconf->value, "false"))
+				return 1; /* back to beginning of questions */
+		}
+	} else {
+		free(default_suite);
+	}
+
+	return 0;
+}
+
 /*
  * Using the current debconf settings for a mirror, figure out which suite
  * to use from the mirror and set mirror/suite.
@@ -466,62 +677,89 @@ static int check_mirror(void) {
 }
 
 static int choose_suite(void) {
+	char **choices_c, **choices, *list;
+	int i, ret;
+	int have_default = 0;
+
+	ret = find_releases();
+	if (ret)
+		return ret;
+
+	choices_c = malloc(MAXRELEASES * sizeof(char*));
+	choices = malloc(MAXRELEASES * sizeof(char*));
+	for (i=0; releases[i].name != NULL; i++) {
+		char *name;
+
+		if (releases[i].status & GET_SUITE)
+			name = strdup(releases[i].suite);
+		else
+			name = strdup(releases[i].name);
+
+		choices_c[i] = strdup(name);
+		if (strcmp(name, releases[i].name) != 0)
+			asprintf(&choices[i], "%s - %s", releases[i].name, name);
+		else
+			choices[i] = strdup(name);
+		if (releases[i].status & IS_DEFAULT) {
+			debconf_set(debconf, DEBCONF_BASE "suite", name);
+			have_default = 1;
+		}
+
+		free(name);
+	}
+
+	choices_c[i] = NULL;
+	list = debconf_list(choices_c);
+	debconf_subst(debconf, DEBCONF_BASE "suite", "CHOICES-C", list);
+	choices[i] = NULL;
+	list = debconf_list(choices);
+	debconf_subst(debconf, DEBCONF_BASE "suite", "CHOICES", list);
+	free(choices_c);
+	free(choices);
+	free(list);
+
 	/* If the base system can be installed from CD, don't allow to
 	 * select a different suite
 	 */
+	if (! have_default)
+		debconf_fset(debconf, DEBCONF_BASE "suite", "seen", "false");
 	if (! base_on_cd)
-		debconf_input(debconf, "medium", DEBCONF_BASE "suite");
+		debconf_input(debconf, have_default ? "medium" : "critical",
+			      DEBCONF_BASE "suite");
+
 	return 0;
 }
 
-/* Get the codename for the selected suite. */
-int get_codename (void) {
-	char *command;
-	FILE *f = NULL;
-	char *hostname, *directory, *suite = NULL;
-	int ret = 1;
-
-	hostname = add_protocol("hostname");
-	debconf_get(debconf, hostname);
-	free(hostname);
-	hostname = strdup(debconf->value);
-	directory = add_protocol("directory");
-	debconf_get(debconf, directory);
-	free(directory);
-	directory = strdup(debconf->value);
+/* Set the codename for the selected suite. */
+int set_codename (void) {
+	char *suite;
+	int i;
 
 	/* As suite has been determined previously, this should not fail */
 	debconf_get(debconf, DEBCONF_BASE "suite");
 	if (strlen(debconf->value) > 0) {
 		suite = strdup(debconf->value);
 
-		asprintf(&command, "wget -q %s://%s%s/dists/%s/Release -O - | grep ^Codename: | cut -d' ' -f 2",
-			 protocol, hostname, directory, suite);
-		di_log(DI_LOG_LEVEL_DEBUG, "command: %s", command);
-		f = popen(command, "r");
-		free(command);
-
-		if (f != NULL) {
-			char buf[SUITE_LENGTH];
-			if (fgets(buf, SUITE_LENGTH - 1, f)) {
-				if (buf[strlen(buf) - 1] == '\n')
-					buf[strlen(buf) - 1] = '\0';
-				debconf_set(debconf, DEBCONF_BASE "codename", buf);
-				di_log(DI_LOG_LEVEL_INFO, "codename set to: %s", buf);
-				ret = 0;
+		for (i=0; releases[i].name != NULL; i++) {
+			if (strcmp(releases[i].name, suite) == 0 ||
+			    strcmp(releases[i].suite, suite) == 0) {
+				char *codename;
+
+				if (releases[i].status & GET_CODENAME)
+					codename = releases[i].name;
+				else
+					codename = releases[i].suite;
+				debconf_set(debconf, DEBCONF_BASE "codename", codename);
+				di_log(DI_LOG_LEVEL_INFO, "codename set to: %s", codename);
+				free(codename);
+				break;
 			}
 		}
-		pclose(f);
-	}
 
-	free(hostname);
-	free(directory);
-	if (suite)
 		free(suite);
+	}
 
-	if (ret != 0)
-		di_log(DI_LOG_LEVEL_ERROR, "Error getting codename");
-	return ret;
+	return 0;
 }
 
 /* Check if the mirror carries the architecture that's being installed. */
@@ -590,9 +828,8 @@ int main (int argc, char **argv) {
 		validate_mirror,
 		choose_proxy,
 		set_proxy,
-		check_mirror,
 		choose_suite,
-		get_codename,
+		set_codename,
 		check_arch,
 		NULL,
 	};
diff --git a/packages/choose-mirror/debian/choose-mirror-bin.templates-in b/packages/choose-mirror/debian/choose-mirror-bin.templates-in
index 6385fc5..169c868 100644
--- a/packages/choose-mirror/debian/choose-mirror-bin.templates-in
+++ b/packages/choose-mirror/debian/choose-mirror-bin.templates-in
@@ -10,14 +10,16 @@ Description: country code or "manual" (for internal use)
 
 Template: mirror/suite
 Type: select
-# :sl2:
-__Choices: stable, testing, unstable
+Choices-C: ${CHOICES-C}
+Choices: ${CHOICES}
 # :sl2:
 _Description: Debian version to install:
  Debian comes in several flavors. Stable is well-tested and rarely changes.
  Unstable is untested and frequently changing. Testing is a middle ground,
  that receives many of the new versions from unstable if they are not too
  buggy.
+ .
+ Only flavors available on the selected mirror are listed.
 
 Template: mirror/codename
 Type: string
@@ -33,12 +35,31 @@ Type: text
 # :sl1:
 _Description: Downloading the Release file...
 
+Template: mirror/no-default
+Type: boolean
+Default: true
+# :sl2:
+_Description: Go back and try a different mirror?
+ The specified (default) Debian version (${RELEASE}) is not available from
+ the selected mirror. It is possible to continue and select a different
+ release for your installation, but normally you should go back and select
+ a different mirror that does support the correct version.
+
 Template: mirror/bad
 Type: error
 # :sl2:
 _Description: Bad archive mirror
- The specified Debian archive mirror is either not available, or does not
- have a valid Release file on it. Please try a different mirror.
+ An error has been detected while trying to use the specified Debian archive
+ mirror.
+ .
+ Possible reasons for the error are: incorrect mirror specified; mirror is
+ not available (possibly due to an unreliable network connection); mirror is
+ broken (for example because an invalid Release file was found); mirror does
+ not support the correct Debian version.
+ .
+ Additional details may be available in /var/log/syslog or on virtual console 4.
+ .
+ Please check the specified mirror or try a different one.
 
 Template: mirror/noarch
 Type: error
diff --git a/packages/choose-mirror/mirrors.h b/packages/choose-mirror/mirrors.h
index 579abd0..22c9068 100644
--- a/packages/choose-mirror/mirrors.h
+++ b/packages/choose-mirror/mirrors.h
@@ -10,16 +10,33 @@ struct mirror_t {
 };
 
 /*
+ * Data structure containing information on releases supported by a mirror
+ */
+struct release_t {
+	char *name;
+	char *suite;
+	int status;
+};
+
+/* Values for status field in release_t */
+#define IS_VALID	0x1
+#define IS_DEFAULT	0x2
+#define GET_SUITE	0x4
+#define GET_CODENAME	0x8
+
+/*
  * The string defined below must match the string used in the templates
  * (http and ftp) for this option.
  */
 #define MANUAL_ENTRY "manual"
 
 #define SUITE_LENGTH 32
+#define MAXRELEASES 6
 
 /* Stack of suites */
 static const char suites[][SUITE_LENGTH] = {
 	/* higher preference */
+	"oldstable",
 	"stable",
 	"testing",
 	"unstable"

Attachment: signature.asc
Description: This is a digitally signed message part.


Reply to: