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

libblockdev: add gptfdisk as a dependency #62757

Merged
merged 1 commit into from Jun 8, 2019

Conversation

lightbulbjim
Copy link
Contributor

Motivation for this change

Certain udisk2 operations (eg creating luks-encrypted ext4 volumes) require libblockdev to have access to the sgdisk utility. This change adds gptfdisk (which includes sgdisk) to libblockdev.

Fixes #62749.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

This fixes NixOS#62749 (sgdisk needed for creating luks-encrypted ext4
volumes via udisk2).
Copy link
Contributor

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

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

Just a 1M size increase:

$ nix path-info -f https://github.com/lightbulbjim/nixpkgs/archive/libblockdev-sgdisk.tar.gz -S libblockdev
/nix/store/p8wi7jmzx690ycfab0fkskv4mha4wdgp-libblockdev-2.20	  258715416
$ nix path-info -f https://github.com/NixOS/nixpkgs-channels/archive/nixos-unstable.tar.gz -S libblockdev
/nix/store/5rajvy6hjj10r4la6j1p4gjlp50vsbck-libblockdev-2.20	  257905504a

@worldofpeace
Copy link
Contributor

worldofpeace commented Jun 7, 2019

Should fix things as proposed for gnome-disk-utility, note there's more utilities that are expected like this

Wouldn't really make sense to carry a patch to hardcode everything though.

@lightbulbjim
Copy link
Contributor Author

Should fix things as proposed for gnome-disk-utility, note there's more utilities that are expected like this

I forgot to mention that gnome-disk-utility only seems to care about sgdisk (at least from this library). Creating various volume types, including btrfs and swap, all work after the change. gnome-disk-utility doesn't support creating LVM or RAID volumes.

There doesn't seem to be anything else in nixpkgs which would rely on further patching either. These are the reverse dependencies according to nix-review:

  • atom
  • atom-beta
  • cantata
  • clementine
  • clementineUnfree
  • far2l
  • gnome3.gnome-control-center
  • gnome3.gnome-disk-utility
  • gnome3.gvfs
  • gvfs
  • hal-flash
  • psensor
  • rabbitvcs
  • rapid-photo-downloader
  • spaceFM
  • udiskie
  • udisks
  • usermount
  • xfce.gigolo
  • xfce.gvfs

As far as I can tell none of them involve creating/formatting filesystems. So I think we're probably safe just to patch the sgdisk path, although I'm happy to include the other utilities in the patch for completeness if you'd prefer.

I'd also like it if this could be covered by tests, but without being able to predict new features I'm not sure how to do that effectively.

@worldofpeace
Copy link
Contributor

I'd also like it if this could be covered by tests, but without being able to predict new features I'm not sure how to do that effectively.

It appears that they need sudo

Though that being needed isn't mentioned in the documentation I'd assume we couldn't run them without a vm in this case.

As far as I can tell none of them involve creating/formatting filesystems. So I think we're probably safe just to patch the sgdisk path, although I'm happy to include the other utilities in the patch for completeness if you'd prefer.

I mentioned

Wouldn't really make sense to carry a patch to hardcode everything though.

I'm assuming, and now with the information you've provided, this should be sufficient.
Just wanted to have a FTR that this possibility was addressed.

@worldofpeace worldofpeace merged commit 059ce69 into NixOS:master Jun 8, 2019
@lightbulbjim lightbulbjim deleted the libblockdev-sgdisk branch June 8, 2019 06:52
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.

gnome3.gnome-disk-utility unable to find sgdisk
3 participants