search

trailofbits / uthenticode

A cross-platform library for verifying Authenticode signatures
https://trailofbits.github.io/uthenticode/
MIT License
136 stars 33 forks source link

Add cmake flag to use system gtest #40

Closed arcz closed 3 years ago

arcz commented 3 years ago

This allows to use already built gtest by supplying a cmake flag. Doesn't change the current build.

woodruffw commented 3 years ago

While I haven't tested this yet, I think we might need to add something like a find_package(GTest CONFIG REQUIRED) when USE_SYSTEM_GTEST is ON, and then this line will probably also need a conditional on USE_SYSTEM_GTEST

Yeah, I was confused as to how Nix was picking up the GTest build correctly without that. @arcz any thoughts?

woodruffw commented 3 years ago

(FWIW, the build failures are probably unrelated. I'll look into them in a moment.)

woodruffw commented 3 years ago 
Install/Update ports
  Running 'vcpkg remove --outdated --recurse ' in directory '/Users/runner/work/uthenticode/uthenticode/vcpkg' ...
  dyld: Symbol not found: ___emutls_v._ZSt11__once_call

🤔

woodruffw commented 3 years ago

https://github.com/lukka/run-vcpkg/issues/69 has the problem.

Edit: And #41 should have the fix in a moment.

woodruffw commented 3 years ago

Just something to note: the reason uthenticode didn't use a system copy of GTest is because GTest's authors recommend vendoring it as part of each build (presumably to avoid ABI issues).

That shouldn't be a problem with Nix, but we probably don't want to encourage people to use an arbitrary system build of GTest (e.g., the version distributed as googletest by Ubuntu).

While I haven't tested this yet, I think we might need to add something like a find_package(GTest CONFIG REQUIRED) when USE_SYSTEM_GTEST is ON, and then this line will probably also need a conditional on USE_SYSTEM_GTEST

Yep, this currently works magically on Nix because Nix adds the GTest include and link directories on its own. IMO that's actually probably how we want things for now, since we don't want to encourage people to use arbitrary system distributions of GTest (per above).