Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12350 )

Change subject: KUDU-2411: linux: Ship the sasl2 modules with the binary 
artifact
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/12350/1/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py
File build-support/mini-cluster/relocate_binaries_for_mini_cluster.py:

http://gerrit.cloudera.org:8080/#/c/12350/1/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py@385
PS1, Line 385:   for path in SASL_MODULE_PATHS:
             :     if os.path.exists(path):
             :       children = os.listdir(path)
             :       for child in children:
             :         if PAT_SASL_LIBPLAIN.search(child):
             :           sasl_path = path
             :           break
FWIW, I think this would be more robust as something like "$(dirname $(ldd kudu 
| grep sasl | cut -d '>' -f 2 | cut -d '(' -f 1 | xargs))/sasl2" (adapted into 
Python of course).

On el6:
/usr/lib64/sasl2

On Ubuntu 18:
/usr/lib/x86_64-linux-gnu/sasl2

But such a change would require more changes to the flow of the script.


http://gerrit.cloudera.org:8080/#/c/12350/1/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py@393
PS1, Line 393:   if sasl_path is None:
Nit: "if not sasl_path" is more Pythonic.


http://gerrit.cloudera.org:8080/#/c/12350/1/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py@397
PS1, Line 397:   dest_dir = os.path.join(config[ARTIFACT_LIB_DIR], 'sasl2')
             :   os.makedirs(dest_dir)
Should we have called prep_artifact_dirs prior to this?



--
To view, visit http://gerrit.cloudera.org:8080/12350
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I44afa05bcfa1a29a90f58c72ed6b0752014a16eb
Gerrit-Change-Number: 12350
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Brian McDevitt <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Mon, 04 Feb 2019 23:58:17 +0000
Gerrit-HasComments: Yes

Reply via email to