Re: Review Request 45668: Enable CMake build for Linux as an extra COMPILER option.

2016-04-04 Thread Juan Larriba


> On Abr. 4, 2016, 12:32 p.m., Joerg Schad wrote:
> > support/docker_build.sh, line 19
> > 
> >
> > What is the reason for this change? Was it a bug before? (Feel free to 
> > drop if there is a good reason, it just seems unclear at this point why you 
> > changed it).

Yes, it is a bug, at least on Ubuntu and CentOS:

[root@jlarriba ~]# dirname "$0"
dirname: invalid option -- 'b'
Try 'dirname --help' for more information.

[root@jlarriba ~]# dirname -- "$0"
.


> On Abr. 4, 2016, 12:32 p.m., Joerg Schad wrote:
> > support/docker_build.sh, line 21
> > 
> >
> > Do we need these echo 1 etc for something? Or is it just for debugging?

It is indeed for debugging; It should not be there.


> On Abr. 4, 2016, 12:32 p.m., Joerg Schad wrote:
> > support/docker_build.sh, line 90
> > 
> >
> > again it feels weird to see cmake on the same level as gcc vs clang.
> > Shouldn't these decisions be orthogonal (i.e., using Cmake and gcc, 
> > cmake and clang, autotools and gcc, ...)

I don't know about this. Currently CMake always uses gcc to compile, I have not 
tried to use clang with CMake yet. However, making it orthogonal makes sense, I 
will try to build using cmake with clang and make it orthogonal.


> On Abr. 4, 2016, 12:32 p.m., Joerg Schad wrote:
> > support/docker_build.sh, line 91
> > 
> >
> > Why is this needed here?

Just to notice the build that cmake is being used, just in case. However, if we 
make the build orthogonal, this will dissapear.


> On Abr. 4, 2016, 12:32 p.m., Joerg Schad wrote:
> > support/docker_build.sh, line 1
> > 
> >
> > When testing did you test both cmake and the non-cmake way on both 
> > systems? If so could you make this explicit in testing done? 
> > THX!

I did not test the non-cmake way, but I will and modify the testing done field.


- Juan


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


On Abr. 4, 2016, 6:54 a.m., Juan Larriba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45668/
> ---
> 
> (Updated Abr. 4, 2016, 6:54 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [MESOS-5101] Enable CMake build for Linux as an extra COMPILER option.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh e9b1d7219b261475fb29118ee27d11743c2c5e0d 
> 
> Diff: https://reviews.apache.org/r/45668/diff/
> 
> 
> Testing
> ---
> 
> Built using docker_build.sh on both centos:7 and ubuntu:14.04
> 
> 
> Thanks,
> 
> Juan Larriba
> 
>



Re: Review Request 45668: Enable CMake build for Linux as an extra COMPILER option.

2016-04-04 Thread Joerg Schad

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




support/docker_build.sh (line 1)


When testing did you test both cmake and the non-cmake way on both systems? 
If so could you make this explicit in testing done? 
THX!



support/docker_build.sh (line 19)


What is the reason for this change? Was it a bug before? (Feel free to drop 
if there is a good reason, it just seems unclear at this point why you changed 
it).



support/docker_build.sh (line 21)


Do we need these echo 1 etc for something? Or is it just for debugging?



support/docker_build.sh (line 90)


again it feels weird to see cmake on the same level as gcc vs clang.
Shouldn't these decisions be orthogonal (i.e., using Cmake and gcc, cmake 
and clang, autotools and gcc, ...)



support/docker_build.sh (line 91)


Why is this needed here?


- Joerg Schad


On April 4, 2016, 6:54 a.m., Juan Larriba wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45668/
> ---
> 
> (Updated April 4, 2016, 6:54 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [MESOS-5101] Enable CMake build for Linux as an extra COMPILER option.
> 
> 
> Diffs
> -
> 
>   support/docker_build.sh e9b1d7219b261475fb29118ee27d11743c2c5e0d 
> 
> Diff: https://reviews.apache.org/r/45668/diff/
> 
> 
> Testing
> ---
> 
> Built using docker_build.sh on both centos:7 and ubuntu:14.04
> 
> 
> Thanks,
> 
> Juan Larriba
> 
>



Re: Review Request 45668: Enable CMake build for Linux as an extra COMPILER option.

2016-04-04 Thread Juan Larriba

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

(Updated Abr. 4, 2016, 6:54 a.m.)


Review request for mesos, Alex Clemmer and Vinod Kone.


Repository: mesos


Description (updated)
---

[MESOS-5101] Enable CMake build for Linux as an extra COMPILER option.


Diffs
-

  support/docker_build.sh e9b1d7219b261475fb29118ee27d11743c2c5e0d 

Diff: https://reviews.apache.org/r/45668/diff/


Testing
---

Built using docker_build.sh on both centos:7 and ubuntu:14.04


Thanks,

Juan Larriba



Review Request 45668: Enable CMake build for Linux as an extra COMPILER option.

2016-04-04 Thread Juan Larriba

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

Review request for mesos, Alex Clemmer and Vinod Kone.


Repository: mesos


Description
---

Enable CMake build for Linux as an extra COMPILER option.


Diffs
-

  support/docker_build.sh e9b1d7219b261475fb29118ee27d11743c2c5e0d 

Diff: https://reviews.apache.org/r/45668/diff/


Testing
---

Built using docker_build.sh on both centos:7 and ubuntu:14.04


Thanks,

Juan Larriba