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 Thu, 12 Apr 2018 16:27:13 GMT


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


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