Skip to content

android-tools: add patch to fix build against 6.0 kernel headers#196146

Merged
vcunat merged 1 commit intoNixOS:masterfrom
yu-re-ka:staging-next-fix-android-tools
Oct 25, 2022
Merged

android-tools: add patch to fix build against 6.0 kernel headers#196146
vcunat merged 1 commit intoNixOS:masterfrom
yu-re-ka:staging-next-fix-android-tools

Conversation

@yu-re-ka
Copy link
Contributor

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@yu-re-ka yu-re-ka changed the title android-tools: add patch to fix build against 6.0 kernel headers [staging-next] android-tools: add patch to fix build against 6.0 kernel headers Oct 15, 2022
@ofborg ofborg bot requested a review from primeos October 15, 2022 14:50
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Oct 15, 2022
@vcunat vcunat mentioned this pull request Oct 18, 2022
13 tasks
@trofi
Copy link
Contributor

trofi commented Oct 19, 2022

Result of nixpkgs-review pr 196146 run on x86_64-linux 1

2 packages built:
  • agi
  • android-tools

Copy link
Contributor

@trofi trofi left a comment

Choose a reason for hiding this comment

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

Is it an upstream patch? If not should it be forwarded there as well?

@primeos
Copy link
Member

primeos commented Oct 19, 2022

Is it an upstream patch? If not should it be forwarded there as well?

Agreed. If it's an upstream patch then fetchpatch is much preferred (AFAIK; and the source should definitely be documented in Nixpkgs). If this patch is from you then it should be sent upstream as well (and ideally it would even get merged so that we can use fetchpatch).

@vcunat
Copy link
Member

vcunat commented Oct 21, 2022

Upstream merged a similar patch in the meantime: nmeum/android-tools#85

They're exactly the same, except they have additional diffs in usb_close() and register_device(). I'd expect their patch is more correct, but I haven't really reviewed either.

@yu-re-ka
Copy link
Contributor Author

I wrote this patch with the help of a friend. I did not get around to submitting it upstream, but if they already have an equivalent patch submitted there that's great too.

They're exactly the same, except they have additional diffs in usb_close() and register_device().

You overlooked that my patch adds the allocation/free of heap-allocated child structs to the constructor/destructor of usb_handle. This is cleaner than what they are doing because it makes it impossible to accidentally delete the usb_handle without freeing the child structs too. But well, theirs will probably work too (except for leaking memory in some failure cases).

@vcunat
Copy link
Member

vcunat commented Oct 22, 2022

Oh right, I did overlook that when diffing the diffs.

@vcunat vcunat changed the base branch from staging-next to master October 22, 2022 14:27
@vcunat vcunat changed the title [staging-next] android-tools: add patch to fix build against 6.0 kernel headers android-tools: add patch to fix build against 6.0 kernel headers Oct 22, 2022
@yu-re-ka
Copy link
Contributor Author

Please someone else update or create a new PR for importing the upstream patch. I can't focus on this currently.
Feel free to request review from me then.

@vcunat vcunat merged commit 870ebcb into NixOS:master Oct 25, 2022
@vcunat
Copy link
Member

vcunat commented Oct 25, 2022

The upstream patch wouldn't apply as-is. Maybe it will after the update. Anyway, your patch should be OK and at least it fixes the build now. Apparently, many nixpkgs users were affected.

@yu-re-ka yu-re-ka deleted the staging-next-fix-android-tools branch October 25, 2022 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants