-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
qemu: enable canokey by default #311914
qemu: enable canokey by default #311914
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with enabling it as default. Diff LGTM.
If the impact on closure size is minimal, I'm for this. |
|
Weird error to only get for a single architecture (x86_64-darwin):
|
I guess maybe on aarch64-darwin's libsystem memcpy isn't a macro |
6b5549c
to
ab94b67
Compare
Given that we were overriding qemu_test to enable this anyway, enabling this by default saves Hydra a QEMU build. There's also clear demand from users[1] for this feature, so our alternatives are: - Offer a qemu-canokey attribute. I don't want to do this, because I don't think there's any reason to make Hydra build an extra QEMU. - Enable it only for qemu_test. I don't want to do this, because it will lead to users using qemu_test without understanding its subtleties. - Force users to build from source. I don't think there's any reason to do this when it's unlikely to hurt anybody having it enabled by default. There's no reason to single out canokey to be disabled by default in spite of users' needs given that we enable so many other optional QEMU features. [1]: canokeys/canokey-qemu#6
@ofborg build qemu.tests |
https://nixpk.gs/pr-tracker.html?pr=311914 Still new to nix so not sure how to track when things are part of nixos-unstable |
Description of changes
Given that we were overriding qemu_test to enable this anyway, enabling this by default saves Hydra a QEMU build.
There's also clear demand from users for this feature, so our alternatives are:
Offer a qemu-canokey attribute. I don't want to do this, because I don't think there's any reason to make Hydra build an extra QEMU.
Enable it only for qemu_test. I don't want to do this, because it will lead to users using qemu_test without understanding its subtleties.
Force users to build from source. I don't think there's any reason to do this when it's unlikely to hurt anybody having it enabled by default. There's no reason to single out canokey to be disabled by default in spite of users' needs given that we enable so many other optional QEMU features.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.