mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Schwartzmeyer <and...@schwartzmeyer.com>
Subject Re: Review Request 66493: Made FreeBSD default to non-GNU ld.
Date Mon, 09 Apr 2018 21:22:56 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message