Re: new IM driver for Image_Transform

At 09:10 AM 5/24/02 -0700, Dan Allen wrote:

Sorry this looks broken up, but I figured the discussion should be made public...

I wondered why you took it off-list in the first place :-)

What is the advantage of this rather than using a defined constant? AFAIK which() won't work on Windows? (using CMD::which() for path to identify and convert) Well, it should...the advantage is for autodetecting (good) instead of using a constant (bad). It is common in PEAR to try to autodetect the environment...and frees the user from having to know that imagemagick is being used from the commandline...

Well, in its current state the function *will not* work on windows: a) It expects $HTTP_ENV_VARS['PATH'] to have ":" between the paths, and Windows uses ";" b) $location = $path . "/" . $cmd; has the / when all the other slashes in the Window's path will be \. c) On Windows it isn't customary to always put files in the environment path, unlike Unix. Maybe a compromise is to let people define a path, and then if they haven't defined one we can autodetect it: if (!defined('IMAGE_TRANSFORM_LIB_PATH')) {

        include_once "CMD.php";
        // Do the rest

}

Why not use passthru over exec? I used passthru so that any error message printed to sdout would be echoed to screen for the user. This is definitely not the PEAR standard...when running a class, errors should always be passed to PEAR's error handler and never echoed directly to screen.

I couldn't find a way of capturing the error message... is this what exec will do? I thought it was just a status value, not an error message that was returned.

Imagine a production environment where the end user is seeing something like 'convert' [Unknown arguments]

Imagine anyone putting some code into production without testing it.

Very bad...we should try to capture every last bit of error output and handle it appropriately...Using the commandline is very fragile unless this is done, since it is a child process and it is easily to write a program that can silently fail.

That was the problem I had as I couldn't capture or display any error message without using passthru()

it so that we can support cURL etc for fetching the images - I have remote fopen calls disabled and use cURL instead on my server. Fine, either way...autodetect of course the best method...point is it should be able to do this since the GD extension can.

Q: Can we autodetect if the curl module is loaded into PHP? I'm sure there's a simple way but I've never looked...

I don't believe that this is necessary (error checking). If there's an error in the usage of the class then the person will find out when: a) they view the image b) when a PHP error message is printed to the screen This is very non-PEAR standard...PEAR has always held validation sacred and has done everything it can to check even programming errors...

But given that these errors should never occur in production isn't this overkill? Error checking for the sake of it?

If you believe it slows down the class, then talk to the PEAR designers because regardless of speed, they believe importance outweighs this...

I have heard some saying that the error handling is overkill.

No need IMO to slow down the classes more than we have to :-) It really doesn't slow it down at all, since php is so fast at these trivial checks.

OK, maybe I just expect any additional code to slow down PHP a lot, which isn't what happens...

I can't find CMD.php anywhere in PEAR :-( It is in the original cvs tree in /php4/pear/CMD.php...it will be moved soon, but until it is...just require it there.

OK. This is the one part of PEAR that isn't installed on my server :-)

Yes, since an object is created internally and then returned...you pass by reference so that all calls on it will work with the original object and not a copy....check out the php manual, they refer to this exact thing...I got the idea for this function from Horde (great examples of php PEAR classes there) and they pass it by reference.

Time for me to do a little reading :-)

PEAR standard as it is used elsewhere, such as in Auth...I don't really care what the name is, as long as it contains the code I had suggested...namely including the driver file and instantiating the class.

Talking of driver files, what is the preferred way to make these changes in CVS? Delete everything and start again? I've started reorganizing the files locally but am unsure how to make the changes in the Repository.

I found out why ImageMagick gives me out of memory errors: every process on my server is limited in size to 14Mb. Thank you very &%#*@ much Verio! I, hence the call to ulimit (which you had mistakenly put as unlimit, at least there is no such Unix shell function unlimit)...

No wonder it had absolutely no effect :-D

I am not sure this call is necessary...plus ulimit seems to print to stdout...anyway, I am not 100% on that last paragraph, no real opinions.

ulimit shouldn't need to be in the final release as it was only there to let me try and get it working on my server. Best wishes Peter