-
-
Notifications
You must be signed in to change notification settings - Fork 15.5k
virtualbox: add warning for ineffective nixpkgs config #56322
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
Conversation
Awesome cc #56227 |
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.
There's warnings for things like that:
{
# ...
config = {
# ...
warnings = mkIf (config.nixpkgs.config.virtualbox.enableExtensionPack or false)
"'nixpkgs.virtualbox.enableExtensionPack' has no effect, please use 'virtualisation.virtualbox.host.enableExtensionPack'";
};
}
1d21a21
to
805039d
Compare
Oh and one more thing, by convention, the commit message should have the |
nixpkgs.config.virtualbox.enableExtensionPack doesn't do anything, but used to. Add a warning for the unsuspecting.
805039d
to
8b6a38c
Compare
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.
LGTM. About reintroducing, we should get some consensus on #56227
Cherry-picked to 19.03 in 35ee677 |
@infinisil this is the case for 18.09 too - shouldn't hurt to cherry-pick it there, too :-) |
Done in 37694c8 :) |
Ah yeah I made a mistake in showing the warnings code, it's a list of strings, not just a string, I didn't test this, was hoping you'd do it when changing the code. |
nixpkgs.config.virtualbox.enableExtensionPack doesn't do anything, but
used to. Add a warning for the unsuspecting.
Motivation for this change
When testing a virtualbox update, I complained about a non-working expansion-pack, while running afoul of having it misconfigured myself (sorry for that, @ptrhlm)
Things done
Tested positive and negative.
I wonder, whether we should re-introduce the config option, though. I don't like needing os-level configuration for using the expansion pack.