Henry Robinson has posted comments on this change. Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil ......................................................................
Patch Set 9: (2 comments) Thanks for the quick reviews! http://gerrit.cloudera.org:8080/#/c/5688/9/be/src/util/time.h File be/src/util/time.h: Line 36: return ts.tv_sec * NANOS_PER_SEC + ts.tv_nsec; > are you sure impala doesn't have a commit that introduced GetMonoTimeNanos( I think we did add it for MacOS (see https://github.com/apache/incubator-impala/commit/aeadd326a982af0f2ac8d55e425248a8bc7babfe), and that added GetMonoTimeNanos() for both platforms. I took the view that it's easier to maintain Impala-side dependencies for now, and that the MacOS port is effectively dead at this point. http://gerrit.cloudera.org:8080/#/c/5688/9/cmake_modules/kudu_cmake_fns.txt File cmake_modules/kudu_cmake_fns.txt: Line 64: endif() > what do these two changes do and why are they needed now? The first change adds custom compile flags to this target. This is needed to suppress some spurious warnings. The second change isn't strictly needed here, but since gutil is the first user of ADD_EXPORTABLE_LIBRARY I figured I'd include the fix at the earliest point. This allows CMake to properly handle transitive dependencies, so we don't need to 'flatten' all the deps of, e.g., kudu_util when linking against it. -- To view, visit http://gerrit.cloudera.org:8080/5688 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80 Gerrit-PatchSet: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
