> On March 25, 2016, 11:43 p.m., Jiang Yan Xu wrote:
> > configure.ac, lines 957-960
> > <https://reviews.apache.org/r/44945/diff/8/?file=1311200#file1311200line957>
> >
> >     Can we put AC_MSG_CHECKING before `AC_CHECK_HEADERS` and inside `AS_IF` 
> > above?
> >     
> >     i.e., AC_MSG_CHECKING/AC_MSG_CHECKING/AC_MSG_ERROR inform user what 
> > they don't already know (whether the system dependencies are met), not what 
> > they already know (the fact that they provided the 
> > `--enable_xfs_disk_isolator` flag).
> >     
> >     And this check is conditional: we only print `AC_MSG_CHECKING` and do 
> > these checks when `test "x$enable_xfs_disk_isolator" = "xyes"`; we print 
> > `AC_MSG_ERROR([...])` when such checks fail and `AC_MSG_RESULT([yes])` when 
> > all checks succeed.
> >     
> >     Would this work?
> >     
> >     If we do this, then the message could be `AC_MSG_CHECKING([whether we 
> > can enable the XFS disk isolator])`
> 
> James Peach wrote:
>     No, this message should be outside the check so that the builder always 
> knows what happened, independent of what configure flags they passed. 
> Typically, people start by not passing any flags, and it is helpful to 
> explicitly log that a feature was not enabled. Even if you pass the --enable 
> flags, it is comforting to get an explicit confirmation that the feature you 
> asked for is enabled.

I agree that explicitly confirming that the user has enabled something is 
better. 

I still don't think we should print it after we've done all the dependency 
checks for it and have potentially aborted configure with error messages but 
without telling the error is due to "whether to enable the XFS disk isolator..."

i.e., the following is better.

```
checking whether to enable the XFS disk isolator... yes

OR 

checking whether to enable the XFS disk isolator... missing libblkid headers
-------------------------------------------------------------------
Please install the libblkid development package.
-------------------------------------------------------------------

OR

checking whether to enable the XFS disk isolator... no / checking whether to 
enable the XFS disk isolator... not enabled by user

```

Agreed?


- Jiang Yan


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


On March 26, 2016, 10:05 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44945/
> -----------------------------------------------------------
> 
> (Updated March 26, 2016, 10:05 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4828
>     https://issues.apache.org/jira/browse/MESOS-4828
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add autoconf tests for XFS project quotas.
> 
> 
> Diffs
> -----
> 
>   configure.ac 9ec4bc1cff3b0b46dd2e7ece2c1fbbbb2d19ffb8 
> 
> Diff: https://reviews.apache.org/r/44945/diff/
> 
> 
> Testing
> -------
> 
> Make check. Manual verification.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Reply via email to