facebook / chisel

Chisel is a collection of LLDB commands to assist debugging iOS apps.
MIT License
9.13k stars 803 forks source link

Add Makefile for installing framework #214

Closed keith closed 6 years ago

keith commented 6 years ago

This allows you to run make install with optional environment variables in order to build and install Chisel.framework.

kastiglione commented 6 years ago

I should hopefully be able to accept this soon. In the mean time, I have a couple questions:

I'm indifferent about the fat binary, I'm fine with being lazy and waiting for a request/need.

I don't know the ins and outs of code signing, but I hit some issues that frustrated me during development of this library. If I loaded the library from where it was originally built and code signed (within the build directory) and then later moved or copied the framework to some install location, then I would no longer be able to load the library from the installed location. The error messages were non-existent or cryptic at best, nothing that I could solve from google. Eventually I figured out that if I re-codesigned the installed library, then I wouldn't have those same loading problems. I guess what I'm asking is, is signing supposed to be post-install or can it be pre-install? Presumably pre-install, but the issues tell me that I don't really know how path comes into play with code signing verification.

keith commented 6 years ago

Should we build a fat binary with x86 too?

This actually is building both right now, since it's a release build, ONLY_ACTIVE_ARCH is NO.

Should we code sign after copying into the destination?

Good question. The adhoc signing that's happening when I run the install now is currently working for me, but I've also only tested on the simulator. If I remove that signature I do get this for dlerror():

(char *) $3 = 0x00007fa68b400249 "dlopen(/usr/local/opt/chisel/lib/Chisel.framework/Chisel, 2): no suitable image found.  Did find:\n\t/usr/local/opt/chisel/lib/Chisel.framework/Chisel: required code signature missing for '/usr/local/opt/chisel/lib/Chisel.framework/Chisel'\n"
kastiglione commented 6 years ago

I should be able to merge this next week.

keith commented 6 years ago

Addressed your feedback!

keith commented 6 years ago

Thanks!

kastiglione commented 6 years ago

You're welcome, thanks for the pull request. I'll create a pull request to add an implementation for the findinstances command some time later today.