Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[Benjamin Bannier]

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


Fix it, then Ship it!





cmake/CompilationConfigure.cmake
Lines 308 (patched)


Drop _on_.


- Benjamin Bannier


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

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[Andrew Schwartzmeyer]

> On April 9, 2018, 6:13 a.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 315 (patched)
> > 
> >
> > 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)
> > 
> >
> > 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
> 
>

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66493]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


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

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66493 was successfully built and tested.

Reviews applied: `['66493']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66493

- Mesos Reviewbot Windows


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

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[David Forsythe]

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

(Updated April 12, 2018, 5:13 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 (updated)
-

  cmake/CompilationConfigure.cmake 3cb072ddcd286c0e40d44eaeba210ddf1796975c 


Diff: https://reviews.apache.org/r/66493/diff/3/

Changes: https://reviews.apache.org/r/66493/diff/2-3/


Testing
---

make on FreeBSD, with both lld and gold.


Thanks,

David Forsythe

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[Andrew Schwartzmeyer]

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


Fix it, then Ship it!





cmake/CompilationConfigure.cmake
Lines 322 (patched)


(Nit: I wouldn't even bother with this doc string. "Linker path." is 
pointless, but so is explaining it, especially considering the comment above 
and the error message below, and the variable name itself.)



cmake/CompilationConfigure.cmake
Lines 330-332 (patched)


Nit: No real reason to set `USE_LD_FLAG` since it's used once. 
`string(APPEND CMAKE_${type}_LINKER_FLAGS " -fuse-ld=${LD_PROGRAM}")` should be 
just fine.


- Andrew Schwartzmeyer


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

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66493]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


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

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66493 was successfully built and tested.

Reviews applied: `['66493']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66493

- Mesos Reviewbot Windows


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

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[David Forsythe]

> On April 9, 2018, 1:13 p.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 308-311 (patched)
> > 
> >
> > 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.
> 
> Andrew Schwartzmeyer wrote:
> 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.
> 
> David Forsythe wrote:
> I'll make the comment a bit more descriptive.

I'm the beginning to think this isn't actually a FreeBSD specific issue, but I 
left the comment FreeBSD specific (and left the CMAKE_SYSTEM_NAME check) since 
I haven't had anyone tell me they've reproduced it on Linux.


> On April 9, 2018, 1:13 p.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 326-329 (patched)
> > 
> >
> > 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.

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.


- David


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


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

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[David Forsythe]

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

(Updated April 10, 2018, 6:36 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 (updated)
-

  cmake/CompilationConfigure.cmake 3cb072ddcd286c0e40d44eaeba210ddf1796975c 


Diff: https://reviews.apache.org/r/66493/diff/2/

Changes: https://reviews.apache.org/r/66493/diff/1-2/


Testing
---

make on FreeBSD, with both lld and gold.


Thanks,

David Forsythe

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[David Forsythe]

> On April 9, 2018, 1:13 p.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 308-311 (patched)
> > 
> >
> > 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.
> 
> Andrew Schwartzmeyer wrote:
> 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.

I'll make the comment a bit more descriptive.


> On April 9, 2018, 1:13 p.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 315 (patched)
> > 
> >
> > 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).

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


> On April 9, 2018, 1:13 p.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 318 (patched)
> > 
> >
> > Since this variable will be setable from the outside _Alternative_ 
> > seems redundant here.

Removed.


> On April 9, 2018, 1:13 p.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 322 (patched)
> > 
> >
> > Could we make clearer what a user needs to do to have this check pass? 
> > The error message appears too technical to me.
> 
> Andrew Schwartzmeyer wrote:
> +1 I have no clue what I would do to get a "non-base LD."

This isn't even accurate since lld is in base. I'll update it.


