Re: Extending libdebian-installer's API to include certainty on subarch detection
On Mon, Sep 06, 2010 at 12:22:03PM +0000, Colin Watson wrote:
> On Tue, Aug 24, 2010 at 11:02:53PM +0800, Michael Casadevall wrote:
> > I've recently been doing some work on improving sub-architecture detection
> > support to be more generic. I currently have several ARM platforms (Marvell
> > Dove, and TI OMAP3 + OMAP4) which have common kernels which work across
> > multiple boards. I have found the explicit whitelist/board name to subarch
> > we currently use for subarch detection on armel to be insufficent for my
> > needs, and I'd like to propose extending the libdebian-installer API with
> > the following function to the API:
> >
> > const char *di_system_subarch_analyze_with_certainity(int * certainity)
>
> Let's ensure that new interfaces are spelled correctly! It's
> "certainty".
>
> > This function behaves just like the existing di_system_subarch_analyze()
> > function, expect the passed pointer returns a subarch specific level of
> > certainity. This certianity is availability by the API, or returned as the
> > exit code of archdetect (and thus available to base-insatller and
> > flash-kernel-installer).
>
> The problem with using archdetect's exit code is that Unix exit codes
> are either success or failure, and there's only one success code. "We
> think we have a reasonable guess" is sort of successful, and making it a
> failure is confusing. I think it would be better to have archdetect
> fail if the board isn't fully-supported, and have something like
> 'archdetect --guess' to ask it to make a best guess.
>
> (Yes, this would require all callers to be changed, but any 'set -e'
> caller would need to be changed anyway.)
>
> This would flow through into the libd-i ABI, so instead of
> di_system_subarch_analyze_with_certainty you might have:
>
> /**
> * As di_system_subarch_analyze, but tries to make a reasonable guess
> * if the system isn't explicitly supported.
> */
> const char *di_system_subarch_analyze_guess (void);
>
> Does that make sense?
>
Perfectly. I've reworked my patches to implement these suggestions, here's my
current result:
=== modified file 'include/debian-installer/system/subarch.h'
--- include/debian-installer/system/subarch.h 2005-08-03 09:42:05 +0000
+++ include/debian-installer/system/subarch.h 2010-09-07 08:09:28 +0000
@@ -24,6 +24,10 @@
#ifndef DEBIAN_INSTALLER__SYSTEM__SUBARCH_H
#define DEBIAN_INSTALLER__SYSTEM__SUBARCH_H
+#ifdef __ARMEL__
+#define SUBARCH_CAN_GUESS 1
+#endif
/**
* @addtogroup di_system_subarch
* @{
@@ -35,5 +39,13 @@
*/
const char *di_system_subarch_analyze (void);
+/**
+ * Return a string with a best-guess of the current subarchitecture
+ *
+ * Only present on armel currently, and is a stub on all other subarchs
+ */
+
+const char *di_system_subarch_analyze_guess(void);
+
/** @} */
#endif
=== modified file 'src/libdebian-installer.ver'
--- src/libdebian-installer.ver 2006-09-30 15:06:52 +0000
+++ src/libdebian-installer.ver 2010-09-07 08:03:43 +0000
@@ -146,6 +146,7 @@
LIBDI_4.6 {
global:
+ di_system_subarch_analyze_guess;
di_tree_destroy;
di_tree_foreach;
di_tree_insert;
=== modified file 'src/system/subarch-arm-linux.c'
--- src/system/subarch-arm-linux.c 2010-05-06 14:47:46 +0000
+++ src/system/subarch-arm-linux.c 2010-09-07 08:07:34 +0000
@@ -3,6 +3,7 @@
#include <stdio.h>
#include <string.h>
#include <strings.h>
+#include <sys/utsname.h>
#include <debian-installer/system/subarch.h>
@@ -11,6 +12,13 @@
char *ret;
};
+static const char *supported_generic_subarches[] = {
+ "dove",
+ "omap",
+ "omap4",
+ NULL
+};
+
static struct map map_hardware[] = {
{ "Acorn-RiscPC" , "rpc" },
{ "EBSA285" , "netwinder" },
@@ -102,3 +110,27 @@
return "unknown";
}
+
+const char *di_system_subarch_analyze_guess (void)
+{
+ struct utsname sysinfo;
+ int uname_release_len, i;
+
+ /* Attempt to determine subarch based on kernel release version */
+ uname(&sysinfo);
+ uname_release_len = strlen(sysinfo.release);
+
+ for (i = 0; supported_generic_subarches[i] != NULL; i++)
+ {
+ int subarch_len = strlen (supported_generic_subarches[i]);
+ if (!strncmp(sysinfo.release+uname_release_len-subarch_len,
+ supported_generic_subarches[i],
+ subarch_len))
+ {
+ return supported_generic_subarches[i];
+ }
+ }
+
+ /* If we get here, try falling back on the normal detection method */
+ return di_system_subarch_analyze();
+}
=== modified file 'src/system/utils.c'
--- src/system/utils.c 2005-07-31 16:10:46 +0000
+++ src/system/utils.c 2010-09-07 08:09:05 +0000
@@ -22,6 +22,7 @@
#include <config.h>
+#include <debian-installer/system/subarch.h>
#include <debian-installer/system/utils.h>
#include <debian-installer/log.h>
@@ -36,4 +37,16 @@
di_log_set_handler (DI_LOG_LEVEL_MASK, di_log_handler_syslog, NULL);
}
-
+#ifndef SUBARCH_CAN_GUESS
+
+/**
+ * HACK: If there's a better way to do this, we should probably use that
+ * instead of thus stub function for non armel archs
+ */
+
+const char *di_system_subarch_analyze_guess (void)
+{
+ return di_system_subarch_analyze();
+}
+
+#endif
and for hw-detect:
=== modified file 'archdetect.c'
--- archdetect.c 2009-07-23 15:40:25 +0000
+++ archdetect.c 2010-09-07 09:44:50 +0000
@@ -1,23 +1,47 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <getopt.h>
#include <debian-installer/macros.h>
#include <debian-installer/system/subarch.h>
-#if DI_GNUC_PREREQ(2,4)
-# define ATTRIBUTE_UNUSED __attribute__((__unused__))
-#else
-# define ATTRIBUTE_UNUSED
-#endif
+static struct option long_options[] =
+{
+ {"guess", 0, 0, 0},
+ {0, 0, 0,0}
+};
-int main(int argc ATTRIBUTE_UNUSED, char *argv[] ATTRIBUTE_UNUSED)
+int main(int argc, char *argv[])
{
const char *subarch;
+ while (1)
+ {
+ int option_index = 0;
+ int c;
+
+ c = getopt_long (argc, argv, "g", long_options,
+ &option_index);
+ if (c == -1) break;
+
+ switch (c)
+ {
+ case 'g':
+ if (!(subarch = di_system_subarch_analyze_guess()))
+ return EXIT_FAILURE;
+
+ goto print_subarch;
+ break;
+ default:
+ continue;
+ }
+ }
+
if (!(subarch = di_system_subarch_analyze()))
return EXIT_FAILURE;
+print_subarch:
printf("%s/%s\n", CPU_TEXT, subarch);
return 0;
> --
> Colin Watson [cjwatson@debian.org]
Reply to: