beanz added inline comments.
================
Comment at: CMakeLists.txt:403
+set(LLVM_CODESIGNING_IDENTITY "" CACHE STRING
+ "Sign executables and dylibs with the given identity or skip if empty
(Darwin Only)")
+
----------------
sgraenitz wrote:
> Identity should be a string right?
Yep, makes sense.
================
Comment at: cmake/modules/AddLLVM.cmake:795
- llvm_codesign(${name})
+ llvm_codesign(TARGET ${name} ENTITLEMENTS ${ARG_ENTITLEMENTS} FORCE)
endmacro(add_llvm_executable name)
----------------
sgraenitz wrote:
> sgraenitz wrote:
> > beanz wrote:
> > > sgraenitz wrote:
> > > > Would we want to pass `FORCE` to `add_llvm_executable` conditionally?
> > > I'm trying to think about the situations in which we need the `FORCE`
> > > option. Since this is connecting as a post-build event it shouldn't be
> > > running unless the target re-generates the output, so I'm not sure I
> > > understand why we ever need it.
> > >
> > > I did the git blame walk back to when the code was initially added in
> > > `49dd98a03a`, and there is no explanation. I suspect debugserver doesn't
> > > actually need the `--force` option because the author of the initial
> > > patch probably hit errors when re-signing the pre-built binary in his
> > > build directory.
> > >
> > > Thoughts?
> > I think you are right, it shouldn't be necessary. In practice, though, I
> > can imagine situations when we want to make sure this won't fail in any
> > case.
> >
> > The options are: remove entirely (most correct) OR allow enable per target
> > (most flexible) OR allow enable globally.
> >
> > What about the last one? I could add `LLVM_CODESIGNING_FORCE` which is OFF
> > by default. In failsafe/debugging situations it could be turned ON
> > globally. I could remove the `FORCE` parameter here and check the flag in
> > `llvm_codesign` (similar to `LLVM_CODESIGNING_IDENTITY`).
> Patch updated
My gut is to just remove forcing entirely, and only add it back if we actually
need it. Short of post-build steps being incorrectly implemented in a CMake
generator, I can't imagine a situation where it would be needed.
Repository:
rL LLVM
https://reviews.llvm.org/D54443
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
