Skip to content

Enable fixes for W20 repeated keys - group them into attribute sets#163

Open
jakob1379 wants to merge 1 commit intooppiliappan:masterfrom
jakob1379:t3code/group-repeated-keys
Open

Enable fixes for W20 repeated keys - group them into attribute sets#163
jakob1379 wants to merge 1 commit intooppiliappan:masterfrom
jakob1379:t3code/group-repeated-keys

Conversation

@jakob1379
Copy link
Copy Markdown

Disclaimer: This has been made in collaboration with t3code, but I have been in the code, testing it thoroughly on various nixpkgs, and other nix repos.


This should solve various issues related to #99 (comment) and the W20 (repeated keys) by finding the repeated keys and group them together:

--- ./nixos/hosts/yoga/hardware-configuration.nix
+++ ./nixos/hosts/yoga/hardware-configuration.nix [fixed]
@@ -10,27 +10,30 @@

{
  imports = [ (modulesPath + "/installer/scan/not-detected.nix") ];

-  boot.initrd.availableKernelModules = [
-    "xhci_pci"
-    "ahci"
-    "usbhid"
-    "usb_storage"
-    "sd_mod"
-    "rtsx_pci_sdmmc"
-  ];
-  boot.initrd.kernelModules = [ ];
-  boot.kernelModules = [ "kvm-intel" ];
-  boot.extraModulePackages = [ ];
+  boot = {
+    initrd = {
+      availableKernelModules = [
+          "xhci_pci"
+          "ahci"
+          "usbhid"
+          "usb_storage"
+          "sd_mod"
+          "rtsx_pci_sdmmc"
+        ];
+      kernelModules = [ ];
+      luks.devices."luks-1ee74338-0c7c-4ef9-8f1e-1a1ef31a45a1".device =
+          "/dev/disk/by-uuid/1ee74338-0c7c-4ef9-8f1e-1a1ef31a45a1";
+    };
+    kernelModules = [ "kvm-intel" ];
+    extraModulePackages = [ ];
+  };

  fileSystems."/" = {
    device = "/dev/disk/by-uuid/8deb8129-8399-49d3-9d97-6aa30abe8a95";
    fsType = "ext4";
  };
-
-  boot.initrd.luks.devices."luks-1ee74338-0c7c-4ef9-8f1e-1a1ef31a45a1".device =
-    "/dev/disk/by-uuid/1ee74338-0c7c-4ef9-8f1e-1a1ef31a45a1";

  fileSystems."/boot" = {
    device = "/dev/disk/by-uuid/31A9-F01B";
    fsType = "vfat";

If there are any funky business you do not like feel free to give feedback as I am still new to coding with Nix :)

Copy link
Copy Markdown
Collaborator

@mightyiam mightyiam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Comment on lines +10 to +11
- foo.bar = 1;
- foo.bar."hello" = 1;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, shouldn't this fail to parse?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Nix itself this seems to be a parsing error. Should we even have such a test expression? It seems that rnix accepts this. Perhaps this is another reason to have a HIR.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I have corrected this in master. Mind rebasing?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, wait, I thought that this was my fork but it's not!

Comment on lines +90 to +93
assert!(
!check_output.contains("Avoid repeated keys in attribute sets"),
"repeated_keys warning remained after fix\nfixed:\n{fixed}\ncheck output:\n{check_output}",
);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind explaining in English what the problem is and what the strategy behind this solution is?

@mightyiam
Copy link
Copy Markdown
Collaborator

Would you mind submitting against https://github.com/molybdenumsoftware/statix ?

@jakob1379
Copy link
Copy Markdown
Author

Would you mind submitting against https://github.com/molybdenumsoftware/statix ?

thank you for the feedback! I will get to it all - but i see you have a fork, why should this be submitted there?

@mightyiam
Copy link
Copy Markdown
Collaborator

I added a little near the top of the fork's readme about why I forked. Still hoping to unfork.

@nyabinary
Copy link
Copy Markdown

I added a little near the top of the fork's readme about why I forked. Still hoping to unfork.

What's stopping you, you can't contact the repo owner or?

@mightyiam
Copy link
Copy Markdown
Collaborator

They seem to be reluctant to accept any of my suggestions towards unforking 😢

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants