You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2015/08/24 23:54:41 UTC

Re: Review Request 33752: MESOS-2537: Fix AC_ARG_ENABLED option processing

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

(Updated Aug. 24, 2015, 9:54 p.m.)


Review request for mesos, Cody Maloney and Timothy St. Clair.


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


Repository: mesos


Description
-------

In a number of places, the Mesos configure script passes "$foo=yes"
to the 2nd argument of AC_ARG_ENABLED. However, the 2nd argument
is invoked when the option is provided in any form, not just when
the --enable-foo form is used. One result of this is that
--disable-optimize doesn't work.

The correct handling of the 2nd argument is to save the value of
"$enableval". This change sets the value of all the enable variables
using $enableval, and sets the default value based on the option
name.

There are a number of enable options that were internally named
"$with_foo" and "$without_foo". Rename these to "$enable_foo" for
clarity and to remove the need for both a with_ and a without_
version.

Finally, emit the compilation flags at the end of the configure
phase so it is easier to see the results of your configuration
options.


Diffs (updated)
-----

  configure.ac db0632d60a6d82ab396931b4d913f34b625a45a7 

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


Testing
-------

Configure and build on CentOS 7 and Mac OS X 10.10.3. Verify that the status summary reflects the expected compiler flags. Verify that --enable-foo and --disable-foo do different things.


Thanks,

James Peach


Re: Review Request 33752: MESOS-2537: Fix AC_ARG_ENABLED option processing

Posted by James Peach <jp...@apache.org>.

> On Aug. 26, 2015, 9:01 p.m., Timothy St. Clair wrote:
> >
> 
> Timothy St. Clair wrote:
>     Have you tested all combinations here?

No, only some of the possible combinations. The addition of buld flags output makes is easier to tell when it it doing the right thing. This change doesn't really change any logic except for ```--enable-debug``` and ```--enable-optimize```, since the absence of defaults for those is confusing.


> On Aug. 26, 2015, 9:01 p.m., Timothy St. Clair wrote:
> > configure.ac, line 145
> > <https://reviews.apache.org/r/33752/diff/5/?file=1055235#file1055235line145>
> >
> >     doesn't this change default toggle to an evaluation of the output variable?  
> >     
> >     Shouldn't this just be a toggle on existance?  
> >     
> >     I'm ok on uses of "$withval" but do you have an arguement for the enableval?

The ```--enable-foo``` syntax also supports ```--disable-foo```. So if the build option is not specified, the default should apply, otherwise the operator can specify the build option must be present or absent. For example, our internal RPM build was specifying ```--disable-perftools```, but this had the unintended effect of enabling perftools.


- James


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


On Aug. 26, 2015, 8:11 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33752/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2015, 8:11 p.m.)
> 
> 
> Review request for mesos, Cody Maloney and Timothy St. Clair.
> 
> 
> Bugs: MESOS-2537
>     https://issues.apache.org/jira/browse/MESOS-2537
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In a number of places, the Mesos configure script passes "$foo=yes"
> to the 2nd argument of AC_ARG_ENABLED. However, the 2nd argument
> is invoked when the option is provided in any form, not just when
> the --enable-foo form is used. One result of this is that
> --disable-optimize doesn't work.
> 
> The correct handling of the 2nd argument is to save the value of
> "$enableval". This change sets the value of all the enable variables
> using $enableval, and sets the default value based on the option
> name.
> 
> There are a number of enable options that were internally named
> "$with_foo" and "$without_foo". Rename these to "$enable_foo" for
> clarity and to remove the need for both a with_ and a without_
> version.
> 
> Finally, emit the compilation flags at the end of the configure
> phase so it is easier to see the results of your configuration
> options.
> 
> 
> Diffs
> -----
> 
>   configure.ac 87461d73ed04c4cf176c3475ded9f98dadcda608 
> 
> Diff: https://reviews.apache.org/r/33752/diff/
> 
> 
> Testing
> -------
> 
> Configure and build on CentOS 7 and Mac OS X 10.10.3. Verify that the status summary reflects the expected compiler flags. Verify that --enable-foo and --disable-foo do different things.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 33752: MESOS-2537: Fix AC_ARG_ENABLED option processing

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

> On Aug. 26, 2015, 9:01 p.m., Timothy St. Clair wrote:
> >

Have you tested all combinations here?


- Timothy


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


