Page MenuHomePhabricator

[CMake] Explicit lldb_codesign function with application in debugserver and lldb-server
AbandonedPublic

Authored by sgraenitz on Nov 9 2018, 1:28 PM.

Details

Summary

Add LLDB-specific utility function lldb_codesign. In contrast to llvm_codesign it must be invoked explicitly and allows to pass a target-specific entitlements file as well as an override for the codesign identity.

Event Timeline

sgraenitz created this revision.Nov 9 2018, 1:28 PM

This would soon be used for other targets as well. llvm_codesign is here: https://reviews.llvm.org/D48797

cmake/modules/AddLLDB.cmake
212

So far --force was used everywhere to avoid trouble with double signing I guess (turns the error into a warning). IIUC this should never happen for a post-build action right? Thus, the "more correct" way may be to remove this from invocations/altogether and go fix the actual issue if it ever fails. But then I think, who wants to bother with double code signing issues..

Anyway, llvm_codesign doesn't force. Any good reason to keep it?

sgraenitz added a subscriber: Restricted Project.Nov 9 2018, 1:57 PM
beanz added a comment.Nov 9 2018, 3:18 PM

Why not just add entitlement support to ‘llvm_codesign’? Or feed through extra arguments?

Yes, considered it, but was not sure whether it's a good idea to bloat the general mechanism in LLVM to match the requirements for a single subproject. Do you think other projects would benefit from it?

My reasoning was: Passing through target-specific parameters to llvm_codesign is not simple as it runs "implicitly" as part of add_llvm_executable/library. In order to align with its current approach, entitlements had to be set in a global variable (like LLVM_CODESIGNING_IDENTITY) beforehand. Furthermore, we want to avoid entitlements to be used for yet another target, so we had to unset it afterwards. The conditions that determine the correct entitlements for each situation seem to be complicated already. Not sure if that's a good combination.

OTOH, if we are in favour of a unified implementation, it would be great to encapsulate the details and write something like:

add_lldb_tool(target
  ${sources}

  LINK_LIBS
    ${libs}

  ENTITLEMENTS
    ${entitlements}
)

I think this would still require quite some additions to llvm_codesign. I went with the simpler version for now. What do you think?

beanz added a comment.Nov 12 2018, 9:58 AM

I can certainly foresee other LLVM-based projects needing entitlements, so I am very much in favor of adding ENTITLEMENTS and FORCE options onto llvm_codesign.

Ok, I will have a look and prepare a draft.

Please find the alternative proposal in https://reviews.llvm.org/D54443 (llvm) and https://reviews.llvm.org/D54444 (lldb).

lanza resigned from this revision.Nov 12 2018, 9:40 PM
sgraenitz abandoned this revision.Nov 13 2018, 7:47 AM

Finally, debugserver with llvm_codesign: https://reviews.llvm.org/D54476