> On April 9, 2018, 1:13 p.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 326-329 (patched)
> > 
> >
> > 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`.

Will switch over to something like that.


- David


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


On April 10, 2018, 6:36 a.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66493/
> ---
> 
> (Updated April 10, 2018, 6:36 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> 

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[Andrew Schwartzmeyer]

> On April 9, 2018, 6:13 a.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 308-311 (patched)
> > 
> >
> > 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)
> > 
> >
> > 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)
> > 
> >
> > 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)
> > 
> >
> > 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
> 
>

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[Benjamin Bannier]

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




cmake/CompilationConfigure.cmake
Lines 308-311 (patched)


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.



cmake/CompilationConfigure.cmake
Lines 315 (patched)


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?



cmake/CompilationConfigure.cmake
Lines 318 (patched)


Since this variable will be setable from the outside _Alternative_ seems 
redundant here.



cmake/CompilationConfigure.cmake
Lines 322 (patched)


Could we make clearer what a user needs to do to have this check pass? The 
error message appears too technical to me.



cmake/CompilationConfigure.cmake
Lines 326-329 (patched)


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?


- Benjamin Bannier


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

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66493]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


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

Re: Review Request 66493: Made FreeBSD default to non-GNU ld.

[Mesos Reviewbot Windows]

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['66493']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66493

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66493/logs/mesos-tests-stdout.log):

```
[   OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (123 ms)
[--] 9 tests from Endpoint/SlaveEndpointTest (1084 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (45 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (40 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (86 ms 
total)

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (781 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (812 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (820 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (850 ms total)

[--] Global test environment tear-down
[==] 954 tests from 94 test cases ran. (471114 ms total)
[  PASSED  ] 953 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout

 1 FAILED TEST
  YOU HAVE 214 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66493/logs/mesos-tests-stderr.log):

```
I0407 15:54:06.660614  6244 slave.cpp:3973] Shutting down framework 
9a9994b3-1787-4ad6-8233-b01006c6d4cc-
I0407 15:54:06.660614 11604 master.cpp:10452] Updating the state of task 
11cd7869-bc96-4572-9fd5-50a79f56ea4f of framework 
9a9994b3-1787-4ad6-8233-b01006c6d4cc- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0407 15:54:06.660614  6244 slave.cpp:6670] Shutting down executor 
'11cd7869-bc96-4572-9fd5-50a79f56ea4f' of framework 
9a9994b3-1787-4ad6-8233-b01006c6d4cc-I0407 15:54:06.476617 10892 
exec.cpp:162] Version: 1.6.0
I0407 15:54:06.505643 19036 exec.cpp:236] Executor registered on agent 
9a9994b3-1787-4ad6-8233-b01006c6d4cc-S0
I0407 15:54:06.509647 17620 executor.cpp:177] Received SUBSCRIBED event
I0407 15:54:06.515609 17620 executor.cpp:181] Subscribed executor on 
winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0407 15:54:06.515609 17620 executor.cpp:177] Received LAUNCH event
I0407 15:54:06.520608 17620 executor.cpp:649] Starting task 
11cd7869-bc96-4572-9fd5-50a79f56ea4f
I0407 15:54:06.605608 17620 executor.cpp:484] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0407 15:54:06.633612 17620 executor.cpp:662] Forked command at 19160
I0407 15:54:06.662616  6500 exec.cpp:445] Executor asked to shutdown
I0407 15:54:06.663614 18180 executor.cpp:177] Received SHUTDOWN event
I0407 15:54:06.663614 18180 executor.cpp:759] Shutting down
I0407 15:54:06.663614 18180 executor.cpp:869] Sending SIGTERM to process tree 
at pid  at executor(1)@10.3.1.8:51782
I0407 15:54:06.662616  6244 slave.cpp:923] Agent terminating
W0407 15:54:06.662616  6244 slave.cpp:3969] Ignoring shutdown framework 
9a9994b3-1787-4ad6-8233-b01006c6d4cc- because it is terminating
I0407 15:54:06.663614 11604 master.cpp:10551] Removing task 
11cd7869-bc96-4572-9fd5-50a79f56ea4f with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 9a9994b3-1787-4ad6-8233-b01006c6d4cc- on 
agent 9a9994b3-1787-4ad6-8233-b01006c6d4cc-S0 at slave(423)@10.3.1.8:51761 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0407 15:54:06.665613 12344 containerizer.cpp:2338] Destroying container 
eecdbd30-1cf4-4c9b-8485-0e1260cf5ada in RUNNING state
I0407 15:54:06.13 12344 containerizer.cpp:2952] Transitioning the state of 
container eecdbd30-1cf4-4c9b-8485-0e1260cf5ada from RUNNING to DESTROYING
I0407 15:54:06.13 11604 master.cpp:1295] Agent 
9a9994b3-1787-4ad6-8233-b01006c6d4cc-S0 at slave(423)@10.3.1.8:51761 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0407 15:54:06.13 17440 hierarchical.cpp:344] Removed framework 
9a9994b3-1787-4ad6-8233-b01006c6d4cc-
I0407 15:54:06.667620 11604 master.cpp:3286] Disconnecting agent 
9a99

Review Request 66493: Made FreeBSD default to non-GNU ld.

[David Forsythe]

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

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