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

WARP: Update README with macOS library path instructions #6256

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

cyberkaida
Copy link

@cyberkaida cyberkaida commented Dec 14, 2024

The README said to use DEP_BINARYNINJACORE_PATH but this was not referenced.

I updated the build.rs to use this variable, and as a treat I added the default install locations from the User Guide

I think this works on Windows too, but I do not have a Windows install to test. Thank you for the hard work on WARP I am excited to use it! 💜💜🥷

@CLAassistant
Copy link

CLAassistant commented Dec 14, 2024

CLA assistant check
All committers have signed the CLA.

cyberkaida

This comment was marked as resolved.

@emesare
Copy link
Member

emesare commented Dec 14, 2024

Hi, thank you for the PR.

I don't know why the README says to set DEP_BINARYNINJACORE_PATH, that env var is only used on a separate branch.

Anyways what you want is to set BINARYNINJADIR instead.

Also this might interest you: Vector35/warp#22 (sigem docker image)

@@ -18,10 +19,48 @@ fn compile_rust(file: PathBuf) -> bool {
}

fn main() {
if let Some(link_path) = option_env!("BINARYNINJADIR") {

let binja_base_dir: &str = option_env!("DEP_BINARYNINJACORE_BASE_DIR").unwrap_or_else(|| {
Copy link
Member

Choose a reason for hiding this comment

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

Our build system needs to set its own paths, so unfortunately we can't do lookups like this.

Copy link
Member

Choose a reason for hiding this comment

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

This would work fine: 5239ad6

@emesare
Copy link
Member

emesare commented Dec 14, 2024

Correct the README: 487fa4b
Add auto linking: 5239ad6

Let me know if those two suffice.

FWIW we store a lastrun artifact on your system which can be used instead of guessing for the install.

Thank you for the PR!

@emesare emesare self-assigned this Dec 14, 2024
@psifertex
Copy link
Member

Correct the README: 487fa4b Add auto linking: 5239ad6

Let me know if those two suffice.

FWIW we store a lastrun artifact on your system which can be used instead of guessing for the install.

Thank you for the PR!

Lastrun is documented here for those curious:
https://docs.binary.ninja/guide/index.html#user-folder (it lives in your user folder)

@cyberkaida
Copy link
Author

Correct the README: 487fa4b

Add auto linking: 5239ad6

Let me know if those two suffice.

FWIW we store a lastrun artifact on your system which can be used instead of guessing for the install.

Thank you for the PR!

This looks much better, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants