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

python3Packages.capstone: aarch64 platform name #110548

Merged

Conversation

urbas
Copy link
Contributor

@urbas urbas commented Jan 22, 2021

Motivation for this change

This package currently doesn't build on aarch64 without this change.

This change fixes this build failure.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@risicle
Copy link
Contributor

risicle commented Jan 23, 2021

Can't test but this looks sensible.

@urbas urbas force-pushed the python-capstone-aarch64-build-fix branch from 7de5f9d to 25a1b3b Compare January 23, 2021 22:15
@SuperSandro2000
Copy link
Member

@jonringer is this approach acceptable?


src = capstone.src;
sourceRoot = "${capstone.name}/bindings/python";

postPatch = ''
ln -s ${capstone}/lib/libcapstone${stdenv.targetPlatform.extensions.sharedLibrary} prebuilt/
ln -s ${capstone}/lib/libcapstone.a prebuilt/
'' + lib.optionalString stdenv.targetPlatform.isAarch64 ''
substituteInPlace setup.py --replace manylinux1 manylinux2014
Copy link
Member

Choose a reason for hiding this comment

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

I thought about this a bit more and do you know if it would be save to apply on all platforms? Otherwise it might be forgotten with the next update.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SuperSandro2000: Indeed, that makes sense. I removed the conditional entirely. PTAL

@urbas
Copy link
Contributor Author

urbas commented Feb 14, 2021

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

16 packages built:
  • pwndbg
  • pwntools (python38Packages.pwntools)
  • python37Packages.ROPGadget
  • python37Packages.capstone
  • python37Packages.manticore
  • python37Packages.pwntools
  • python37Packages.ropper
  • python38Packages.ROPGadget
  • python38Packages.capstone
  • python38Packages.manticore
  • python38Packages.ropper
  • python39Packages.ROPGadget
  • python39Packages.capstone
  • python39Packages.manticore
  • python39Packages.pwntools
  • python39Packages.ropper

@SuperSandro2000 SuperSandro2000 merged commit 49414d2 into NixOS:master Feb 14, 2021
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.

None yet

3 participants