Improve Python helper script (#34)

Fixed a lot of issues with argparse, added option for building in release mode,
and documented the new command line arguments.

Clippy support has been added back, and we can now run it in CI.

Author: GabrielMajeri

.travis.yml

@@ -14,6 +14,8 @@ install:
   - cargo -V
   # Required by cargo-xbuild
   - rustup component add rust-src
+  # Required by cargo-xclippy
+  - rustup component add clippy-preview
   # Try installing cargo-xbuild if it's not already installed
   - hash cargo-xbuild || cargo install cargo-xbuild
   # Download OVMF
@@ -25,7 +27,9 @@ before_script:
   - qemu-system-x86_64 --version
 
 script:
-  - cd uefi-test-runner && ./build.py run --headless
+  - cd uefi-test-runner
+  - ./build.py run --headless
+  - ./build.py clippy
 
 cache:
   directories:

src/lib.rs

@@ -30,7 +30,7 @@
 #![no_std]
 // Enable some additional warnings and lints.
 #![warn(missing_docs, unused)]
-#![warn(clippy::all)]
+#![deny(clippy::all)]
 
 mod error;
 pub use self::error::{Result, Status};

uefi-alloc/src/lib.rs

@@ -10,10 +10,11 @@
 
 // Enable additional lints.
 #![warn(missing_docs)]
-#![cfg_attr(feature = "cargo-clippy", warn(clippy))]
+#![deny(clippy::all)]
 #![no_std]
 // Custom allocators are currently unstable.
 #![feature(allocator_api)]
+#![feature(tool_lints)]
 
 use core::alloc::{GlobalAlloc, Layout};
 use core::ptr;

uefi-logger/src/lib.rs

@@ -13,8 +13,9 @@
 //! supported by the UEFI console. Don't expect emoji output support.
 
 #![warn(missing_docs)]
-#![cfg_attr(feature = "cargo-clippy", warn(clippy))]
+#![deny(clippy::all)]
 #![no_std]
+#![feature(tool_lints)]
 
 extern crate uefi;
 use uefi::proto::console::text::Output;

uefi-test-runner/README.md

@@ -28,3 +28,9 @@ Available commands:
 - `run`: (re)build and run
 - `doc`: generate documentation
 - `clippy`: run Clippy
+
+Available options:
+
+- `--verbose`: enables verbose mode, prints commands before running them
+- `--headless`: enables headless mode, which runs QEMU without a GUI
+- `--release`: builds the code with optimizations enabled

uefi-test-runner/build.py

@@ -11,53 +11,80 @@
 import sys
 
 ## Configurable settings
-# Target to build for.
-TARGET = 'x86_64-uefi'
-# Configuration to build.
-CONFIG = 'debug'
-
-# QEMU executable to use
-QEMU = 'qemu-system-x86_64'
-
 # Path to workspace directory (which contains the top-level `Cargo.toml`)
 WORKSPACE_DIR = Path(__file__).resolve().parents[1]
 
-# Path to directory containing `OVMF_{CODE/VARS}.fd`.
-# TODO: use installed OVMF, if available.
-OVMF_DIR = WORKSPACE_DIR / 'uefi-test-runner'
-
-# Set to `True` or use the `--verbose` argument to print commands.
-VERBOSE = False
+# Try changing these with command line flags, where possible
+SETTINGS = {
+    # Print commands before running them.
+    'verbose': False,
+    # Run QEMU without showing GUI
+    'headless': False,
+    # Target to build for.
+    'target': 'x86_64-uefi',
+    # Configuration to build.
+    'config': 'debug',
+    # QEMU executable to use
+    'qemu_binary': 'qemu-system-x86_64',
+    # Path to directory containing `OVMF_{CODE/VARS}.fd`.
+    # TODO: use installed OVMF, if available.
+    'ovmf_dir': WORKSPACE_DIR / 'uefi-test-runner',
+}
+
+def build_dir():
+    'Returns the directory where Cargo places the build artifacts'
+    return WORKSPACE_DIR / 'target' / SETTINGS['target'] / SETTINGS['config']
+
+def esp_dir():
+    'Returns the directory where we will build the emulated UEFI system partition'
+    return build_dir() / 'esp'
+
+def run_xtool(tool, *flags):
+    'Runs cargo-x<tool> with certain arguments.'
+
+    cmd = ['cargo', tool, '--target', SETTINGS['target'], *flags]
+
+    if SETTINGS['verbose']:
+        print(' '.join(cmd))
 
-BUILD_DIR = WORKSPACE_DIR / 'target' / TARGET / CONFIG
-ESP_DIR = BUILD_DIR / 'esp'
+    sp.run(cmd).check_returncode()
 
 def run_xbuild(*flags):
-    'Runs Cargo XBuild with certain arguments.'
-
-    cmd = ['cargo', 'xbuild', '--target', TARGET, *flags]
-
-    if VERBOSE:
-        print(' '.join(cmd))
+    'Runs cargo-xbuild with certain arguments.'
+    run_xtool('xbuild', *flags)
 
-    sp.run(cmd).check_returncode()
+def run_xclippy(*flags):
+    'Runs cargo-xclippy with certain arguments.'
+    run_xtool('xclippy', *flags)
 
 def build(*test_flags):
     'Builds the tests and examples.'
 
-    run_xbuild('--package', 'uefi-test-runner', *test_flags)
-    run_xbuild('--package', 'uefi', '--examples')
+    xbuild_args = [
+        '--package', 'uefi-test-runner',
+        *test_flags,
+    ]
+
+    if SETTINGS['config'] == 'release':
+        xbuild_args.append('--release')
+
+    run_xbuild(*xbuild_args)
 
     # Copy the built test runner file to the right directory for running tests.
-    built_file = BUILD_DIR / 'uefi-test-runner.efi'
+    built_file = build_dir() / 'uefi-test-runner.efi'
 
-    boot_dir = ESP_DIR / 'EFI' / 'Boot'
+    boot_dir = esp_dir() / 'EFI' / 'Boot'
     boot_dir.mkdir(parents=True, exist_ok=True)
 
     output_file = boot_dir / 'BootX64.efi'
 
     shutil.copy2(built_file, output_file)
 
+def clippy():
+    'Runs Clippy on all projects'
+
+    run_xclippy('--all')
+
 def doc():
     'Generates documentation for the library crates.'
     sp.run([
@@ -69,18 +96,19 @@ def doc():
         '--package', 'uefi-services',
     ])
 
-def run_qemu(headless):
+def run_qemu():
     'Runs the code in QEMU.'
 
     # Rebuild all the changes.
     build('--features', 'qemu-f4-exit')
 
-    ovmf_code, ovmf_vars = OVMF_DIR / 'OVMF_CODE.fd', OVMF_DIR / 'OVMF_VARS.fd'
+    ovmf_dir = SETTINGS['ovmf_dir']
+    ovmf_code, ovmf_vars = ovmf_dir / 'OVMF_CODE.fd', ovmf_dir / 'OVMF_VARS.fd'
 
     if not ovmf_code.is_file():
-        raise FileNotFoundError(f'OVMF_CODE.fd not found in the `{OVMF_DIR}` directory')
+        raise FileNotFoundError(f'OVMF_CODE.fd not found in the `{ovmf_dir}` directory')
 
-    examples_dir = BUILD_DIR / 'examples'
+    examples_dir = build_dir() / 'examples'
 
     qemu_flags = [
         # Disable default devices.
@@ -98,7 +126,7 @@ def run_qemu(headless):
         '-drive', f'if=pflash,format=raw,file={ovmf_vars},readonly=on',
 
         # Mount a local directory as a FAT partition.
-        '-drive', f'format=raw,file=fat:rw:{ESP_DIR}',
+        '-drive', f'format=raw,file=fat:rw:{esp_dir()}',
 
         # Mount the built examples directory.
         '-drive', f'format=raw,file=fat:rw:{examples_dir}',
@@ -111,7 +139,7 @@ def run_qemu(headless):
     # When running in headless mode we don't have video, but we can still have
     # QEMU emulate a display and take screenshots from it.
     qemu_flags.extend(['-vga', 'std'])
-    if headless:
+    if SETTINGS['headless']:
         # Do not attach a window to QEMU's display
         qemu_flags.extend(['-display', 'none'])
 
@@ -125,9 +153,9 @@ def run_qemu(headless):
         #'-debugcon', 'file:debug.log', '-global', 'isa-debugcon.iobase=0x402',
     ])
 
