Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/sd-image: explicit reference to the gawk package #106751

Merged
merged 1 commit into from Dec 12, 2020

Conversation

urbas
Copy link
Contributor

@urbas urbas commented Dec 12, 2020

Motivation for this change

The awk command is not installed in the standard env. So this command fails if the awk command is not installed by some external module.

This is the error you'll get if you try to boot an sd-image built with the sd-image module (without the base profile):

Jan 01 00:00:01 pi1 stage-2-init: ++ awk -F: '{print $2}'
Jan 01 00:00:01 pi1 stage-2-init: ++ lsblk -npo MAJ:MIN /dev/disk/by-label/NIXOS_SD
Jan 01 00:00:01 pi1 stage-2-init: /nix/store/q8layiggpgj0cgf7yvjh9xxbdyyyi054-local-cmds: line 8: awk: command not found
Things done

Built NixOS with this change and verified that the post-boot script successfully ran. I verified this by checking that the /nix-path-registration file has been deleted by the post-boot script to mark a successful completion (it wasn't deleted before this change).

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

cc: @Mic92 @samueldr @SuperSandro2000

The `awk` command is not installed in the standard env. So this command fails if the `awk` command is not installed by some external module.
@Mic92 Mic92 merged commit 95042a5 into NixOS:master Dec 12, 2020
@urbas urbas deleted the sd-image-first-boot-awk-missing branch December 12, 2020 19:29
@samueldr
Copy link
Member

samueldr commented Jan 4, 2021

Was it figured out why awk was previously available, but isn't anymore? it seems that whatever broke this also broke it on 20.09. (now backporting the change)

[release-20.09 c5c6009fb43] nixos/sd-image: explicit reference to the gawk package
 Author: Matej Urbas <matej.urbas@gmail.com>
 Date: Sat Dec 12 15:23:04 2020 +0000
 1 file changed, 1 insertion(+), 1 deletion(-)

@urbas
Copy link
Contributor Author

urbas commented Jan 4, 2021

Was it figured out why awk was previously available, but isn't anymore? it seems that whatever broke this also broke it on 20.09. (now backporting the change)

[release-20.09 c5c6009fb43] nixos/sd-image: explicit reference to the gawk package
 Author: Matej Urbas <matej.urbas@gmail.com>
 Date: Sat Dec 12 15:23:04 2020 +0000
 1 file changed, 1 insertion(+), 1 deletion(-)

The previous change (made 12 days before I hit this bug) introduced awk into the script (there was no awk before that).

I hit this bug when I was building a minimal image. The "regular"[1] image didn't have this bug. This implies that the awk package must have been placed into the PATH by some module that is imported in the regular image but not in my minimal one.

I suppose this bug hasn't been spotted before because not many people build minimal images and we have no tests for it. Perhaps it would be a good test to add? If you could point me to an example test I could use for inspiration then I could try and add one.

[1] By "regular" image I mean nixos/modules/installer/cd-dvd/sd-image-aarch64.nix, which imports ../../profiles/base.nix and ../../profiles/installation-device.nix (both of which I removed from my "minimal" configuration).

@samueldr
Copy link
Member

samueldr commented Jan 4, 2021

Oh, I missed that change, even though I tried to look for changes including awk.

And yeah, we'd need a test for this, but it's not really trivial to test with the way it's currently made.

We'd need the test to run a QEMU VM using u-boot for QEMU, and which also would validate that the size changed for the partition. Since, as you can see, even if we booted that machine it wouldn't have caught this regression!

Though you say sd-image-aarch64didn't have this bug, I've seen this be an issue with sd-image-aarch64 for 20.09. Odd.

@urbas
Copy link
Contributor Author

urbas commented Jan 4, 2021

Now that you mention it, I'm actually not sure anymore that sd-image-aarch64 didn't have this bug. I might have built an older revision. Please disregard my comment for now. I'll try to reproduce it when I get a chance and I'll comment here.

@samueldr
Copy link
Member

samueldr commented Jan 5, 2021

I don't think we need a reproducer, at this point. I simply missed the (quite obvious) commit when looking for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants