511337 - checksetup.pl not properly throwing errors when it can't chmod a file

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, defect)

Description

[Max Kanat-Alexander]

There's a bug in Bugzilla::Install::Filesystem--it's supposed to throw a warning when it can't change the permissions of a file, but it should be using "or" instead of "||". (Right now, the way it's written, it will pretty much never throw the warning.)

Comment 1

[Max Kanat-Alexander]

Attachment: v1

Granting myself review as module owner, for tip only.

This exposes a few interesting bugs--namely, that we can't chown/chmod files that the webserver creates. We should probably do something about that, if we can (though what, I don't know).

Comment 2

[Max Kanat-Alexander]

Checking in Bugzilla/Install/Filesystem.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/Filesystem.pm,v  <--  Filesystem.pm
new revision: 1.36; previous revision: 1.35
done
Checking in template/en/default/setup/strings.txt.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/setup/strings.txt.pl,v  <--  strings.txt.pl
new revision: 1.15; previous revision: 1.14
done

Comment 3

[Frédéric Buclin]

Comment on attachment 395267 [details] [diff] [review]
v1

>Index: Bugzilla/Install/Filesystem.pm

>+        or warn install_string('chown_failed', { path => $name, 
>+                                                 error => $! }) . "\n";

\n is not very useful here as you are writing to STDERR

Comment 4

[Max Kanat-Alexander]

(In reply to comment #3)
> \n is not very useful here as you are writing to STDERR

  It's incredibly useful. It stops the "at blah blah blah line 72" part of "warn".

Comment 5

[Frédéric Buclin]

(In reply to comment #4)
>   It's incredibly useful. It stops the "at blah blah blah line 72" part of
> "warn".

I like to know which line of which script thrown the error.