-    cmd = [QEMU] + qemu_flags
+    cmd = [SETTINGS['qemu_binary']] + qemu_flags
 
-    if VERBOSE:
+    if SETTINGS['verbose']:
         print(' '.join(cmd))
 
     # This regex can be used to detect and strip ANSI escape codes when
@@ -167,32 +195,36 @@ def main():
 
     parser = argparse.ArgumentParser(usage=usage, description=desc)
 
-    common = argparse.ArgumentParser(add_help=False)
-    common.add_argument('--verbose', '-v', help='print commands before executing them', action='store_true')
-    common.add_argument('--headless', help='run QEMU without a GUI', action='store_true')
+    parser.add_argument('verb', help='command to run', type=str,
+                        choices=['build', 'run', 'doc', 'clippy'])
 
-    subparsers = parser.add_subparsers(dest='verb')
+    parser.add_argument('--verbose', '-v', help='print commands before executing them',
+                        action='store_true')
 
-    build_parser = subparsers.add_parser('build', parents=[common])
-    run_parser = subparsers.add_parser('run', parents=[common])
-    doc_parser = subparsers.add_parser('doc', parents=[common])
+    parser.add_argument('--headless', help='run QEMU without a GUI',
+                        action='store_true')
+
+    parser.add_argument('--release', help='build in release mode',
+                        action='store_true')
 
     opts = parser.parse_args()
 
     # Check if we need to enable verbose mode
