> On April 9, 2018, 6:13 a.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 308-311 (patched)
> > <https://reviews.apache.org/r/66493/diff/1/?file=1993835#file1993835line308>
> >
> >     I was briefly wondering why we checked for clang below. The reason is 
> > of course that cmake by default uses the compiler as linker, and there is 
> > an incompatibility on FreeBSD when compiling with clang and linking with 
> > `ld.bfd`.
> >     
> >     I think it would be useful to describe this in more detail here so we 
> > have a chance to understand when this check can go in the future. Ideally 
> > we'd be able to link some upstream issue, but it may not exist.

Yeah, it took me a while to reason through this too, even with your comment 
Benjamin. It's because of how the linker command is formed (described 
[here](https://cmake.org/pipermail/cmake/2014-August/058268.html)). Not knowing 
this will leave you bewildered here.


> 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?

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).


> On April 9, 2018, 6:13 a.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 322 (patched)
> > <https://reviews.apache.org/r/66493/diff/1/?file=1993835#file1993835line322>
> >
> >     Could we make clearer what a user needs to do to have this check pass? 
> > The error message appears too technical to me.

+1 I have no clue what I would do to get a "non-base LD."


> 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?

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`.


- Andrew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66493/#review200725
-----------------------------------------------------------


On April 7, 2018, 7:54 a.m., David Forsythe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66493/
> -----------------------------------------------------------
> 
> (Updated April 7, 2018, 7:54 a.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/1/
> 
> 
> Testing
> -------
> 
> make on FreeBSD, with both lld and gold.
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Reply via email to