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

RESOLVED FIXED in Bugzilla 3.6

Status

()

Bugzilla
Installation & Upgrading
--
minor
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Max Kanat-Alexander)

Tracking

Bugzilla 3.6

Details

Attachments

(1 attachment)

v1
2.06 KB, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Assignee)

Description

9 years ago
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.)
(Assignee)

Comment 1

9 years ago
Created attachment 395267 [details] [diff] [review]
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).
Assignee: installation → mkanat
Status: NEW → ASSIGNED
Attachment #395267 - Flags: review+
(Assignee)

Comment 2

9 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 3

9 years ago
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
(Assignee)

Comment 4

9 years ago
(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

9 years ago
(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.
You need to log in before you can comment on or make changes to this bug.