You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@mesos.apache.org by Cody Maloney <co...@mesosphere.io> on 2014/10/23 22:59:32 UTC

Re: Review Request 26426: Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos

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

(Updated Oct. 23, 2014, 8:59 p.m.)

Review request for mesos, Benjamin Hindman and Timothy St. Clair.

Changes
-------

Largely a complete rewrite. See updated description.

Summary (updated)
-----------------

Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos

Repository: mesos-git

Description (updated)
-------

Reworks buiding mesos in a "debug" vs. a "release" configuration. By default, mesos is built in a developer-centric setup (No optimizations, minimal debug info), in order to maximize developer productivity

None: '-O0 -g1'
--enable-optimize == '-O2'
--enable-debug == '-g'
--enable-optimize --enable-debug == '-O2 -g'

If a user / developer passes CXXFLAGS or CFLAGS manually, then they are not changed / touched at all. This is important so that Mesos is a good citizen when being built for various distributions (As well as making it so specialized one-off groupings of flags are feasible to use).

Adds two defines for accessing what mode things are being built in: 'DEBUG' and 'OPTIMIZE' which can be hooked into later to enable extra logging and the like. For release builds we may want to set 'NDEBUG' which removes assert()'s, but that is a seperate discussion.

Diffs (updated)
-----

configure.ac 2b372e06006250b5230956ef096473e98f3fa590

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

Testing (updated)
-------

Tested all four combinations of flags, making sure they set the right CXXFLAGS. Also specified

Thanks,

Cody Maloney

Re: Review Request 26426: Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos

Posted by "Timothy St. Clair" <ts...@redhat.com>.

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

Ship it!


Just noting that in our code base we've about picked either [action-if-not-given], or [action-if-give] but we should probably do both.

- Timothy St. Clair


On Oct. 23, 2014, 8:59 p.m., Cody Maloney wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26426/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2014, 8:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Timothy St. Clair.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Reworks buiding mesos in a "debug" vs. a "release" configuration. By default, mesos is built in a developer-centric setup (No optimizations, minimal debug info), in order to maximize developer productivity
> 
> None: '-O0 -g1'
> --enable-optimize == '-O2'
> --enable-debug == '-g'
> --enable-optimize --enable-debug == '-O2 -g'
> 
> If a user / developer passes CXXFLAGS or CFLAGS manually, then they are not changed / touched at all. This is important so that Mesos is a good citizen when being built for various distributions (As well as making it so specialized one-off groupings of flags are feasible to use).
> 
> Adds two defines for accessing what mode things are being built in: 'DEBUG' and 'OPTIMIZE' which can be hooked into later to enable extra logging and the like. For release builds we may want to set 'NDEBUG' which removes assert()'s, but that is a seperate discussion.
> 
> 
> Diffs
> -----
> 
>   configure.ac 2b372e06006250b5230956ef096473e98f3fa590 
> 
> Diff: https://reviews.apache.org/r/26426/diff/
> 
> 
> Testing
> -------
> 
> Tested all four combinations of flags, making sure they set the right CXXFLAGS. Also specified
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 26426: Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos

Posted by Cody Maloney <co...@mesosphere.io>.

(Updated Oct. 24, 2014, 9:35 p.m.)

Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy St. Clair.

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

Repository: mesos-git

Description
-------

None: '-O0 -g1'
--enable-optimize == '-O2'
--enable-debug == '-g'
--enable-optimize --enable-debug == '-O2 -g'

Diffs
-----

configure.ac 2b372e06006250b5230956ef096473e98f3fa590

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

Testing
-------

Tested all four combinations of flags, making sure they set the right CXXFLAGS. Also specified

Thanks,

Cody Maloney

Re: Review Request 26426: Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos

Posted by Cody Maloney <co...@mesosphere.io>.

(Updated Oct. 24, 2014, 9:29 p.m.)

Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy St. Clair.

Changes
-------

Update per Dominic Hamon and Ben Mahler's comments.

Repository: mesos-git

Description
-------

None: '-O0 -g1'
--enable-optimize == '-O2'
--enable-debug == '-g'
--enable-optimize --enable-debug == '-O2 -g'

Diffs (updated)
-----

configure.ac 2b372e06006250b5230956ef096473e98f3fa590

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

Testing
-------

Tested all four combinations of flags, making sure they set the right CXXFLAGS. Also specified

Thanks,

Cody Maloney

Re: Review Request 26426: Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos

Posted by Ben Mahler <be...@gmail.com>.

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

Ship it!


Thanks Cody! Just some trivial comments below and we can get this committed!


configure.ac
<https://reviews.apache.org/r/26426/#comment99282>

    Can you wrap after the comma to be consistent with the other AC_ARG_ENABLE blocks? Any reason to do it differently here?



configure.ac
<https://reviews.apache.org/r/26426/#comment99283>

    How about s/$debug/$debug_flags/ and s/$optimize/$optimize_flags/ ?


- Ben Mahler


On Oct. 23, 2014, 9:20 p.m., Cody Maloney wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26426/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2014, 9:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy St. Clair.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Reworks buiding mesos in a "debug" vs. a "release" configuration. By default, mesos is built in a developer-centric setup (No optimizations, minimal debug info), in order to maximize developer productivity
> 
> None: '-O0 -g1'
> --enable-optimize == '-O2'
> --enable-debug == '-g'
> --enable-optimize --enable-debug == '-O2 -g'
> 
> If a user / developer passes CXXFLAGS or CFLAGS manually, then they are not changed / touched at all. This is important so that Mesos is a good citizen when being built for various distributions (As well as making it so specialized one-off groupings of flags are feasible to use).
> 
> Adds two defines for accessing what mode things are being built in: 'DEBUG' and 'OPTIMIZE' which can be hooked into later to enable extra logging and the like. For release builds we may want to set 'NDEBUG' which removes assert()'s, but that is a seperate discussion.
> 
> 
> Diffs
> -----
> 
>   configure.ac 2b372e06006250b5230956ef096473e98f3fa590 
> 
> Diff: https://reviews.apache.org/r/26426/diff/
> 
> 
> Testing
> -------
> 
> Tested all four combinations of flags, making sure they set the right CXXFLAGS. Also specified
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 26426: Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos

Posted by Mesos ReviewBot <de...@mesos.apache.org>.

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


Patch looks great!

Reviews applied: [26426]

All tests passed.

- Mesos ReviewBot


On Oct. 23, 2014, 9:20 p.m., Cody Maloney wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26426/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2014, 9:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy St. Clair.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Reworks buiding mesos in a "debug" vs. a "release" configuration. By default, mesos is built in a developer-centric setup (No optimizations, minimal debug info), in order to maximize developer productivity
> 
> None: '-O0 -g1'
> --enable-optimize == '-O2'
> --enable-debug == '-g'
> --enable-optimize --enable-debug == '-O2 -g'
> 
> If a user / developer passes CXXFLAGS or CFLAGS manually, then they are not changed / touched at all. This is important so that Mesos is a good citizen when being built for various distributions (As well as making it so specialized one-off groupings of flags are feasible to use).
> 
> Adds two defines for accessing what mode things are being built in: 'DEBUG' and 'OPTIMIZE' which can be hooked into later to enable extra logging and the like. For release builds we may want to set 'NDEBUG' which removes assert()'s, but that is a seperate discussion.
> 
> 
> Diffs
> -----
> 
>   configure.ac 2b372e06006250b5230956ef096473e98f3fa590 
> 
> Diff: https://reviews.apache.org/r/26426/diff/
> 
> 
> Testing
> -------
> 
> Tested all four combinations of flags, making sure they set the right CXXFLAGS. Also specified
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 26426: Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos

Posted by Cody Maloney <co...@mesosphere.io>.


> On Oct. 23, 2014, 9:50 p.m., Dominic Hamon wrote:
> > configure.ac, line 289
> > <https://reviews.apache.org/r/26426/diff/3/?file=731214#file731214line289>
> >
> >     might be simpler to set a default then override:
> >     
> >         debug="-g1"
> >         if ...
> >           debug="-g"
> >         else if ...
> >           debug=""
> >         fi
> >     
> >         optimize="-O0"
> >         if ...
> >           optimize="-O2"
> >         fi