On Aug. 26, 2015, 8:11 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33752/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2015, 8:11 p.m.)
> 
> 
> Review request for mesos, Cody Maloney and Timothy St. Clair.
> 
> 
> Bugs: MESOS-2537
>     https://issues.apache.org/jira/browse/MESOS-2537
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In a number of places, the Mesos configure script passes "$foo=yes"
> to the 2nd argument of AC_ARG_ENABLED. However, the 2nd argument
> is invoked when the option is provided in any form, not just when
> the --enable-foo form is used. One result of this is that
> --disable-optimize doesn't work.
> 
> The correct handling of the 2nd argument is to save the value of
> "$enableval". This change sets the value of all the enable variables
> using $enableval, and sets the default value based on the option
> name.
> 
> There are a number of enable options that were internally named
> "$with_foo" and "$without_foo". Rename these to "$enable_foo" for
> clarity and to remove the need for both a with_ and a without_
> version.
> 
> Finally, emit the compilation flags at the end of the configure
> phase so it is easier to see the results of your configuration
> options.
> 
> 
> Diffs
> -----
> 
>   configure.ac 87461d73ed04c4cf176c3475ded9f98dadcda608 
> 
> Diff: https://reviews.apache.org/r/33752/diff/
> 
> 
> Testing
> -------
> 
> Configure and build on CentOS 7 and Mac OS X 10.10.3. Verify that the status summary reflects the expected compiler flags. Verify that --enable-foo and --disable-foo do different things.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 33752: MESOS-2537: Fix AC_ARG_ENABLED option processing

Posted by "Timothy St. Clair" <ts...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33752/#review96588
-----------------------------------------------------------



configure.ac (line 145)
<https://reviews.apache.org/r/33752/#comment152174>

    doesn't this change default toggle to an evaluation of the output variable?  
    
    Shouldn't this just be a toggle on existance?  
    
    I'm ok on uses of "$withval" but do you have an arguement for the enableval?


- Timothy St. Clair


On Aug. 26, 2015, 8:11 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33752/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2015, 8:11 p.m.)
> 
> 
> Review request for mesos, Cody Maloney and Timothy St. Clair.
> 
> 
> Bugs: MESOS-2537
>     https://issues.apache.org/jira/browse/MESOS-2537
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In a number of places, the Mesos configure script passes "$foo=yes"
> to the 2nd argument of AC_ARG_ENABLED. However, the 2nd argument
> is invoked when the option is provided in any form, not just when
> the --enable-foo form is used. One result of this is that
> --disable-optimize doesn't work.
> 
> The correct handling of the 2nd argument is to save the value of
> "$enableval". This change sets the value of all the enable variables
> using $enableval, and sets the default value based on the option
> name.
> 
> There are a number of enable options that were internally named
> "$with_foo" and "$without_foo". Rename these to "$enable_foo" for
> clarity and to remove the need for both a with_ and a without_
> version.
> 
> Finally, emit the compilation flags at the end of the configure
> phase so it is easier to see the results of your configuration
> options.
> 
> 
> Diffs
> -----
> 
>   configure.ac 87461d73ed04c4cf176c3475ded9f98dadcda608 
> 
> Diff: https://reviews.apache.org/r/33752/diff/
> 
> 
> Testing
> -------
> 
> Configure and build on CentOS 7 and Mac OS X 10.10.3. Verify that the status summary reflects the expected compiler flags. Verify that --enable-foo and --disable-foo do different things.
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 33752: MESOS-2537: Fix AC_ARG_ENABLED option processing

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33752/
-----------------------------------------------------------

(Updated Aug. 26, 2015, 8:11 p.m.)


Review request for mesos, Cody Maloney and Timothy St. Clair.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

In a number of places, the Mesos configure script passes "$foo=yes"
to the 2nd argument of AC_ARG_ENABLED. However, the 2nd argument
is invoked when the option is provided in any form, not just when
the --enable-foo form is used. One result of this is that
--disable-optimize doesn't work.

The correct handling of the 2nd argument is to save the value of
"$enableval". This change sets the value of all the enable variables
using $enableval, and sets the default value based on the option
name.

There are a number of enable options that were internally named
"$with_foo" and "$without_foo". Rename these to "$enable_foo" for
clarity and to remove the need for both a with_ and a without_
version.

Finally, emit the compilation flags at the end of the configure
phase so it is easier to see the results of your configuration
options.


Diffs (updated)
-----

  configure.ac 87461d73ed04c4cf176c3475ded9f98dadcda608 

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


Testing
-------

Configure and build on CentOS 7 and Mac OS X 10.10.3. Verify that the status summary reflects the expected compiler flags. Verify that --enable-foo and --disable-foo do different things.


Thanks,

James Peach