brbill wrote on Thursday, November 12, 2009:
Brady wrote:
*Got white screen death since mandriva(developer appliance) doesn’t have Archive/Tar.php .
…
The biggest issue seems to be 1; would be a bummer to actually break some of the linux boxes whom don’t have Archine/Tar.php. If this package doesn’t exist I wonder if there is a way top bypass with several ‘if’ commands in linux boxes to use command line instead of Archive/Tar.php.*
Yes, if you don’t have Archive/Tar.php, this won’t work. When I was doing some of the testing of this module prior to using it, I noticed that it was not installed with the older version of PHP that came with OSX 10.4 (3 years old, I have since upgraded to 0SX 10.6). But PEAR is now standard with modern PHP and should be available for usage on any system at this time.
Obviously, it is possible for ANY older PHP to be missing this module. But at some point we have to say, get modern. It’s six of one, half dozen of another. One system doesn’t have tar binary, the other doesn’t have Archive/Tar.php.
Your suggestion for supporting this with different code for Windows and *nix is possible (going back to old tar command for boxen), but doing this will be a much uglier kludge than I can ever be satisfied with.
Rod’s recent updates to backup.php made it quite a bit more complicated. The backup program definitely does not need to be made any more complicated by adding another state model. I urge you to reconsider this objection based on the situation, and possibly peruse how the program executes system commands vs. how it is calling Tar.php.
*1) This is really for Rod. Why are we using the sed stuff to replace things in the dump file. Is this really needed? Because if not, then the Windows users won’t need perl set up.*
I will leave the reason for having this to Rod, because I did not change this functionality. I just needed to make it work in Windows. Since perl comes with the XAMPP stack, I just don’t find this to be a worry point. Windows installations are going to have perl if they have XAMPP, and they will. Won’t they?
*2) To simplify code and temp directories, why not just have it hard coded always and not check for the php tmp directory? I say this because losing track of these files and placing them in an unsecure area would be bad. so seems more secure and would simplify code to just always direct where it goes from globals.php.*
Earlier in this thread, I asked if this was important, so I guess the answer is yes. It increases the amount of customization that has to be done for each installation.
And I have to ask what is more trouble: reading these conditional lines, whether they be ternary operator or if blocks, or having to decide on their own where they want their temp directories to be?
*3) The globals.php stuff will throw off end users especially the () ? : one liners. Seems like could just have a ‘windows only’ section for mysql/perl pathnames (then backup.php can then decide to use the windows path if windows setting is on and use no path if it’s linux). Then you’ll just be left with one () ? : statement for the tmp directory would could be described with several lines of comments.*
We could have separate windows and *nix sections, as you say, but then we have to decide whether these conditionals, which may be sprinkled throughout globals.php to be associated with other related settings, would seem out of context in such a grouping.
Although I understand that some folks won’t understand the ternary operator, it is supported in C, ruby, perl, PHP, and many other languages, and I found it all over the OEMR source, so it is in common usage. We have users directly modifying actual PHP code in globals.php as part of the installation process, so they have to have some idea of what they are doing.
*4) A minor issue is use of some of your xl() calls in backup.php. Don’t place leading or trailing spaces within the actual string. This gets lost in the translation process and has potential to create redundant english constants for the translators (ie. creating the same string without the trailing space).*
Noted. Will fix.
Bill Cernansky
