Add back support for relocatable packages

capstone-engine / capstone

Capstone disassembly/disassembler framework for ARM, ARM64 (ARMv8), Alpha, BPF, Ethereum VM, HPPA, LoongArch, M68K, M680X, Mips, MOS65XX, PPC, RISC-V(rv32G/rv64G), SH, Sparc, SystemZ, TMS320C64X, TriCore, Webassembly, XCore and X86.

http://www.capstone-engine.org

7.51k stars 1.54k forks source link

Add back support for relocatable packages #2431

Closed mrexodia closed 1 month ago

mrexodia commented 1 month ago

Your checklist for this pull request

[x] I've documented or updated the documentation of every API function and struct this PR changes.
[x] I've added tests that prove my fix is effective or that my feature works (if possible)

Detailed description

The previous PR https://github.com/capstone-engine/capstone/pull/2134 not only 'allowed' an absolute CMAKE_INSTALL_LIBDIR/CMAKE_INSTALL_INCLUDEDIR, it also broke support for relocatable packages if the path was not absolute.

These changes should also be cherry-picked in v5.

Discussion: https://github.com/NixOS/nixpkgs/issues/144170

Rot127 commented 1 month ago

These changes should also be cherry-picked in v5.

Could you please open a PR with it? We might even get it into the v5.0.2 path release today or tomorrow.

Rot127 commented 1 month ago

@kabeor Why is the CI not running here?

mrexodia commented 1 month ago

In general looks good to me. Can you tell me how quickly this becomes "tested" via NixOS?

No idea, I do not use NixOS. They are doing something different that is specifically recommended against by CMake's documentation. This patch goes back to the behavior before https://github.com/capstone-engine/capstone/pull/2134, unless those paths are absolute (as will be the case with NixOS). It's silly this workaround is required, but such is life 🤷🏻‍♂️

kabeor commented 1 month ago

Really wired, may be affected by github downtime? Can we commit something or open a new pr to see if CI come back?

mrexodia commented 1 month ago

This one also doesn't run CI: https://github.com/capstone-engine/capstone/pull/2447

XVilka commented 1 month ago

If CI doesn't run, it might be the sign of some error in the Actions YAML file. Check "Actions" log of the repository. Note also an error of using outdated NodeJS runner: https://github.com/capstone-engine/capstone/actions/runs/10419892239

Rot127 commented 1 month ago

@XVilka The yaml files are fine. The CI also runs for the other PRs as well. @mrexodia Can you rebase this one? The labeler was at least working on the v5 PR. Maybe it is indeed a Github outage problem.

mrexodia commented 1 month ago

Done!

Rot127 commented 1 month ago

For me all of the checks are green now. @kabeor Please take a look.