disko icon indicating copy to clipboard operation
disko copied to clipboard

refac(examples): Denest the source

Open goyalyashpal opened this issue 1 year ago • 6 comments

Hi!

I was just trying to understand the disko's examples, and i did this refactoring.

i found this interesting and thought to share with yous and ask if you'd be interested to get a merge request refactoring others in a similar fashion too :smiley:?

image


i tried to run the nix-instantiate --eval --strict (nixel reference) and yes, these evaluated to absolutely same thing :smiley:

$ nix-instantiate --eval --strict legacy-table.nix
{ disko = { devices = { disk = { main = { content = { format = "gpt"; partitions = [ { bootable = true; content = { format = "vfat"; mountpoint = "/boot"; type = "filesystem"; }; end = "500M"; name = "ESP"; start = "1M"; } { bootable = true; content = { format = "ext4"; mountpoint = "/"; type = "filesystem"; }; end = "100%"; name = "root"; part-type = "primary"; start = "500M"; } ]; type = "table"; }; device = "/dev/sda"; type = "disk"; }; }; }; }; }

$ nix-instantiate --eval --strict legacy-table-refactored.nix 
{ disko = { devices = { disk = { main = { content = { format = "gpt"; partitions = [ { bootable = true; content = { format = "vfat"; mountpoint = "/boot"; type = "filesystem"; }; end = "500M"; name = "ESP"; start = "1M"; } { bootable = true; content = { format = "ext4"; mountpoint = "/"; type = "filesystem"; }; end = "100%"; name = "root"; part-type = "primary"; start = "500M"; } ]; type = "table"; }; device = "/dev/sda"; type = "disk"; }; }; }; }; }


a more elaborate setup to compare automated-ly, rather than eyeballing :eyes: :

eval-diff() {
  # Show diff of strict-eval of 2 nix files

  # Get the parameters
  local oldfile=$1
  local newfile=$2

  # Function: Abstract out the flags in invocation
  sevalnix() { nix-instantiate --eval --strict $1; }

  # Strict-evaluate both the files
  local oldval="$(sevalnix ${oldfile})"
  local newval="$(sevalnix ${newfile})"

  # Use process substitution to make stdout act as files to `diff` utility
  diff -u <(echo "$oldval") <(echo "$newval") | 

  # Pipe to `bat` with syntax-highlighting for diff
  bat -l diff;
}
$ eval-diff legacy-table{,-refactored}.nix 
───────┬──────────────────────────────────────────────────────────────
       │ STDIN   <EMPTY>
───────┴──────────────────────────────────────────────────────────────


goyalyashpal avatar Aug 02 '24 17:08 goyalyashpal

not sure, this touches the legacy partitions which we should migrate from anyway. Also by splitting it up people could be inclined to split the code over multiple files and this will trigger weird errors.

Lassulus avatar Aug 12 '24 06:08 Lassulus

not sure, this touches the legacy partitions which we should migrate from anyway.

I think this was just meant as an example to show what this would look like for all examples. Also as long as we still have uboot firmware that requires fat partitioning, we cannot get rid of it until there is a proper replacement for MBR.

Also by splitting it up people could be inclined to split the code over multiple files and this will trigger weird errors.

What parts don't merge correctly? There are ways to allow only one instance to be defined. I think this is not a very strong argument to make things less readable for everyone.

Mic92 avatar Aug 12 '24 06:08 Mic92

What parts don't merge correctly? There are ways to allow only one instance to be defined. I think this is not a very strong argument to make things less readable for everyone.

https://github.com/nix-community/disko/issues/678

Lassulus avatar Aug 12 '24 15:08 Lassulus

There are ways to allow only one instance to be defined. - @ Mic92 at https://github.com/nix-community/disko/pull/724#issuecomment-2283201907

what ways? can those help in this #678, where same key was defined more than once? it would be awesome if there is :smiley:

by splitting it up people could be inclined to split ... trigger[ing] weird errors [as seen in] https://github.com/nix-community/disko/issues/678 - @ Lassulus at https://github.com/nix-community/disko/pull/724#issuecomment-2283195200

how about some note like any of following or smth similar; maybe in the example files themselves?

  • "Do NOT do multiple-conflicting definitions for same key"
  • "Unexpected behaviour for multiple-conflicting definitions for same key"
  • "Do not define same key multiple times"
  • "Unexpected behaviour if same key defined more than once"
  • ...

'cz in the current form, the examples are unreadable and don't really fulfill their purpose to the full potent.

goyalyashpal avatar Aug 14 '24 08:08 goyalyashpal

hi all. so, did you reach on any conclusion?

should this be kept open, discussed, closed or merged?

goyalyashpal avatar Mar 12 '25 20:03 goyalyashpal

maybe there's some way in nix-language to restrict (say error out and early exit) in such cases?

goyalyashpal avatar Mar 16 '25 07:03 goyalyashpal