oxidecomputer / hubris

A lightweight, memory-protected, message-passing kernel for deeply embedded systems.
Mozilla Public License 2.0
2.96k stars 169 forks source link

missing configuration sections result in confusing error messages at build #482

Closed faithanalog closed 2 years ago

faithanalog commented 2 years ago

This is something I ran into while working on my pinetime port, and I promised I'd submit it as a proper issue. I'm going to be picking on the stm32h7-spi-server driver as a case study here, but this is more a build_util thing.

Problem

This driver's build.rs has two lines which load config that generate confusing errors if the config it wants doesn't exist:

    let task_config = build_util::task_config::<TaskConfig>()?;
    let global_config = build_util::config::<GlobalConfig>()?;

I'll reference demo-stm32h7-nucleo/app-h743.toml. For the first error case, remove the task_config by commenting out these lines:

[tasks.spi_driver.config.spi]
global_config = "spi1"

Loading the task config fails, and you're given the following error message during dist:

error: failed to run custom build command for `drv-stm32h7-spi-server v0.1.0 (/home/artemis/z/hubris/drv/stm32h7-spi-server)`

Caused by:
  process didn't exit successfully: `/home/artemis/z/hubris/target/release/build/drv-stm32h7-spi-server-9f4f9b676721a4da/build-script-build` (exit status: 1)
  --- stdout
  cargo:rustc-cfg=target_board="nucleo-h743zi2"
  cargo:rerun-if-env-changed=HUBRIS_BOARD

  --- stderr
  Error: environment variable not found
Error: failed to build spi_driver

Caused by:
    command failed, see output for details

Causing the second error case is a little more complem, put the task config back, and then remove the global_config by commenting out these lines:

[config.spi.spi1]
controller = 1
fifo_depth = 16

[config.spi.spi1.mux_options.cn7_arduino]
outputs = [
    {port = "A", pins = [5], af = 5}, # sck
    {port = "B", pins = [5], af = 5}, # mosi
]
input = {port = "A", pin = 6, af = 5} # miso

[config.spi.spi1.devices.pins]
mux = "cn7_arduino"
cs = {port = "D", pin = 14}
clock_divider = "DIV16"

You'll get an error like this, which is actually good:

error: failed to run custom build command for `drv-stm32h7-spi-server v0.1.0 (/home/artemis/z/hubris/drv/stm32h7-spi-server)`

Caused by:
  process didn't exit successfully: `/home/artemis/z/hubris/target/release/build/drv-stm32h7-spi-server-9f4f9b676721a4da/build-script-build` (exit status: 1)
  --- stdout
  cargo:rustc-cfg=target_board="nucleo-h743zi2"
  cargo:rerun-if-env-changed=HUBRIS_BOARD
  --- toml for $HUBRIS_TASK_CONFIG ---
  [spi]
  global_config = "spi1"

  cargo:rerun-if-env-changed=HUBRIS_TASK_CONFIG
  --- toml for $HUBRIS_APP_CONFIG ---
  [[i2c.controllers]]
  controller = 2
  [[i2c.controllers.ports.F.pins]]
  pins = [0, 1]
  af = 4
  [net.sockets.echo]
  kind = "udp"
  port = 7

  [net.sockets.echo.owner]
  name = "udpecho"
  notification = 1

  [net.sockets.echo.tx]
  packets = 3
  bytes = 1024

  [net.sockets.echo.rx]
  packets = 3
  bytes = 1024

  --- stderr
  Error: missing field `spi` at line 18 column 1
Error: failed to build spi_driver

Caused by:
    command failed, see output for details

Now, this error message actually tells us that what we're missing is the spi section, so that's great.

The problem comes if you don't have a global [config] section at all. To generate this next error message I commented out the entire global config, as well as all the other tasks that had things in the global config. This let me replicate the exact error scenario I had when bringing SPI up on the pinetime.

error: failed to run custom build command for `drv-stm32h7-spi-server v0.1.0 (/home/artemis/z/hubris/drv/stm32h7-spi-server)`

Caused by:
  process didn't exit successfully: `/home/artemis/z/hubris/target/release/build/drv-stm32h7-spi-server-9f4f9b676721a4da/build-script-build` (exit status: 1)
  --- stdout
  cargo:rustc-cfg=target_board="nucleo-h743zi2"
  cargo:rerun-if-env-changed=HUBRIS_BOARD
  --- toml for $HUBRIS_TASK_CONFIG ---
  [spi]
  global_config = "spi1"

  cargo:rerun-if-env-changed=HUBRIS_TASK_CONFIG

  --- stderr
  Error: environment variable not found
Error: failed to build spi_driver

Caused by:
    command failed, see output for details

Without commenting out the other tasks I get a similar error, just from the i2c task since that's what got built first.

The main thing here is that instead of letting us know that a configuration section is missing and telling us which section it is, it's just giving us Error: environment variable not found on stderr and that's it.

faithanalog commented 2 years ago

I added a PR that implements a draft of what I think a better error report might look like

faithanalog commented 2 years ago

Hey thanks for merging my PR, I'll close this issue out now.