Created attachment 234058 [details] [review]
Add utility function to compare CPU architectures

Created attachment 234059 [details] [review]
unattended-installer: Be less strict about driver arch

This fixes the issue of "i386" drivers not being picked for "i686" OS.

Review of attachment 234058 [details] [review]:

It seems to me like this should use an enum rather than hardcoding the int values.

Review of attachment 234058 [details] [review]:

::: src/installer-media.vala
@@ +96,3 @@
     public bool is_architecture_compatible (string architecture) {
         return os_media == null || // Unknown media
+               compare_cpu_architectures (architecture, os_media.architecture) >= 0;

i.e. this woudld be compare_... () != INCOMPATIBLE

Review of attachment 234058 [details] [review]:

Actually, maybe it could be a enum bitfield?

Review of attachment 234059 [details] [review]:

::: src/unattended-installer.vala
@@ +681,3 @@
+            if (compatibility == 0 || compatibility == 1)
+                // We don't entertain compatiblity when word-size is different because 32-bit drivers
+                // are not guaranteed to work on 64-bit architectures in all OSs.

What about a "modern" (say i686) driver on an old OS? i386? 
With this code that won't be used, but is that really right? Just because an os is marked as "works on i386" doesn't mean it can't handle i686 drivers?

Review of attachment 234059 [details] [review]:

::: src/unattended-installer.vala
@@ +681,3 @@
+            if (compatibility == 0 || compatibility == 1)
+                // We don't entertain compatiblity when word-size is different because 32-bit drivers
+                // are not guaranteed to work on 64-bit architectures in all OSs.

I just added a warning comment to the util function to avoid this misunderstanding:
    // Warning: architecture compability is not computative. e.g "i386" is compatible with "i686" but "i686" is
    // incompatible with "i386".

Created attachment 234141 [details] [review]
Add utility function to compare CPU architectures

v2:

* Add enum for comparison results.
* Handle "i486" and "i586" as well.

Created attachment 234142 [details] [review]
unattended-installer: Be less strict about driver arch

v2: Use enum introduced in v2 of previous patch.

Review of attachment 234141 [details] [review]:

::: src/installer-media.vala
@@ +102,3 @@
+        return compatibility == CPUArchCompatibity.IDENTICAL ||
+               compatibility == CPUArchCompatibity.COMPATIBLE ||
+               compatibility == CPUArchCompatibity.COMPATIBLE_DIFF_WORDSIZE;

This seems a bit large, why not just compatibility != INCOMPATIBLE? not a blocker issue though.

Review of attachment 234142 [details] [review]:

ack

Review of attachment 234141 [details] [review]:

::: src/installer-media.vala
@@ +102,3 @@
+        return compatibility == CPUArchCompatibity.IDENTICAL ||
+               compatibility == CPUArchCompatibity.COMPATIBLE ||
+               compatibility == CPUArchCompatibity.COMPATIBLE_DIFF_WORDSIZE;

I'll change that. Lets agree to forget I made this mistake. :P

Attachment 234141 [details] pushed as fb018a6 - Add utility function to compare CPU architectures
Attachment 234142 [details] pushed as 11cfd62 - unattended-installer: Be less strict about driver arch