Jimmy said a review.log file was supposed to be created for each test on the client, but it doesn't appear to be written anywhere in the web tree nor is it generating an error about it not writing that I can find. When I hard coded a path, it wrote where I told it to.

CommentFileSizeAuthor
#3 622300-realpath.patch800 bytesboombatower

Comments

boombatower’s picture

boombatower commented
Assigned: Unassigned » boombatower
Status: Active » Postponed (maintainer needs more info)

Can you print out file_directory_path() to hard coded log file and see if it is right?

That and print the whole generated url.

deekayen’s picture

deekayen commented
Status: Postponed (maintainer needs more info) » Active 
file_directory_path == sites/default/files
$this->log_file == ''

/me aims blame at realpath()

boombatower’s picture

boombatower commented
Status: Active » Needs review
StatusFileSize
new 622300-realpath.patch800 bytes

Perhaps it has an issue if the file does not exist first, although if you hard coded...not really sure.

If this doesn't work perhaps, try adding './' in front.

deekayen’s picture

deekayen commented
Status: Needs review » Needs work

I think that patch is going to do exactly the same thing - all you did was break it up over two lines.

deekayen’s picture

deekayen commented

I tried $this->log_file = realpath('./'. file_directory_path() . '/review.log'); and still no log_file value.

I'm interested to see how you'll derive a full path - I have a custom module at Classic Graphics where I ended up coding a configuration variable for it cause I didn't get it figured out quick enough.

boombatower’s picture

boombatower commented
Status: Needs work » Postponed (maintainer needs more info)

I would really like to know why this doesn't work. Makes absolutely no sense.

http://us3.php.net/manual/en/function.realpath.php

realpath() expands all symbolic links and resolves references to '/./', '/../' and extra '/' characters in the input path . and return the canonicalized absolute pathname.

If it can't handle the fact the file is not there then #3 should do fine, but otherwise I am lost. Works like a charm locally.

Felicitus’s picture

Felicitus commented

I stumbled over exactly this issue, and have an explanation:

realpath() returns an empty string if the file doesn't exist. In my case, a realpath("sites/default/files") did work, but a realpath("sites/default/files/review.log") did not unless the file was manually created. This is either a bug or a documentation issue; I believe it is a little bit of both. As a temporary fix, it is possible to simply create the file.

deekayen’s picture

deekayen commented

I did this on #34 - let's see how it goes after a couple tests:

cd /var/www/sites/default/files
touch review.log
chmod 777 review.log
chown www-data:www-data review.log
deekayen’s picture

deekayen commented

The log contents are reported at http://qa.drupal.org/pifr/test/16620 as they should be (I think). Now I guess the question is whether or not the creation of review.log should be done through PIFR's installer or as part of the client configuration and setup. Personally, I think touching the file is very workaround-ish, so I'm leaving this open rather than changing my PIFR setup instructions.

deekayen’s picture

deekayen commented

I'm trying this patch on #34:

    // Initialize the log file.
+   touch(file_directory_path() . '/review.log');
    $this->log_file = realpath(file_directory_path() . '/review.log');
    $this->log('Log initialzed.');
deekayen’s picture

deekayen commented
Status: Postponed (maintainer needs more info) » Needs review
deekayen’s picture

deekayen commented
Status: Needs review » Reviewed & tested by the community

Seems to work. http://qa.drupal.org/pifr/test/20292

Re: my email, after this is committed, please comment on which branch or tag to update our clients from CVS.

boombatower’s picture

boombatower commented

That is exactly what I did in #3...?

deekayen’s picture

deekayen commented

mm... i guess.
I like mine better :) I already added review.log to all three live testing clients and added it to the install instructions anyway.

Perhaps adding a comment to the usage of realpath there would be helpful about needing the file to exist first since the only docs on that fact are a comment on the doc page.

There's a resolution somewhere in this thread, whichever one you pick. I'll be ignoring it now.

boombatower’s picture

boombatower commented
Status: Reviewed & tested by the community » Fixed

Committed your solution.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.