Add nix flakes support #2

Merged
sb10q merged 10 commits from mwojcik/humpback-dds:nix-flakes into master 2022-01-25 10:13:35 +08:00
Member

This was a tougher nut to crack - mostly because of the failing compilation.

Tried updating stm32h7xx-hal but besides some breaking changes, that pulled more newer dependencies, most notably smoltcp == 0.8.0 and I had some issues with embedded-time and all in all would require pretty big refactoring and other changes in code - something for some other time, I think.

The compilation error plaguing mcu for a long time now is actually from the check phase (actual firmware compiles without issues), but I couldn't pinpoint why - weirdly enough just putting cargo test in checkPhase gives error[E0463]: can't find crate for `test` - or I may be screwing something up. So the tests are disabled now - figured it would still be an improvement over previous situation. Rustc version can be actually on par with the others.

Included updated README (preemptively).

This was a tougher nut to crack - mostly because of the failing compilation. Tried updating ``stm32h7xx-hal`` but besides some breaking changes, that pulled more newer dependencies, most notably ``smoltcp == 0.8.0`` and I had some issues with ``embedded-time`` and all in all would require pretty big refactoring and other changes in code - something for some other time, I think. The compilation error plaguing mcu for a long time now is actually from the check phase (actual firmware compiles without issues), but I couldn't pinpoint why - weirdly enough just putting ``cargo test`` in checkPhase gives ``error[E0463]: can't find crate for `test` `` - or I may be screwing something up. So the tests are disabled now - figured it would still be an improvement over previous situation. Rustc version can be actually on par with the others. Included updated README (preemptively).
mwojcik added 4 commits 2022-01-19 16:36:00 +08:00
Owner

Just use the older Rust version where it worked.

Just use the older Rust version where it worked.
sb10q reviewed 2022-01-19 17:22:51 +08:00
Cargo.toml Outdated
@ -15,3 +15,3 @@
nb = "1.0.0"
embedded-nal = "0.1.0"
minimq = { git = "https://github.com/quartiq/minimq.git", branch = "master" }
minimq = { version = "0.1.0", git = "https://github.com/quartiq/minimq.git" }
Owner

I'm surprised that this doesn't come with a corresponding change to Cargo.lock.

I'm surprised that this doesn't come with a corresponding change to Cargo.lock.
Author
Member

Going back to a version that could possibly build and pass tests breaks SaiTLS build instead. I'll try carefully upgrading other packages and getting it to run on newer nightly...

Going back to a version that could possibly build and pass tests breaks SaiTLS build instead. I'll try carefully upgrading other packages and getting it to run on newer nightly...
Owner

That's surprising, I recall it was fine before. Look at the commit history of nix-scripts and see what Rust versions were used.

That's surprising, I recall it was fine before. Look at the commit history of nix-scripts and see what Rust versions were used.
Author
Member

I went through all the rust versions that were used in nix-scripts - SaiTLS breaks with 2020-07-25 or earlier, and with neither everything compiles successfully.

Updating stm32h7xx-hal does not help - updated to 0.9.0 with all the necessary code changes and it still throws this in the checkPhase (again, builds fine):

error[E0512]: cannot transmute between types of different sizes, or dependently-sized types
   --> /build/cargo-vendor-dir/stm32h7xx-hal-0.9.0/src/rng.rs:110:67
    |
110 |                         let bytes: [$type; BATCH_SIZE] = unsafe { mem::transmute(random_word) };
    |                                                                   ^^^^^^^^^^^^^^
...
126 | rng_core!(usize);
    | ----------------- in this macro invocation
    |
    = note: source type: `u32` (32 bits)
    = note: target type: `[usize; 0]` (0 bits)
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

(all of it defined here: 6ce89dffc8/src/rng.rs (L128))

I'll save the changes for later just in case we'd like to bump the library anyway but I'll try one more thing.

