Re: Review Request 70047: Updated build specific artefact generation.

2019-03-05 Thread Till Toenshoff via Review Board 

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

(Updated March 6, 2019, 1:12 a.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


Bugs: MESOS-9605
https://issues.apache.org/jira/browse/MESOS-9605


Repository: mesos


Description
---

For autotools, we extracted additional build info like the git branch
and sha during the automake phase handing them into libbuild via
commandline defines.

CMake builds however used a configuration file for this purpose.

This patch updates both build systems to make use of
build_git_config.hpp.in for build specific git information.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
  configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
  src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
  src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
  src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
  src/common/git_version.hpp.in PRE-CREATION 


Diff: https://reviews.apache.org/r/70047/diff/6/

Changes: https://reviews.apache.org/r/70047/diff/5-6/


Testing
---

Manually tested both cmake and autotools.

First configure run:
```
[...]
checking for src/common/build_git_config.hpp... no
configure: creating src/common/build_git_config.hpp
[...]

```

Subsequent configure runs:
```
[...]
checking for src/common/build_git_config.hpp... yes
[...]
```

Final build from `support/packaging/centos/build-docker-rpmbuild.sh` installed 
and ran agent;
```
Installed:
  mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7

Complete!
[root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
```


Thanks,

Till Toenshoff

Re: Review Request 70047: Updated build specific artefact generation.

2019-03-05 Thread Till Toenshoff via Review Board 

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

(Updated March 5, 2019, 6:46 p.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


Changes
---

addressed comments


Bugs: MESOS-9605
https://issues.apache.org/jira/browse/MESOS-9605


Repository: mesos


Description
---

For autotools, we extracted additional build info like the git branch
and sha during the automake phase handing them into libbuild via
commandline defines.

CMake builds however used a configuration file for this purpose.

This patch updates both build systems to make use of
build_git_config.hpp.in for build specific git information.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
  configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
  src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
  src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
  src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
  src/common/git_version.hpp.in PRE-CREATION 


Diff: https://reviews.apache.org/r/70047/diff/5/

Changes: https://reviews.apache.org/r/70047/diff/4-5/


Testing
---

Manually tested both cmake and autotools.

First configure run:
```
[...]
checking for src/common/build_git_config.hpp... no
configure: creating src/common/build_git_config.hpp
[...]

```

Subsequent configure runs:
```
[...]
checking for src/common/build_git_config.hpp... yes
[...]
```

Final build from `support/packaging/centos/build-docker-rpmbuild.sh` installed 
and ran agent;
```
Installed:
  mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7

Complete!
[root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
```


Thanks,

Till Toenshoff

Re: Review Request 70047: Updated build specific artefact generation.

2019-03-01 Thread Benjamin Bannier 


> On Feb. 26, 2019, 9:22 p.m., Joseph Wu wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 630-637 (original), 663-667 (patched)
> > 
> >
> > I don't think the CMake build needs to follow the autotools one here.  
> > For one, the packaging for CMake does not need to configure more than once 
> > (assuming we finish the CMake packaging ;).
> > 
> > A comment mentioning the difference should be enough.  i.e. 
> > ```
> > # NOTE: The `build_git_config.hpp.in` file is specific to the autotools 
> > build.
> > # `build_config.hpp.in` includes the definitions for the CMake build.
> > ```
> 
> Till Toenshoff wrote:
> There is indeed only one reason to do this, it is because we might want 
> zero suprises when finally switching over -- waiting for my shepherd to make 
> a call here :)
> 
> Till Toenshoff wrote:
> I totally omitted the source-distribution argument when stating the above 
> -- so while academically speaking we wont need this for cmake based package 
> creation (as done via CPack and not rpmbuild), we will still need it to 
> include a git sha artefact in our source tarballs/packages.

I intended to respond here, https://reviews.apache.org/r/70047/#comment299248. 
Agreeing with Till here to also touch this in cmake for now.


- Benjamin


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


On Feb. 26, 2019, 12:05 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70047/
> ---
> 
> (Updated Feb. 26, 2019, 12:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9605
> https://issues.apache.org/jira/browse/MESOS-9605
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools, we extracted additional build info like the git branch
> and sha during the automake phase handing them into libbuild via
> commandline defines.
> 
> CMake builds however used a configuration file for this purpose.
> 
> This patch updates both build systems to make use of
> build_git_config.hpp.in for build specific git information.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
>   configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
>   src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
>   src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
>   src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
>   src/common/build_git_config.hpp.in PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70047/diff/4/
> 
> 
> Testing
> ---
> 
> Manually tested both cmake and autotools.
> 
> First configure run:
> ```
> [...]
> checking for src/common/build_git_config.hpp... no
> configure: creating src/common/build_git_config.hpp
> [...]
> 
> ```
> 
> Subsequent configure runs:
> ```
> [...]
> checking for src/common/build_git_config.hpp... yes
> [...]
> ```
> 
> Final build from `support/packaging/centos/build-docker-rpmbuild.sh` 
> installed and ran agent;
> ```
> Installed:
>   mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7
> 
> Complete!
> [root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
> I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
> I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
> I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
> c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
> I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
> I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 70047: Updated build specific artefact generation.

2019-02-28 Thread Till Toenshoff via Review Board 


> On Feb. 26, 2019, 8:22 p.m., Joseph Wu wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 630-637 (original), 663-667 (patched)
> > 
> >
> > I don't think the CMake build needs to follow the autotools one here.  
> > For one, the packaging for CMake does not need to configure more than once 
> > (assuming we finish the CMake packaging ;).
> > 
> > A comment mentioning the difference should be enough.  i.e. 
> > ```
> > # NOTE: The `build_git_config.hpp.in` file is specific to the autotools 
> > build.
> > # `build_config.hpp.in` includes the definitions for the CMake build.
> > ```
> 
> Till Toenshoff wrote:
> There is indeed only one reason to do this, it is because we might want 
> zero suprises when finally switching over -- waiting for my shepherd to make 
> a call here :)

I totally omitted the source-distribution argument when stating the above -- so 
while academically speaking we wont need this for cmake based package creation 
(as done via CPack and not rpmbuild), we will still need it to include a git 
sha artefact in our source tarballs/packages.


- Till


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


On Feb. 26, 2019, 11:05 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70047/
> ---
> 
> (Updated Feb. 26, 2019, 11:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9605
> https://issues.apache.org/jira/browse/MESOS-9605
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools, we extracted additional build info like the git branch
> and sha during the automake phase handing them into libbuild via
> commandline defines.
> 
> CMake builds however used a configuration file for this purpose.
> 
> This patch updates both build systems to make use of
> build_git_config.hpp.in for build specific git information.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
>   configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
>   src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
>   src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
>   src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
>   src/common/build_git_config.hpp.in PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70047/diff/4/
> 
> 
> Testing
> ---
> 
> Manually tested both cmake and autotools.
> 
> First configure run:
> ```
> [...]
> checking for src/common/build_git_config.hpp... no
> configure: creating src/common/build_git_config.hpp
> [...]
> 
> ```
> 
> Subsequent configure runs:
> ```
> [...]
> checking for src/common/build_git_config.hpp... yes
> [...]
> ```
> 
> Final build from `support/packaging/centos/build-docker-rpmbuild.sh` 
> installed and ran agent;
> ```
> Installed:
>   mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7
> 
> Complete!
> [root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
> I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
> I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
> I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
> c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
> I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
> I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 70047: Updated build specific artefact generation.

2019-02-28 Thread Till Toenshoff via Review Board 


> On March 1, 2019, 12:02 a.m., Benjamin Bannier wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 630-637 (original), 663-667 (patched)
> > 
> >
> > The idea (at least on the autotools side which currently supports 
> > packaging) is to generate an artifact specifiying git info which is 
> > included with the distribution tarball.
> > 
> > AFAICT, unless we completely rip out autotools while implementing cmake 
> > distribution tarballs, we'll need to implement something like this to 
> > support showing git info when building from distribution tarballs, even in 
> > the cmake build (which would probably emit this also during packaging time).
> > 
> > Suggest to keep as is.

Yup!


> On March 1, 2019, 12:02 a.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 2829 (patched)
> > 
> >
> > Lets use `presence` instead of `for`,
> > 
> > checking src/common/build_git_config.hpp presence ...

ok


> On March 1, 2019, 12:02 a.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 2831 (patched)
> > 
> >
> > Even though there is some nesting I'd prefer more general `AS_IF` and 
> > friends.
> > 
> > Here and below.

ok


> On March 1, 2019, 12:02 a.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 2836 (patched)
> > 
> >
> > `s/creating/generating/`

ok


> On March 1, 2019, 12:02 a.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 2843 (patched)
> > 
> >
> > Do you understand why this command could print something to stderr? It 
> > seems it should be possible to treat the absent case as error and define 
> > the variable with a value immediately.
> > 
> > Also, what do you think about instead
> > ```
> > git --git-dir=${srcdir}"/.git/ rev-parse HEAD ...
> > ```
> > 
> > We could put the git dir into a variable.
> > 
> > Here and below.

> Do you understand why this command could print something to stderr?

Yes - well not the head SHA, that should indeed always work.

Let me take the tag for an example;
```
lobomacpro4:~/Development/mesos-private (ci/till/build-config-revamp ?) $ git 
describe --exact --tags
fatal: no tag exactly matches '9ed7e160f003b5e22bf9c41e0104e0efca1df682'
lobomacpro4:~/Development/mesos-private (ci/till/build-config-revamp ?) $ git 
describe --exact --tags 2>/dev/null
lobomacpro4:~/Development/mesos-private (ci/till/build-config-revamp ?) $ echo 
$?
128
```

Then imagine a detached checkout, that would similarly fail for the branch 
detection.

```
lobomacpro4:~/Development/mesos-private (no branch! ?) $ git symbolic-ref HEAD
fatal: ref HEAD is not a symbolic ref
lobomacpro4:~/Development/mesos-private (no branch! ?) $ git symbolic-ref HEAD 
2>/dev/null
lobomacpro4:~/Development/mesos-private (no branch! ?) $ echo $?
128
```

So I would argue that the current approach for error handling is rather well 
adapted.


> > `git --git-dir=${srcdir}"/.git/ rev-parse HEAD ...`

Much better, thanks!


> On March 1, 2019, 12:02 a.m., Benjamin Bannier wrote:
> > src/common/build_git_config.hpp.in
> > Lines 1 (patched)
> > 
> >
> > Since this isn't about the build but git version, what do you think 
> > abot calling this file e.g., `git_version.hpp.in` or similar?

Yes - good point.


> On March 1, 2019, 12:02 a.m., Benjamin Bannier wrote:
> > src/common/build_git_config.hpp.in
> > Lines 23-25 (patched)
> > 
> >
> > Not really a fan of how we directly emit a CPP macro from autoconf, but 
> > without config headers I cannot really think of a objectively better 
> > approach :(

I wish you could! :(


- Till


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


On Feb. 26, 2019, 11:05 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70047/
> ---
> 
> (Updated Feb. 26, 2019, 11:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9605
> https://issues.apache.org/jira/browse/MESOS-9605
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools, we extracted additional build info like the git branch
> and sha during the automake phase

Re: Review Request 70047: Updated build specific artefact generation.

2019-02-28 Thread Benjamin Bannier 

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


Fix it, then Ship it!




Thanks Till. While I am not a big fan of how we directly emit CPP macros from 
configure, I cannot think of a much better approach.

Left some mostly stylistic points.


cmake/CompilationConfigure.cmake
Lines 630-637 (original), 663-667 (patched)


The idea (at least on the autotools side which currently supports 
packaging) is to generate an artifact specifiying git info which is included 
with the distribution tarball.

AFAICT, unless we completely rip out autotools while implementing cmake 
distribution tarballs, we'll need to implement something like this to support 
showing git info when building from distribution tarballs, even in the cmake 
build (which would probably emit this also during packaging time).

Suggest to keep as is.



configure.ac
Lines 2829 (patched)


Lets use `presence` instead of `for`,

checking src/common/build_git_config.hpp presence ...



configure.ac
Lines 2831 (patched)


Even though there is some nesting I'd prefer more general `AS_IF` and 
friends.

Here and below.



configure.ac
Lines 2836 (patched)


`s/creating/generating/`



configure.ac
Lines 2843 (patched)


Do you understand why this command could print something to stderr? It 
seems it should be possible to treat the absent case as error and define the 
variable with a value immediately.

Also, what do you think about instead
```
git --git-dir=${srcdir}"/.git/ rev-parse HEAD ...
```

We could put the git dir into a variable.

Here and below.



src/common/build_git_config.hpp.in
Lines 1 (patched)


Since this isn't about the build but git version, what do you think abot 
calling this file e.g., `git_version.hpp.in` or similar?



src/common/build_git_config.hpp.in
Lines 23-25 (patched)


Not really a fan of how we directly emit a CPP macro from autoconf, but 
without config headers I cannot really think of a objectively better approach :(


- Benjamin Bannier


On Feb. 26, 2019, 12:05 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70047/
> ---
> 
> (Updated Feb. 26, 2019, 12:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9605
> https://issues.apache.org/jira/browse/MESOS-9605
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools, we extracted additional build info like the git branch
> and sha during the automake phase handing them into libbuild via
> commandline defines.
> 
> CMake builds however used a configuration file for this purpose.
> 
> This patch updates both build systems to make use of
> build_git_config.hpp.in for build specific git information.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
>   configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
>   src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
>   src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
>   src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
>   src/common/build_git_config.hpp.in PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70047/diff/4/
> 
> 
> Testing
> ---
> 
> Manually tested both cmake and autotools.
> 
> First configure run:
> ```
> [...]
> checking for src/common/build_git_config.hpp... no
> configure: creating src/common/build_git_config.hpp
> [...]
> 
> ```
> 
> Subsequent configure runs:
> ```
> [...]
> checking for src/common/build_git_config.hpp... yes
> [...]
> ```
> 
> Final build from `support/packaging/centos/build-docker-rpmbuild.sh` 
> installed and ran agent;
> ```
> Installed:
>   mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7
> 
> Complete!
> [root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
> I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
> I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
> I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
> c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
> I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
> I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
> ```
> 
> 
> Thanks,
> 
> Till

Re: Review Request 70047: Updated build specific artefact generation.

2019-02-28 Thread Till Toenshoff via Review Board 


> On Feb. 26, 2019, 8:22 p.m., Joseph Wu wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 630-637 (original), 663-667 (patched)
> > 
> >
> > I don't think the CMake build needs to follow the autotools one here.  
> > For one, the packaging for CMake does not need to configure more than once 
> > (assuming we finish the CMake packaging ;).
> > 
> > A comment mentioning the difference should be enough.  i.e. 
> > ```
> > # NOTE: The `build_git_config.hpp.in` file is specific to the autotools 
> > build.
> > # `build_config.hpp.in` includes the definitions for the CMake build.
> > ```

There is indeed only one reason to do this, it is because we might want zero 
suprises when finally switching over -- waiting for my shepherd to make a call 
here :)


- Till


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


On Feb. 26, 2019, 11:05 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70047/
> ---
> 
> (Updated Feb. 26, 2019, 11:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9605
> https://issues.apache.org/jira/browse/MESOS-9605
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools, we extracted additional build info like the git branch
> and sha during the automake phase handing them into libbuild via
> commandline defines.
> 
> CMake builds however used a configuration file for this purpose.
> 
> This patch updates both build systems to make use of
> build_git_config.hpp.in for build specific git information.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
>   configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
>   src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
>   src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
>   src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
>   src/common/build_git_config.hpp.in PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70047/diff/4/
> 
> 
> Testing
> ---
> 
> Manually tested both cmake and autotools.
> 
> First configure run:
> ```
> [...]
> checking for src/common/build_git_config.hpp... no
> configure: creating src/common/build_git_config.hpp
> [...]
> 
> ```
> 
> Subsequent configure runs:
> ```
> [...]
> checking for src/common/build_git_config.hpp... yes
> [...]
> ```
> 
> Final build from `support/packaging/centos/build-docker-rpmbuild.sh` 
> installed and ran agent;
> ```
> Installed:
>   mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7
> 
> Complete!
> [root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
> I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
> I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
> I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
> c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
> I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
> I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 70047: Updated build specific artefact generation.

2019-02-26 Thread Joseph Wu 

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




cmake/CompilationConfigure.cmake
Lines 630-637 (original), 663-667 (patched)


I don't think the CMake build needs to follow the autotools one here.  For 
one, the packaging for CMake does not need to configure more than once 
(assuming we finish the CMake packaging ;).

A comment mentioning the difference should be enough.  i.e. 
```
# NOTE: The `build_git_config.hpp.in` file is specific to the autotools 
build.
# `build_config.hpp.in` includes the definitions for the CMake build.
```


- Joseph Wu


On Feb. 26, 2019, 3:05 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70047/
> ---
> 
> (Updated Feb. 26, 2019, 3:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joseph Wu.
> 
> 
> Bugs: MESOS-9605
> https://issues.apache.org/jira/browse/MESOS-9605
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For autotools, we extracted additional build info like the git branch
> and sha during the automake phase handing them into libbuild via
> commandline defines.
> 
> CMake builds however used a configuration file for this purpose.
> 
> This patch updates both build systems to make use of
> build_git_config.hpp.in for build specific git information.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
>   configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
>   src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
>   src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
>   src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
>   src/common/build_git_config.hpp.in PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70047/diff/4/
> 
> 
> Testing
> ---
> 
> Manually tested both cmake and autotools.
> 
> First configure run:
> ```
> [...]
> checking for src/common/build_git_config.hpp... no
> configure: creating src/common/build_git_config.hpp
> [...]
> 
> ```
> 
> Subsequent configure runs:
> ```
> [...]
> checking for src/common/build_git_config.hpp... yes
> [...]
> ```
> 
> Final build from `support/packaging/centos/build-docker-rpmbuild.sh` 
> installed and ran agent;
> ```
> Installed:
>   mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7
> 
> Complete!
> [root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
> I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
> I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
> I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
> c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
> I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
> I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 70047: Updated build specific artefact generation.

2019-02-26 Thread Till Toenshoff via Review Board 

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

(Updated Feb. 26, 2019, 11:05 a.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


Bugs: MESOS-9605
https://issues.apache.org/jira/browse/MESOS-9605


Repository: mesos


Description
---

For autotools, we extracted additional build info like the git branch
and sha during the automake phase handing them into libbuild via
commandline defines.

CMake builds however used a configuration file for this purpose.

This patch updates both build systems to make use of
build_git_config.hpp.in for build specific git information.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
  configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
  src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
  src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
  src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
  src/common/build_git_config.hpp.in PRE-CREATION 


Diff: https://reviews.apache.org/r/70047/diff/4/

Changes: https://reviews.apache.org/r/70047/diff/3-4/


Testing
---

Manually tested both cmake and autotools.

First configure run:
```
[...]
checking for src/common/build_git_config.hpp... no
configure: creating src/common/build_git_config.hpp
[...]

```

Subsequent configure runs:
```
[...]
checking for src/common/build_git_config.hpp... yes
[...]
```

Final build from `support/packaging/centos/build-docker-rpmbuild.sh` installed 
and ran agent;
```
Installed:
  mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7

Complete!
[root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
```


Thanks,

Till Toenshoff

Re: Review Request 70047: Updated build specific artefact generation.

2019-02-25 Thread Till Toenshoff via Review Board 

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

(Updated Feb. 26, 2019, 2:20 a.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


Bugs: MESOS-9605
https://issues.apache.org/jira/browse/MESOS-9605


Repository: mesos


Description (updated)
---

For autotools, we extracted additional build info like the git branch
and sha during the automake phase handing them into libbuild via
commandline defines.

CMake builds however used a configuration file for this purpose.

This patch updates both build systems to make use of
build_git_config.hpp.in for build specific git information.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
  configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
  src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
  src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
  src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
  src/common/build_git_config.hpp.in PRE-CREATION 


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

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


Testing
---

Manually tested both cmake and autotools.

First configure run:
```
[...]
checking for src/common/build_git_config.hpp... no
configure: creating src/common/build_git_config.hpp
[...]

```

Subsequent configure runs:
```
[...]
checking for src/common/build_git_config.hpp... yes
[...]
```

Final build from `support/packaging/centos/build-docker-rpmbuild.sh` installed 
and ran agent;
```
Installed:
  mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7

Complete!
[root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
```


Thanks,

Till Toenshoff

Re: Review Request 70047: Updated build specific artefact generation.

2019-02-25 Thread Till Toenshoff via Review Board 

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

(Updated Feb. 26, 2019, 2:18 a.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


Bugs: MESOS-9605
https://issues.apache.org/jira/browse/MESOS-9605


Repository: mesos


Description (updated)
---

For autotools, we extracted additional build info like the git branch
and sha during the automake phase handing them into libbuild via
commandline supplied defines.

CMake builds however used a configuration file for this purpose.

This patch updates both build systems to make use of
build_git_config.hpp.in for build specific git information. The data
is persisted only once for the initial configuration / cmake run.
Subsequent invocations will not update build_git_config.hpp. We do
this to make sure that snapshots can get built without having git
specific information available at build time.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
  configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
  src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
  src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
  src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 
  src/common/build_git_config.hpp.in PRE-CREATION 


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

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


Testing (updated)
---

Manually tested both cmake and autotools.

First configure run:
```
[...]
checking for src/common/build_git_config.hpp... no
configure: creating src/common/build_git_config.hpp
[...]

```

Subsequent configure runs:
```
[...]
checking for src/common/build_git_config.hpp... yes
[...]
```

Final build from `support/packaging/centos/build-docker-rpmbuild.sh` installed 
and ran agent;
```
Installed:
  mesos.x86_64 0:1.8.0-0.1.pre.20190226gitc125541.el7

Complete!
[root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
I0226 01:00:48.581748   157 main.cpp:350] Build: 2019-02-26 00:36:49 by centos
I0226 01:00:48.581817   157 main.cpp:351] Version: 1.8.0
I0226 01:00:48.581823   157 main.cpp:358] Git SHA: 
c125541b8e4f2c2f6f56fe7e1c2e0b26d5bbfc62
I0226 01:00:48.587003   157 systemd.cpp:240] systemd version `219` detected
I0226 01:00:48.587026   157 main.cpp:453] Initializing systemd state
```


Thanks,

Till Toenshoff

Re: Review Request 70047: Updated build specific artefact generation.

2019-02-24 Thread Till Toenshoff via Review Board 

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

(Updated Feb. 25, 2019, 2:35 a.m.)


Review request for mesos, Benjamin Bannier and Joseph Wu.


Bugs: MESOS-9605
https://issues.apache.org/jira/browse/MESOS-9605


Repository: mesos


Description
---

For autotools, we extracted additional build info like the git branch
and sha during the automake phase handing them into libbuild via
commandline defines.

CMake builds however used a configuration file for this purpose.

This patch updates both build systems to make use of
build_config.hpp.in for build specific configuration artefacts.


Diffs
-

  cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
  configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
  src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
  src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
  src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 


Diff: https://reviews.apache.org/r/70047/diff/1/


Testing (updated)
---

manually tested both cmake and autotools.

WIP: A test using the centos RPM toolchain does not yield the intended result;
```
[root@9b0d899ff4c6 ~]# mesos-slave --work_dir=/tmp --master=127.0.0.1:5050
I0225 02:30:16.11352897 main.cpp:350] Build: 2019-02-25 01:59:55 by centos
I0225 02:30:16.11360697 main.cpp:351] Version: 1.8.0
I0225 02:30:16.12117397 systemd.cpp:240] systemd version `219` detected
I0225 02:30:16.12119997 main.cpp:453] Initializing systemd state
E0225 02:30:16.12125197 main.cpp:462] EXIT with status 1: Failed to 
initialize systemd: Failed to locate systemd runtime directory: 
/run/systemd/system
```

It still lacks the SHA!


Thanks,

Till Toenshoff

Review Request 70047: Updated build specific artefact generation.

2019-02-24 Thread Till Toenshoff via Review Board 

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

Review request for mesos, Benjamin Bannier and Joseph Wu.


Bugs: MESOS-9605
https://issues.apache.org/jira/browse/MESOS-9605


Repository: mesos


Description
---

For autotools, we extracted additional build info like the git branch
and sha during the automake phase handing them into libbuild via
commandline defines.

CMake builds however used a configuration file for this purpose.

This patch updates both build systems to make use of
build_config.hpp.in for build specific configuration artefacts.


Diffs
-

  cmake/CompilationConfigure.cmake c330324e2e3dea6e71980ae8c9ed71632ebb018e 
  configure.ac ee29fc784e53ebaf1bb016c33136b200c646ee9b 
  src/Makefile.am 283d5ed89b36d74da36f38c26aec03c6129d6261 
  src/common/build.cpp f5271d87d33ac429fb94093a347be1d6c25d3432 
  src/common/build_config.hpp.in 4cce2403c1d7a5feee8fd2fffa7cf4308507cd0c 


Diff: https://reviews.apache.org/r/70047/diff/1/


Testing
---

manually tested both cmake and autotools.


Thanks,

Till Toenshoff