> On March 26, 2016, 6:43 a.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. > > Jiang Yan Xu wrote: > 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?
If you do it this way, you get the headers, etc check output intermingled in the middle of your nice clean feature check message. - James ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44945/#review125457 ----------------------------------------------------------- On March 26, 2016, 5:05 p.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44945/ > ----------------------------------------------------------- > > (Updated March 26, 2016, 5:05 p.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 > >