I went through all the rust versions that were used in nix-scripts - SaiTLS breaks with 2020-07-25 or earlier, and with neither everything compiles successfully. Updating stm32h7xx-hal does not help - updated to 0.9.0 with all the necessary code changes and it still throws this in the checkPhase (again, builds fine): ``` error[E0512]: cannot transmute between types of different sizes, or dependently-sized types --> /build/cargo-vendor-dir/stm32h7xx-hal-0.9.0/src/rng.rs:110:67 | 110 | let bytes: [$type; BATCH_SIZE] = unsafe { mem::transmute(random_word) }; | ^^^^^^^^^^^^^^ ... 126 | rng_core!(usize); | ----------------- in this macro invocation | = note: source type: `u32` (32 bits) = note: target type: `[usize; 0]` (0 bits) = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) ``` (all of it defined here: https://github.com/stm32-rs/stm32h7xx-hal/blob/6ce89dffc802f807843b43209e92899434754f38/src/rng.rs#L128) I'll save the changes for later just in case we'd like to bump the library anyway but I'll try one more thing.
Member

What about that Rust nightly version in nix/channel-rust-nightly.toml?

What about that Rust nightly version in `nix/channel-rust-nightly.toml`?
Author
Member

What about that Rust nightly version in nix/channel-rust-nightly.toml?

Nope, same error.

Was it building on Hydra ever? That's the only place I see a build script for, but I suspect it's never been used in this form.

I tried also downgrading SaiTLS but this proves to be difficult, since it doesn't specify smoltcp version (until latest commit), cargo update tries to pull the newest version which has some breaking changes.

...

It looks like to fix this particular error, I need to upgrade the stm32h7xx-hal library further: I found the commit that fixes this particular error, possibly: 096ef75d49 (diff-23d109d84268da0b9fe2597eb0287d022bb8087cc8d6f2973b8d7710d31ad223)

> What about that Rust nightly version in `nix/channel-rust-nightly.toml`? Nope, same error. Was it building on Hydra ever? That's the only place I see a build script for, but I suspect it's never been used in this form. I tried also downgrading SaiTLS but this proves to be difficult, since it doesn't specify smoltcp version (until latest commit), cargo update tries to pull the newest version which has some breaking changes. ... It looks like to fix this particular error, I need to upgrade the stm32h7xx-hal library further: I found the commit that fixes this particular error, possibly: https://github.com/stm32-rs/stm32h7xx-hal/commit/096ef75d490ed4cbd78d910dea018e17065d9800#diff-23d109d84268da0b9fe2597eb0287d022bb8087cc8d6f2973b8d7710d31ad223
Author
Member

Now, that I updated the stm32h7xx-hal library to just that commit (...commited a day after 0.9.0 release), looks like the library goes through test fine, but now I get a new set of errors from the humpback-dds firmware on its own.

error[E0432]: unresolved import `rtic::cyccnt`
  --> src/main.rs:23:11
   |
23 | use rtic::cyccnt::{Instant, U32Ext};
   |           ^^^^^^ could not find `cyccnt` in `rtic`

error: duplicate lang item in crate `std` (which `test` depends on): `panic_impl`.
...
error: duplicate lang item in crate `panic_halt`: `panic_impl`.
...
error[E0152]: found duplicate lang item `oom`
  --> src/main.rs:60:1
   |
60 | fn oom(_: Layout) -> ! {
   | 
error[E0599]: no method named `cycles` found for type `{integer}` in the current scope
   --> src/main.rs:290:24
    |
290 |     next_ms += 400_000.cycles();
    |  
...

That made me think that it's never been tested with cargo test in the first place - because yes, these methods are not available on x86_64-linux target platform. And this is regardless of rustc version.

Now, we have Hydra logs from 2021-06~ so we cannot tell exactly when it broke, but I have a strong suspicion with some coincidental proof.

Consider the three commits:
nix-scripts - 2020-10-08: update nixpkgs from 20.03 to 20.09 for hydra mcu builds a9108d5fa3

NixOS/nixpkgs - 2020-05-07: default checkPhase added, instead of it missing by default - post 20.03, but early 20.09 (before alpha)
04248f606f

Humpback was added to Hydra jobs on 2020-09-25: 657d3c1b27

This would mean that Humpback-dds built successfuly when it was first added but I wager it broke after the 20.09 update commit. Unfortunately we don't have the records to prove that it was exactly the case, but that's my guess.

Now, that I updated the ``stm32h7xx-hal`` library to just that commit (...commited a day after 0.9.0 release), looks like the library goes through test fine, but now I get a new set of errors from the humpback-dds firmware on its own. ```shell error[E0432]: unresolved import `rtic::cyccnt` --> src/main.rs:23:11 | 23 | use rtic::cyccnt::{Instant, U32Ext}; | ^^^^^^ could not find `cyccnt` in `rtic` error: duplicate lang item in crate `std` (which `test` depends on): `panic_impl`. ... error: duplicate lang item in crate `panic_halt`: `panic_impl`. ... error[E0152]: found duplicate lang item `oom` --> src/main.rs:60:1 | 60 | fn oom(_: Layout) -> ! { | error[E0599]: no method named `cycles` found for type `{integer}` in the current scope --> src/main.rs:290:24 | 290 | next_ms += 400_000.cycles(); | ... ``` That made me think that it's never been tested with ``cargo test`` in the first place - because yes, these methods are not available on x86_64-linux target platform. And this is regardless of ``rustc`` version. Now, we have Hydra logs from 2021-06~ so we cannot tell exactly when it broke, but I have a strong suspicion with some coincidental proof. Consider the three commits: nix-scripts - 2020-10-08: update nixpkgs from 20.03 to 20.09 for hydra mcu builds https://git.m-labs.hk/M-Labs/nix-scripts/commit/a9108d5fa3f090fb1b52a0ca3d52d8a3ffce7e98 NixOS/nixpkgs - 2020-05-07: default ``checkPhase`` added, instead of it missing by default - post 20.03, but early 20.09 (before alpha) https://github.com/NixOS/nixpkgs/commit/04248f606f4ebfe298bfbfed3680a2fd5dacf796 Humpback was added to Hydra jobs on 2020-09-25: https://git.m-labs.hk/M-Labs/nix-scripts/commit/657d3c1b274d11352809a0e180308ef6da9f431d This would mean that Humpback-dds built successfuly when it was first added but I wager it broke after the 20.09 update commit. Unfortunately we don't have the records to prove that it was exactly the case, but that's my guess.
Owner

Unfortunately we don't have the records to prove that it was exactly the case, but that's my guess.

Actually we have them in a database backup from the old server, I can try to dig that out if that's really important (I think not?).

> Unfortunately we don't have the records to prove that it was exactly the case, but that's my guess. Actually we have them in a database backup from the old server, I can try to dig that out if that's really important (I think not?).
Author
Member

Nah, definitely not important, unless you wanted to confirm exactly when it broke - I think I dug up enough dirt that this shouldn't be necessary :P

Nah, definitely not important, unless you wanted to confirm exactly when it broke - I think I dug up enough dirt that this shouldn't be necessary :P
sb10q reviewed 2022-01-21 18:07:15 +08:00
flake.nix Outdated
@ -0,0 +87,4 @@
buildInputs = with pkgs; [
rustPlatform.rust.rustc
rustPlatform.rust.cargo
openocd dfu-util
Owner

if nix/openocd.nix is no longer necessary, please delete the file.

if nix/openocd.nix is no longer necessary, please delete the file.
Owner

same with the other files in nix/ - I think the whole directory should be deleted.

same with the other files in nix/ - I think the whole directory should be deleted.
sb10q reviewed 2022-01-21 18:07:27 +08:00
shell.nix Outdated
@ -6,4 +0,0 @@
migen = callPackage ./nix/migen.nix {};
openocd = callPackage ./nix/openocd.nix {};
rustPlatform = callPackage ./nix/rustPlatform.nix {};
itm = callPackage ./nix/itm.nix {inherit rustPlatform;};
Owner

What about itm?

What about itm?
sb10q reviewed 2022-01-21 18:07:57 +08:00
shell.nix Outdated
@ -21,4 +0,0 @@
'';
openOCDFlashCustomised = writeShellScriptBin "openocd-flash-customised" ''
python3 flash.py $1 $2 $3 $4
Owner

Use $@ instead

Use $@ instead
sb10q reviewed 2022-01-21 18:08:06 +08:00
shell.nix Outdated
@ -31,4 +0,0 @@
flash write_image flash_config.bin 0x081e0000 bin
reset run
shutdown"
'';
Owner

don't remove?

don't remove?
sb10q reviewed 2022-01-21 18:08:40 +08:00
shell.nix Outdated
@ -42,4 +0,0 @@
pkgs.nextpnr
pkgs.icestorm
pkgs.gdb
pkgs.mosquitto
Owner

don't remove gdb and mosquitto?

don't remove gdb and mosquitto?
sb10q reviewed 2022-01-21 18:10:12 +08:00
shell.nix Outdated
@ -50,4 +0,0 @@
runOpenOcdBlock
openocdFlash
publishMqtt
openOCDFlashCustomised
Owner

we still need the scripts...

we still need the scripts...
mwojcik added 3 commits 2022-01-24 11:32:16 +08:00
Owner

Why do we still have shell.nix? That can all be ported to flakes AFAICT.

Why do we still have shell.nix? That can all be ported to flakes AFAICT.
sb10q reviewed 2022-01-24 11:43:33 +08:00
@ -0,0 +46,4 @@
sha256 = "sha256-UmWI3NOE8Lf8ICHOR8nNpbCP9+g3R8XHRX+nUJsH6pY=";
};
cargoPatches = [ ./itm-cargo-lock.patch ];
Owner

Is this tested? AFAIK you need "${self}/itm-cargo-lock.patch"

Is this tested? AFAIK you need ``"${self}/itm-cargo-lock.patch"``
Author
Member

Either works, but it's the same principle as

flake.nix Line 87 in 97a52e1073
lockFile = ./Cargo.lock;

Either works, but it's the same principle as https://git.m-labs.hk/M-Labs/humpback-dds/src/commit/97a52e10735c8f643272c1d240331a9d4b4c9af7/flake.nix#L87
mwojcik added 2 commits 2022-01-24 11:52:21 +08:00
mwojcik added 1 commit 2022-01-25 10:04:26 +08:00
Author
Member

Tested flashing with Nucleo board.

I think .gitignore change should stay - result is artifact from nix build which is introduced here. In addition there also should be flash_config.bin in there (as it's the artifact from running the flash command) but that's another topic.

One more thing however that may need some clarification: openocd commands refer to the config in the repo, which refers to build artifacts in target/thumbv7em-none-eabihf/release/humpback-dds (output of cargo build --release) - instead of the nix build result - so this may be a tad bit confusing.

Tested flashing with Nucleo board. I think ``.gitignore`` change should stay - ``result`` is artifact from ``nix build`` which is introduced here. In addition there also should be ``flash_config.bin`` in there (as it's the artifact from running the flash command) but that's another topic. One more thing however that may need some clarification: openocd commands refer to the config in the repo, which refers to build artifacts in ``target/thumbv7em-none-eabihf/release/humpback-dds`` (output of ``cargo build --release``) - instead of the nix build result - so this may be a tad bit confusing.
sb10q merged commit aaa29a73ba into master 2022-01-25 10:13:35 +08:00
sb10q referenced this issue from a commit 2022-01-25 10:13:35 +08:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/humpback-dds#2
No description provided.