> On July 7, 2017, 1:15 p.m., Alexander Rojas wrote: > > src/linux/perf.cpp > > Lines 243 (patched) > > <https://reviews.apache.org/r/60397/diff/3/?file=1768325#file1768325line243> > > > > Why this command? two lines later `perf::version()` is called. Isn't > > there a way to merge this with the `perf::version()` call?
`perf::version()` also used in tests: ```cpp // Returns the detected perf version. Exposed for testing. process::Future<Version> version(); ``` If I move this code to `perf::version()`, then its failure might be interpreted in multiple ways: 1. `perf --version` failed => perf isn’t installed or coredumped 2. `parseVersion()` failed `PerfTest.Version` test checks whether we are able to parse perf version. If perf isn’t installed, the test shall pass. Quote from James: “This drops the test that verifies we can actually parse perf versions that are found in the wild. That test is necessary to catch when new systems play games with the perf version; otherwise we would just detect that the perf version isn’t supported and it would be hard to diagnose why not.” - Andrei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60397/#review179868 ----------------------------------------------------------- On July 3, 2017, 10:57 a.m., Andrei Budnik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60397/ > ----------------------------------------------------------- > > (Updated July 3, 2017, 10:57 a.m.) > > > Review request for mesos, Benjamin Bannier, James Peach, and Neil Conway. > > > Bugs: MESOS-7160 > https://issues.apache.org/jira/browse/MESOS-7160 > > > Repository: mesos > > > Description > ------- > > For autotools build, the docker-build script performs a 'distcheck' > build. This type of build warns if any unexpected files are left in > the build directory after an uninstall, mainly to detect broken > uninstall Makefile rules. The return status of the build container is > the result of the distcheck. > This fixes an issue where in some dockerized configurations > invocations of 'perf' segfaulted (producing a core file as a > side-effect), where the failure case was already anticipated and > handled by the caller. > > > Diffs > ----- > > src/linux/perf.cpp b301e25d5cac6488c57e4f983e4867c72368a040 > src/tests/containerizer/perf_tests.cpp > d8aab08eb131f974821fb85662cbc6cc685d2f3e > > > Diff: https://reviews.apache.org/r/60397/diff/3/ > > > Testing > ------- > > 1. make check (mac os x 10.12, fedora 25) > 2. internal CI > > Needs to be confirmed by Apache CI, e.g., reviewbot. > > > Thanks, > > Andrei Budnik > >