Updated for the debug case where there is a defualt then a "in other cases do this, in other cases the default should be different".

For optimize I like that I see I will definitively get either -O0 or -O2 based on which way the if goes.


- Cody


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


On Oct. 24, 2014, 9:29 p.m., Cody Maloney wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26426/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2014, 9:29 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy St. Clair.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Reworks buiding mesos in a "debug" vs. a "release" configuration. By default, mesos is built in a developer-centric setup (No optimizations, minimal debug info), in order to maximize developer productivity
> 
> None: '-O0 -g1'
> --enable-optimize == '-O2'
> --enable-debug == '-g'
> --enable-optimize --enable-debug == '-O2 -g'
> 
> If a user / developer passes CXXFLAGS or CFLAGS manually, then they are not changed / touched at all. This is important so that Mesos is a good citizen when being built for various distributions (As well as making it so specialized one-off groupings of flags are feasible to use).
> 
> Adds two defines for accessing what mode things are being built in: 'DEBUG' and 'OPTIMIZE' which can be hooked into later to enable extra logging and the like. For release builds we may want to set 'NDEBUG' which removes assert()'s, but that is a seperate discussion.
> 
> 
> Diffs
> -----
> 
>   configure.ac 2b372e06006250b5230956ef096473e98f3fa590 
> 
> Diff: https://reviews.apache.org/r/26426/diff/
> 
> 
> Testing
> -------
> 
> Tested all four combinations of flags, making sure they set the right CXXFLAGS. Also specified
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 26426: Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos

Posted by Dominic Hamon <dh...@twopensource.com>.

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



configure.ac
<https://reviews.apache.org/r/26426/#comment99023>

    might be simpler to set a default then override:
    
        debug="-g1"
        if ...
          debug="-g"
        else if ...
          debug=""
        fi
    
        optimize="-O0"
        if ...
          optimize="-O2"
        fi


- Dominic Hamon


On Oct. 23, 2014, 2:20 p.m., Cody Maloney wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26426/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2014, 2:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy St. Clair.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Reworks buiding mesos in a "debug" vs. a "release" configuration. By default, mesos is built in a developer-centric setup (No optimizations, minimal debug info), in order to maximize developer productivity
> 
> None: '-O0 -g1'
> --enable-optimize == '-O2'
> --enable-debug == '-g'
> --enable-optimize --enable-debug == '-O2 -g'
> 
> If a user / developer passes CXXFLAGS or CFLAGS manually, then they are not changed / touched at all. This is important so that Mesos is a good citizen when being built for various distributions (As well as making it so specialized one-off groupings of flags are feasible to use).
> 
> Adds two defines for accessing what mode things are being built in: 'DEBUG' and 'OPTIMIZE' which can be hooked into later to enable extra logging and the like. For release builds we may want to set 'NDEBUG' which removes assert()'s, but that is a seperate discussion.
> 
> 
> Diffs
> -----
> 
>   configure.ac 2b372e06006250b5230956ef096473e98f3fa590 
> 
> Diff: https://reviews.apache.org/r/26426/diff/
> 
> 
> Testing
> -------
> 
> Tested all four combinations of flags, making sure they set the right CXXFLAGS. Also specified
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Re: Review Request 26426: Add --enable-debug and --enable-optimize flag for controlling building debug and optimized verisons of mesos

Posted by Cody Maloney <co...@mesosphere.io>.

(Updated Oct. 23, 2014, 9:20 p.m.)

Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy St. Clair.

Changes
-------

Adding bmahler to review.

Repository: mesos-git

Description
-------

None: '-O0 -g1'
--enable-optimize == '-O2'
--enable-debug == '-g'
--enable-optimize --enable-debug == '-O2 -g'

Diffs
-----

configure.ac 2b372e06006250b5230956ef096473e98f3fa590

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

Testing
-------

Tested all four combinations of flags, making sure they set the right CXXFLAGS. Also specified

Thanks,

Cody Maloney