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

Jump to bottom

Add libirecovery and idevicerestore #59427

Merged
merged 3 commits into from Aug 13, 2019
Merged

Add libirecovery and idevicerestore #59427

merged 3 commits into from Aug 13, 2019

Conversation

nh2
Copy link
Contributor

@nh2 nh2 commented Apr 13, 2019 •
edited

Motivation for this change

I want to restore an old iPad to factory settings.

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/)
    • not all but I tested that idevicerestore -l -e successfully erased an iPad3,3 I tried it on to firmware 9.3.5 (build 13G36) on Ubuntu 16.04 with the built binary
  • 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 PR also removes one unnecessary --disable-static configure flag from a dependency.

@nh2 nh2 requested a review from worldofpeace April 13, 2019 22:53
@nh2 nh2 mentioned this pull request Apr 13, 2019
Closed
worldofpeace
worldofpeace suggested changes Apr 13, 2019
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

Hi, I've left some comments.

Currently libirecovery fails at a configure error

configure: error: Could not determine an appropriate user group for the udev activation rule.
    Please manually specify a udev activation rule using --with-udevrule=<RULE>
    Example: --with-udevrule="OWNER=\"root\", GROUP=\"myusergroup\", MODE=\"0660\""

pkgs/tools/misc/idevicerestore/default.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/idevicerestore/default.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/idevicerestore/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libirecovery/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libirecovery/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libirecovery/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/libirecovery/default.nix Outdated Show resolved Hide resolved
nh2
nh2 commented Apr 14, 2019
View reviewed changes
Copy link
Contributor Author

@nh2 nh2 left a comment

Choose a reason for hiding this comment

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

Currently libirecovery fails at a configure error

@worldofpeace You're right; this appears only on NixOS, not on Ubuntu, where I had built it. Now that my NixOS build got this far, I also got it.

Looking at the logic in https://github.com/libimobiledevice/libirecovery/blob/5da2a0d7d60f79d93c283964888c6fbbc17be1a3/configure.ac#L180-L185

it prefers one of these groups, in order: plugdev storage disk staff.

plugdev seems to be used in NixOS:

~/src/nixpkgs (git)-[idevicerestore] % git grep plugdev
nixos/modules/hardware/onlykey.udev:SUBSYSTEMS=="usb", ATTRS{idVendor}=="16c0", ATTRS{idProduct}=="04[789ABCD]?", GROUP+="plugdev"
nixos/modules/hardware/onlykey.udev:KERNEL=="ttyACM*", ATTRS{idVendor}=="16c0", ATTRS{idProduct}=="04[789B]?", GROUP+="plugdev"
pkgs/applications/radio/hackrf/default.nix:  cmakeFlags = [ "-DUDEV_RULES_GROUP=plugdev" "-DUDEV_RULES_PATH=lib/udev/rules.d" ];
pkgs/tools/security/nitrokey-app/udev-rules.nix:    substituteInPlace libnitrokey/data/41-nitrokey.rules --replace plugdev "${group}"

so I will go with that for now.

pkgs/tools/misc/idevicerestore/default.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/idevicerestore/default.nix Outdated Show resolved Hide resolved
@nh2
Copy link
Contributor Author

nh2 commented Apr 14, 2019

so I will go with that for now.

Build fixed, please take another look @worldofpeace.

@nh2 nh2 requested a review from worldofpeace April 14, 2019 01:05
@worldofpeace
Copy link
Contributor

@worldofpeace Hmm, the license file https://github.com/libimobiledevice/idevicerestore/blob/8a882038b2b1e022fbd19eaf8bea51006a373c06/COPYING says LGPL v3.

What do we do when files in the upstream projects have different per-file licenses? Also, given that the one you linked says or (at your option) any later version, isn't it still accurate to use license = licenses.lgpl3?

I feel that the license meta attribute is not supposed to represent a preference at our discretion but to just tell us the license exactly as it's specified. Usually the license in the code is the same for every file. (yet to have checked)

So I think lgpl21Plus is most accurate. But it's not that important given the parameters set.

worldofpeace
worldofpeace approved these changes Apr 14, 2019
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

Looks good to me and builds.
Haven't tested the function because I don't have the exact means to do so.

worldofpeace
worldofpeace reviewed Apr 14, 2019
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

Maybe correct licenses as specified in the source code.

@nh2
Copy link
Contributor Author

nh2 commented Aug 12, 2019

So I think lgpl21Plus is most accurate.

OK, I've switched to that.

@nh2
Copy link
Contributor Author

nh2 commented Aug 12, 2019

Add libplist too.

@worldofpeace Why libplist? It's already a propagaged dep of libimobiledevice, and I think the idevicerestore deps just copy-pasted libimobiledevice's deps, see https://github.com/NixOS/nixpkgs/pull/59427/files#diff-eccc4194ea3da17c4d5624f20fe9dbe2R31

@ofborg ofborg bot requested a review from infinisil August 13, 2019 00:02
@nh2
Copy link
Contributor Author

nh2 commented Aug 13, 2019

@worldofpeace Why libplist? It's already a propagaged dep of libimobiledevice, and I think the idevicerestore deps just copy-pasted libimobiledevice's deps, see https://github.com/NixOS/nixpkgs/pull/59427/files#diff-eccc4194ea3da17c4d5624f20fe9dbe2R31

Clarified via IRC:

worldofpeace> nh2: yeah I'm seeing libimobiledevices's .pc file has Requries: libplist so it should be propagated. though unfortunely we're propagating things in Requires.private also.

@nh2
Copy link
Contributor Author

nh2 commented Aug 13, 2019

Another point:

worldofpeace> nh2: We also realized that everything imobile related should be updated at once. so we need to try to match the dates of the commits from the packages already in nixpkgs

My response:

I believe I did that. My PR has idevicerestore-2019-02-14, libimobiledevice-2019-04-04, libirecovery-2019-01-28, which at the time I made the PR, were the 3 latest respective versions.

(I actually wrote libirecovery-2018-01-28, 2018 instead of 2019, that is a typo I will fix now)

@nh2
Copy link
Contributor Author

nh2 commented Aug 13, 2019

@GrahamcOfBorg build

@worldofpeace
Copy link
Contributor

@nh2 I'm noticing that libirecovery propagates libusb but libusb is Requires.private and I don't see readline in the .pc

@nh2
Copy link
Contributor Author

nh2 commented Aug 13, 2019

@nh2 I'm noticing that libirecovery propagates libusb but libusb is Requires.private and I don't see readline in the .pc

Good point, I've changed to make them buildInputs instead.

@worldofpeace
Copy link
Contributor

@GrahamcOfBorg build libirecovery idevicerestore libimobiledevice

@worldofpeace worldofpeace merged commit f4af5f3 into NixOS:master Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@symphorien symphorien symphorien left review comments

@worldofpeace worldofpeace worldofpeace approved these changes

@infinisil infinisil Awaiting requested review from infinisil

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nh2 @worldofpeace @symphorien @GrahamcOfBorg