iTop
iTop copied to clipboard
Combodo
N°5235 - Add tmp dir is writable check in setup
If not present, will crash curing compilation (in \MFCompiler::Compile, when doing unlink($sTempTargetDir);)
Accepted in technical review. @piRGoif will test on Windows / Linux env to ensure it does not bring any regression before reviewing it in functional review.
Discussed during technical review: Why not include the directory test in the already existing self::CheckWritableDirs() method? Is it just for the specific error message?
Discussed during technical review: Why not include the directory test in the already existing
self::CheckWritableDirs()method? Is it just for the specific error message?
That was what I wrote in my message dated of 5th july : this method is meant only to check dirs under approot for now... I tried first to refactor it, but it made the code complicated so I added the tmp dir check as a standalone test : simplier, much easier to read and understand.
Ok thanks for the clarification, maybe we should add a comment above it to avoid someone to refactor this and put the tmp dir in the above array. What do you think?
Yep, adding a phpdoc block to \SetupUtils::CheckWritableDirs would be a good idea ! I'll do it in the upcoming days
Processed today in technical review : good to go for functional review next week !
Functional review: See with @Rorcha07 for possible complications on exotic setups (eg. lodabalancers, netwrk drives, ...) If any concern raised will be part of 3.1.0, if green light from Stephane then 2.7.8
Discussed with Stephane : we will switch the check to warning so it will become non blocking. The information will still be there, and we won't block setup wrongly !
New message when dir isn't writable moved to warning :
And after opening the details :
