Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12153 )
Change subject: build: Factor dependency extraction code from dist_test into a python library ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/12153/1/build-support/dep_extract.py File build-support/dep_extract.py: PS1: > Mind moving this new class into build-support/kudu_util.py? What's the reasoning behind that? This class is pretty specialized whereas most of the contents of that library are more general utilities such as for logging, Python compatibility shims, etc. http://gerrit.cloudera.org:8080/#/c/12153/1/build-support/dep_extract.py@47 PS1, Line 47: def set_library_filter(self, lib_allowed_filter): : """ : Specify a filter predicate that should return True iff the specified : library path should be included in the result from extract_deps(). : By default, all libraries are included in the result. : """ : self.lib_allowed_filter = lib_allowed_filter : : def set_expand_symlinks(self, expand): : """ : Specify whether symlinks should be expanded in the output from : extract_deps(). By default, symlinks are not expanded. See : expand_symlinks(). : """ : self.enable_expand_symlinks = expand > I assume these are configurable because the additional use case you have in Right, the behavior is different between them which is why it's set up like this. http://gerrit.cloudera.org:8080/#/c/12153/1/build-support/dep_extract.py@73 PS1, Line 73: path = os.path.join(os.path.dirname(path), os.readlink(path)) > os.readlink() may return an absolute path, so we don't necessarily want to Hrm. Maybe that was a mistake when this was written but it seems to work fine as-is so I'll just add the TODO. -- To view, visit http://gerrit.cloudera.org:8080/12153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b4cbfceb053c61dbb1f1d16716acc8926987af2 Gerrit-Change-Number: 12153 Gerrit-PatchSet: 1 Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Comment-Date: Fri, 04 Jan 2019 00:41:38 +0000 Gerrit-HasComments: Yes
