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

feat: Add option to static linking with openssl #104

Merged
merged 3 commits into from
Mar 2, 2023

Conversation

ikrivosheev
Copy link
Contributor

  1. Added YARA_OPENSSL_STATIC env for static linking with openssl
  2. Rename env OPENSSL_DIR to YARA_OPENSSL_DIR - remove name conflict with rust-openssl
  3. Rename env OPENSSL_LIB_DIR to YARA_OPENSSL_LIB_DIR - remove name conflict with rust-openssl
  4. Rename env OPENSSL_INCLUDE_DIR to YARA_OPENSSL_INCLUDE_DIR - remove name conflict with rust-openssl
  5. Rename env LIBYARA_STATIC to YARA_STATIC - for uniform with other envs
  6. Improve doc

@vthib you added envs for openssl, can you look at the PR? My thought was: remove name conflict with rust-openssl, maybe I am not right and it's ok.

@Hugal31
Copy link
Owner

Hugal31 commented Mar 2, 2023

LGTM!
Did you consider to use a cargo feature for the YARA_STATIC and YARA_OPENSSL_STATIC? Is that something we want?

@ikrivosheev
Copy link
Contributor Author

LGTM! Did you consider to use a cargo feature for the YARA_STATIC and YARA_OPENSSL_STATIC? Is that something we want?

Yes! It makes sense! Done)

@Hugal31 Hugal31 merged commit d77c4fe into Hugal31:master Mar 2, 2023
@vthib
Copy link
Contributor

vthib commented Mar 2, 2023

@ikrivosheev I added them to mimic rust-openssl, so that setting the env can work for both.

I'm not sure why the collision would be a bad thing, unless you want to use different versions of openssl for both.

But splitting them is fine too, just a bit more annoying when both OPENSSL_* and YARA_OPENSSL_* have to be set instead of just the OPENSSL_* one

@ikrivosheev ikrivosheev deleted the fix/openssl_linking branch March 2, 2023 21:00
@ikrivosheev
Copy link
Contributor Author

@Hugal31 do we want to add support both envs: OPENSSL_* and YARA_OPENSSL_*?

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.

3 participants