Details

Description

Dev had seen the attempt directory permission getting set to 000 or 111 in the CI builds and tests run on dev desktops with 0.20-security.MAPREDUCE-2238 reported and fixed the issue for 0.22.0, back-port to 0.20-security is needed.

-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.

Hadoop QA
added a comment - 06/Jul/11 23:05 -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12485489/MAPREDUCE-2650.patch
against trunk revision 1143252.
+1 @author. The patch does not contain any @author tags.
-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no new tests are needed for this patch.
Also please list what manual steps were performed to verify this patch.
-1 patch. The patch command could not apply the patch.
Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/438//console
This message is automatically generated.

Sherry Chen
added a comment - 18/Jul/11 23:52 Todd,
I did not make it clear in previous comment.
Throws an exception (when makedirs failed) semantics are used in trunk and CDH3.
It's good to put it in 0.20-security.

Sherry,
I think Todd's comment was on localFS.setPermission() calls. Those calls would throw an exception in this patch and would just return false with a log warning in the original code. Is that correct?

Infact all the calls to setpermission have to been changed.

The mkdirs change is ok, but the other changes am not sure should be part of this patch?

Mahadev konar
added a comment - 25/Jul/11 03:59 Sherry,
I think Todd's comment was on localFS.setPermission() calls. Those calls would throw an exception in this patch and would just return false with a log warning in the original code. Is that correct?
Infact all the calls to setpermission have to been changed.
The mkdirs change is ok, but the other changes am not sure should be part of this patch?