<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Let’s try all this again. I have a couple questions.<br><br>On Feb 4, 2014, at 3:41 AM, Stephen Kelly <<a href="mailto:steveire@gmail.com">steveire@gmail.com</a>> wrote:<br><br><blockquote type="cite">2) The change to cmNinjaNormalTargetGenerator seems incorrect. Should <br>linkFlags should be used with AddLinkOptions?<br></blockquote><br>Do you mean linkFlags vs vars[“LINK_FLAGS”]? I suppose it could use linkFlags. I could just revert the call to GetTargetFlags to use vars[“LINK_FLAGS”]. I was trying to avoid the case of getting the link flags for the target twice. Ie getting them once in vars[“LINK_FLAGS”] and then again in AddLinkOptions().<br><br><blockquote type="cite">3) Documentation title underlines should be as long as the title, not 3 <br>dashes longer.<br></blockquote><br>Ok great. What specifically are you referring to with this comment?<br><br><blockquote type="cite">4) Tests should avoid bad practices like using target_link_options to add <br>libraries. Instead try to choose suitable link flags for the tests. <br></blockquote><br>I’m having a little trouble with your notion of ‘bad practices.’ I’m sure you have good reasons for<br>making the determination that they are bad practices, but to me they seem somewhat arbitrary.<br>Is there some design document that you are using to make these determinations? I’m not trying<br>to cause problems, I’m just saying that adding a library as a linker flag is a perfectly normal/legitimate<br>thing to do coming from a Makefile world (or other build environment world). I realize that CMake<br>has different mechanisms for explicitly handling libraries, but the point isn’t to handle the library,<br>the point was just to pass a reasonable linker option.<br><br><blockquote type="cite">On GNU, you can do this:<br><br>add_executable(foo foo.cpp)<br>target_link_options(foo PRIVATE -Wl,--ignore-unresolved-symbol,main)<br><br>and write a foo.cpp which does not define main.<br></blockquote><br>I agree this would make a good test, but it doesn’t change my earlier comment. Adding a library via<br>a linker option is a perfectly valid use of linker options. If it wasn’t, the linker (or the compiler driver) wouldn’t<br>support it.<br><br>Does that make sense? How should I determine what is bad practice in the face of what is reasonable?<br><br><blockquote type="cite">5) Regarding what I said before about accepting -Wl,--no-undefined versus <br>accepting --no-undefined, I think the case of flags with arguments as above <br>shows that only the -Wl, prefixed case should be accepted (as your branch <br>already does). <br></blockquote><br>However, what if someone changes the mechanism that picks the linker so that instead of using the compiler<br>driver to drive the link stage, they use the actual linker? I don’t do such a thing with my build systems and<br>in all probability the majority of users would never need to make such a change. However, if I, for some reason,<br>*did* need to change CMake so that the link stage invoked the linker directly, I would absolutely expect the link<br>options commands to pass whatever linker option I deemed necessary to the linker.<br><br>I don’t think that this suggestion is the right way to go. I will of course defer to your judgement, but I don’t agree.<br><br>Perhaps there is a detail I’m missing about how the code makes a distinction between —no-undefined and<br>-Wl,—no-undefined.<br><br><br><blockquote type="cite">The documentation should possibly be clear that the contents of LINK_OPTIONS <br>should be suitable for passing to the driver, not directly to the linker.</blockquote></body></html>