Makefile.common: use system sha256sum if available

[lschuermann]

Pull Request Overview

Use the sha256sum binary from coreutils/busybox/... if installed.

Somehow the compilation of the custom shipped sha256sum binary often fails for me, chooses the wrong interpreter or does other weird stuff. Often deleting the respective target/ directory helps, sometimes "uninstalling" rustup (I'm on Nix, so I just leave the nix-shell) helps. In any case this is annoying and since a lot of systems have this preinstalled (almost Linux distributions I'm aware of), we can avoid building our custom application.

Going further

There's no technical reason to not do what we currently do, but building our own replacement for something available on virtually every OS's package manager - which could instead be specified as an (optional) build dependency - feels over-engineered. It makes it our responsibility that the produced sha256sum binary is bug-free and secure and we must ensure the build process works.

Even #1669 mentioned that this isn't mandatory for the build. For the reasons outlined above, I would propose to use the binary if present, specify it as an optional build-time dependency and only run it when it's present.

Tagging @gendx @bradjc, since they were involved #1669.

Testing Strategy

Building a board with sha256sum in $PATH and excluded from $PATH.

Documentation Updated

• Updated the relevant files in /docs, or no updates are required.

Formatting

• Ran make prepush.

[ppannuto]

I would like to understand why the build is failing, but I suppose I don't mind using the system copy -- having different implementations for this is actually sort of reasonable given what it's checking

[gendx]

The pull request that added that originally used the system sha256, but then it was noted in #1669 (review) that this could break the Makefile. So putting back in the Makefile a check for whether a system binary exists could go back to square one of breaking the Makefile.

I'm also quite surprised that the current setup is broken for you, as it should be self-contained and the binary is relatively simple. What error do you get?

[lschuermann]

but then it was noted in #1669 (review) that this could break the Makefile

Shoot, I misread @bradjc's review as having no binary at all, instead of a different one with other options.

Nonetheless, this proposed approach shouldn't break anything, as it first uses the system sha256sum, but also falls back onto the custom one - after all the issue was the SHA sum binary being an entirely different one. Are you aware of a SHA-256 program being named sha256sum, but incompatible with the way we're using it? In any case, I think that we could easily support the coreutils/busybox one along with the macOS one - this is only an optional indicator for reproducibility right? The current setup goes to great lengths just to have that hash printed at the end.

I would like to understand why the build is failing

Me as well. I'm not currently aware under what circumstances this actually produces a broken binary, but I suspect it being the result of a combination of the shell.nix build environment together with rustup (which unfortunately is required for some tasks in the current Makefile setup). I'll analyze further once I next hit this error.

It's someting along the lines of no such file or directory when executing the binary - even though it's there, and if I remember correctly is (a) for the correct architecture and (b) has the right ld path set. Note that in that exact same shell I can build other programs with cargo correctly and Tock itself builds perfectly fine as well.

[gendx]

It's someting along the lines of no such file or directory when executing the binary - even though it's there, and if I remember correctly is (a) for the correct architecture and (b) has the right ld path set. Note that in that exact same shell I can build other programs with cargo correctly and Tock itself builds perfectly fine as well.

Maybe instead of invoking the binary directly as $(TOCK_ROOT_DIRECTORY)tools/sha256sum/target/debug/sha256sum <options>, we should do cargo run -- <options>, which should be independent of the platform.

[bradjc]

Should set variable for tool name rather than define different rules.

[lschuermann]

I changed it to work using a conditional variable.

However having a conditionally set prerequisite for a make rule gave me a hard time, so applying @gendx's recommendation of cargo run solved that, since it'll automatically verify that the target binary is indeed there, current, and for the right platform (maybe also ensuring the right linker path which can change on NixOS? we'll see). I'm fairly confident that this will work - still slightly objected to shipping a custom sha256sum, but ultimately that's fine with me.

As per @ppannuto's recommendation to use command instead of which: though command has a more consistent behavior, it is actually a bash builtin, and we don't seem to be using bash to run the shell commands in the Makefile. I found another approach to be working which is calling the binary and checking whether it errors. This should be very portable, probably also work in Cygwin.

I can also add checks for the shasum -a 256 command if desired. I don't have access to a Mac, but it appears to behave identical to the FreeBSD one where I'd test it.

[gendx]

This is fine by me, although given that we have the cargo run based fallback anyway, I'm not convinced that we should attempt to check for a system binary at all (which is biased towards some specific OS anyway).

[ppannuto]

bors r+

I guess I don't really see the issue of using the cargo version, but also don't really see a lot of harm in this. It's pretty contained and seems to work.

[bors]

Build succeeded:

• ci-build (ubuntu-latest)
• ci-build (macos-latest)
• ci-qemu
• ci-tests (macos-latest)
• ci-tests (ubuntu-latest)
• ci-format (ubuntu-latest)
• ci-format (macos-latest)