Conversation
isc-dchui
requested review from
isc-cborbonm,
isc-egabhart,
isc-eneil,
isc-jili,
isc-jlechtne,
isc-kiyer,
isc-pbarton and
isc-tleavitt
as code owners
isc-kiyer
left a comment
isc-kiyer
left a comment
There was a problem hiding this comment.
@isc-dchui some small questions
| set tVerbose = $get(pParams("Verbose")) | ||
|
|
||
| // Check base IPM install location | ||
| set ipmBaseDir = ##class(%Library.File).NormalizeDirectory(##class(%SYSTEM.Util).DataDirectory() _ "ipm") |
There was a problem hiding this comment.
Don't we have a macro/a setting to be configured for the ipmDir? we should use that right?
There was a problem hiding this comment.
Not yet--it's in #1031 but I can drive that forward
| set extension = $$$lcase($piece(tDirectoryName,".",*)) | ||
| if ##class(%File).Exists(tDirectoryName) && ((extension="tgz") || (extension="tar.gz")) { | ||
| set tTargetDirectory = $$$FileTempDirSys | ||
| if tTargetDirectory = ("-" _ -tTargetDirectory) { |
There was a problem hiding this comment.
Can you explain this if condition? Its a little odd to see negation of what seems like it should be a string? Is it an integer when there is no permission?
There was a problem hiding this comment.
Right, it's a negative integer code when there's an error, so it's checking for that. It's a bit ugly/confusing but is used in %IPM.Repo.Oras.PublishService:PublishModule:
if tDir = ("-" _ -tDir) { // $zu(140,17) can return a negative number if it fails
Breaking it down a bit:
tTargetDirectory can either be an actual path like \foo\bar\ or a negative integer code like -13. The coercion, negation, and comparison then check if it's a negative integer or a string.
isc-tleavitt
left a comment
isc-tleavitt
left a comment
There was a problem hiding this comment.
Agreed with Keshav on comment needed re: weird negation, otherwise looks good.
3 participants
Description
installandloadcommand.Testing
Manually tested by changing the permissions of the
/usr/irissys/ipmdirectory and verifying the error appears when trying to install.Checklist
mainbranch rebased or merged.zpm test -only) and integration tests (zpm verify -only) pass.