Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12310 )

Change subject: [build] Fix mini cluster binary relocation on Mac OS
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/12310/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/12310/1//COMMIT_MSG@9
PS1, Line 9: On Mac OS the ssl libs like `libssl.1.0.0.dylib` are read only.
           : When relocate_binaries_for_mini_cluster.py tries to copy
           : these read only libs, the build fails because the file
           : already exists and cannot be replaced.
> This explanation doesn't follow from the UNIX permission model. I presume s
My lib directory is `drwxr-xr-x` and the libssl file with the issue is 
`-r--r--r--`. That is the permission in `/usr/local/opt/openssl/lib/` where it 
comes from.

This is the error I get:
```Traceback (most recent call last):
  File 
"/Volumes/Workspace/Source/Work/kudu/build-support/relocate_binaries_for_mini_cluster.py",
 line 361, in <module>
    main()
  File 
"/Volumes/Workspace/Source/Work/kudu/build-support/relocate_binaries_for_mini_cluster.py",
 line 358, in main
    relocate_target(target, config)
  File 
"/Volumes/Workspace/Source/Work/kudu/build-support/relocate_binaries_for_mini_cluster.py",
 line 335, in relocate_target
    return relocate_deps(target_src, target_dst, config)
  File 
"/Volumes/Workspace/Source/Work/kudu/build-support/relocate_binaries_for_mini_cluster.py",
 line 314, in relocate_deps
    return relocate_deps_macos(target_src, target_dst, config)
  File 
"/Volumes/Workspace/Source/Work/kudu/build-support/relocate_binaries_for_mini_cluster.py",
 line 293, in relocate_deps_macos
    copy_file(resolved_path, lib_dst)
  File 
"/Volumes/Workspace/Source/Work/kudu/build-support/relocate_binaries_for_mini_cluster.py",
 line 231, in copy_file
    shutil.copyfile(src, dest)
  File 
"/usr/local/Cellar/python@2/2.7.15_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/shutil.py",
 line 97, in copyfile
    with open(dst, 'wb') as fdst:
IOError: [Errno 13] Permission denied: 
u'/Volumes/Workspace/Source/Work/kudu/build/minicluster/kudu-binary-1.9.0-SNAPSHOT-osx-x86_64/lib/libssl.1.0.0.dylib'```


http://gerrit.cloudera.org:8080/#/c/12310/1//COMMIT_MSG@14
PS1, Line 14: This patch changes the copy_file method to ignore files
            : that already exist.
> I don't think this is a good idea though; what if one of my dependencies (s
Since the lib relocation work happens every time this is run, I would expect 
this directory to be blown away each time. That would be required to be sure we 
don't leave behind old files/libs that are no longer needed.

Perhaps that isn't happening. I will check.


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

http://gerrit.cloudera.org:8080/#/c/12310/1/build-support/relocate_binaries_for_mini_cluster.py@225
PS1, Line 225:     shutil.copy(src, dest)
> I actually think copyfile+copymode (or maybe copy2) make more sense:
I don't think we need metadata like mtime. I will revert to using copyfile.

Instead of copymode, I will use os.chmod to make sure the files have consistent 
permissions.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib653b175ba2d0e5563509aecac372c9829eca2f5
Gerrit-Change-Number: 12310
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <[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-Comment-Date: Wed, 30 Jan 2019 18:50:31 +0000
Gerrit-HasComments: Yes

Reply via email to