-    global VERBOSE
-    VERBOSE = VERBOSE or opts.verbose
-    headless = opts.headless
+    SETTINGS['verbose'] = opts.verbose
+    SETTINGS['headless'] = opts.headless
+    SETTINGS['config'] = 'release' if opts.release else 'debug'
+
+    verb = opts.verb
 
-    if opts.verb == 'build':
+    if verb == 'build':
         build()
-    elif opts.verb == 'run':
-        run_qemu(headless)
-    elif opts.verb == 'doc':
+    elif verb == 'clippy':
+        clippy()
+    elif verb == 'doc':
         doc()
-    elif opts.verb is None or opts.verb == '':
+    elif verb == 'run' or verb is None or opts.verb == '':
         # Run the program, by default.
-        run_qemu(headless)
+        run_qemu()
     else:
         raise ValueError(f'Unknown verb {opts.verb}')
 

uefi-test-runner/src/boot/memory.rs

@@ -4,6 +4,8 @@ use core::mem;
 use crate::alloc::vec::Vec;
 
 pub fn test(bt: &BootServices) {
+    info!("Testing memory functions");
+
     allocate_pages(bt);
     vec_alloc();
     memmove(bt);
@@ -12,6 +14,8 @@ pub fn test(bt: &BootServices) {
 }
 
 fn allocate_pages(bt: &BootServices) {
+    info!("Allocating some pages of memory");
+
     let ty = AllocateType::AnyPages;
     let mem_ty = MemoryType::LoaderData;
     let pgs = bt
@@ -38,6 +42,8 @@ fn allocate_pages(bt: &BootServices) {
 
 // Simple test to ensure our custom allocator works with the `alloc` crate.
 fn vec_alloc() {
+    info!("Allocating a vector through the `alloc` crate");
+
     let mut values = vec![-5, 16, 23, 4, 0];
 
     values.sort();
@@ -47,6 +53,8 @@ fn vec_alloc() {
 
 // Test that the `memmove` / `memset` functions work.
 fn memmove(bt: &BootServices) {
+    info!("Testing the `memmove` / `memset` functions");
+
     let src = [1, 2, 3, 4];
     let mut dest = [0u8; 4];
 
@@ -66,6 +74,8 @@ fn memmove(bt: &BootServices) {
 }
 
 fn memory_map(bt: &BootServices) {
+    info!("Testing memory map functions");
+
     // Get an estimate of the memory map size.
     let map_sz = bt.memory_map_size();
 

uefi-test-runner/src/boot/mod.rs

@@ -1,6 +1,7 @@
 use uefi::table::boot::BootServices;
 
 pub fn test(bt: &BootServices) {
+    info!("Testing boot services");
     memory::test(bt);
     misc::test(bt);
 }

uefi-test-runner/src/main.rs

@@ -19,6 +19,9 @@ pub extern "C" fn uefi_start(_handle: uefi::Handle, st: &'static SystemTable) ->
     // Initialize logging.
     uefi_services::init(st);
 
+    // Reset the console before running all the other tests.
+    st.stdout().reset(false).expect("Failed to reset stdout");
+
     // Ensure the tests are run on a version of UEFI we support.
     check_revision(st.uefi_revision());
 
@@ -38,7 +41,7 @@ pub extern "C" fn uefi_start(_handle: uefi::Handle, st: &'static SystemTable) ->
 fn check_revision(rev: uefi::table::Revision) {
     let (major, minor) = (rev.major(), rev.minor());
 
-    info!("UEFI {}.{}", major, minor);
+    info!("UEFI {}.{}", major, minor / 10);
 
     assert!(major >= 2, "Running on an old, unsupported version of UEFI");
     assert!(

uefi-test-runner/src/proto/console/gop.rs

@@ -3,6 +3,7 @@ use uefi::table::boot::BootServices;
 use uefi_exts::BootServicesExt;
 
 pub fn test(bt: &BootServices) {
+    info!("Running graphics output protocol test");
     if let Some(mut gop_proto) = bt.find_protocol::<GraphicsOutput>() {
         let gop = unsafe { gop_proto.as_mut() };