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

virtualbox: add warning for ineffective nixpkgs config #56322

Merged
merged 1 commit into from Mar 8, 2019

Conversation

bendlas
Copy link
Contributor

@bendlas bendlas commented Feb 24, 2019

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.

@bendlas bendlas changed the title virtualbox: add warning when for ineffective nixpkgs config virtualbox: add warning for ineffective nixpkgs config Feb 24, 2019
@bendlas bendlas requested a review from flokli February 24, 2019 22:23
@bendlas
Copy link
Contributor Author

bendlas commented Feb 24, 2019

Awesome cc #56227

Copy link
Member

@infinisil infinisil left a 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'";
  };
}

@bendlas bendlas requested a review from infinisil March 6, 2019 00:41
@bendlas bendlas added the 9.needs: port to stable A PR needs a backport to the stable release. label Mar 6, 2019
@bendlas bendlas added this to the 19.03 milestone Mar 6, 2019
@infinisil
Copy link
Member

Oh and one more thing, by convention, the commit message should have the nixos/virtualbox prefix

nixpkgs.config.virtualbox.enableExtensionPack doesn't do anything, but
used to. Add a warning for the unsuspecting.
Copy link
Contributor

@flokli flokli left a 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

@infinisil infinisil merged commit 70ed39d into NixOS:master Mar 8, 2019
@infinisil
Copy link
Member

Cherry-picked to 19.03 in 35ee677

@flokli
Copy link
Contributor

flokli commented Mar 8, 2019

@infinisil this is the case for 18.09 too - shouldn't hurt to cherry-pick it there, too :-)

@infinisil
Copy link
Member

Done in 37694c8 :)

@infinisil
Copy link
Member

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.

@infinisil
Copy link
Member

Was fixed on master in 3671047

Just backported to stable in 28c3ecb

@samueldr samueldr removed the 9.needs: port to stable A PR needs a backport to the stable release. label Apr 17, 2019
@bendlas bendlas deleted the warn-virtualbox-config branch October 22, 2023 02:04
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

6 participants