> On April 9, 2018, 6:13 a.m., Benjamin Bannier wrote: > > cmake/CompilationConfigure.cmake > > Lines 315 (patched) > > <https://reviews.apache.org/r/66493/diff/1/?file=1993835#file1993835line315> > > > > This conditional seems not needed as cmake will automatically detect if > > `LD_PROGRAM` was set by the user. Is this a variable we should document as > > a customization point? > > Andrew Schwartzmeyer wrote: > Do you mean documenting like as an `option(...)` and in the docs, or just > in the docs? Maybe we should wait until the FreeBSD build is working, and > then add a doc for it (in which we could also explain the weirdness with > linkers). > > David Forsythe wrote: > Hm, I didn't realize find_program wouldn't run if I set this on my own. > I'll clean it up. > > As for documenting it, I figured that it would be best to wait until the > build was fixed and settled. Even if someone is building on FreeBSD at this > point nothing is really working. It might be a bit pre-mature to start > surfacing docs about the build (which probably means there should be a ticket > about documenting this?).
+1 > On April 9, 2018, 6:13 a.m., Benjamin Bannier wrote: > > cmake/CompilationConfigure.cmake > > Lines 326-329 (patched) > > <https://reviews.apache.org/r/66493/diff/1/?file=1993835#file1993835line326> > > > > I am not sure this is an issue, but customary there are similar > > variables defined for other build types like `DEBUG`, `RELEASE`, etc. > > (e.g., `CMAKE_EXE_LINKER_FLAGS_DEBUG`). > > > > Do we have something to set all related flags @andschwa? > > Andrew Schwartzmeyer wrote: > It might soon be time to put this into a function, but yeah, it'd be a > nested for loop akin to this: > > ``` > foreach (lang C CXX) > list(APPEND CMAKE_${lang}_FORWARD_ARGS > ${CMAKE_FORWARD_ARGS} > -DCMAKE_${lang}_COMPILER=${CMAKE_${lang}_COMPILER} > -DCMAKE_${lang}_COMPILER_LAUNCHER=${CMAKE_${lang}_COMPILER_LAUNCHER} > -DCMAKE_${lang}_FLAGS=${CMAKE_${lang}_FLAGS}) > > foreach (config DEBUG RELEASE RELWITHDEBINFO MINSIZEREL) > list(APPEND CMAKE_${lang}_FORWARD_ARGS > -DCMAKE_${lang}_FLAGS_${config}=${CMAKE_${lang}_FLAGS_${config}}) > endforeach () > endforeach () > > ``` > > Note that this only matters if you're using a multi-configuration > generator (like Visual Studio and some other IDEs), but not `make` nor > `ninja`. > > David Forsythe wrote: > Will switch over to something like that. > > David Forsythe wrote: > I left off the the config loop because it didn't seem like the other > flags being set in this file were using them. It wasn't clear to me if they > were actually needed, so if I should add them just let me know. I wouldn't add it until you actually need it. - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66493/#review200725 ----------------------------------------------------------- On April 11, 2018, 10:13 p.m., David Forsythe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66493/ > ----------------------------------------------------------- > > (Updated April 11, 2018, 10:13 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier. > > > Bugs: MESOS-8761 > https://issues.apache.org/jira/browse/MESOS-8761 > > > Repository: mesos > > > Description > ------- > > Made FreeBSD default to non-GNU ld. > > > Diffs > ----- > > cmake/CompilationConfigure.cmake 3cb072ddcd286c0e40d44eaeba210ddf1796975c > > > Diff: https://reviews.apache.org/r/66493/diff/3/ > > > Testing > ------- > > make on FreeBSD, with both lld and gold. > > > Thanks, > > David Forsythe > >
