Skip to content

Commit

Permalink
Report failures to add permissions with JavaIoFileSystem
Browse files Browse the repository at this point in the history
Previously the return value of the `File#setX` functions was not
checked, thus masking potential errors. Since these functions are
expected to fail on Windows if asked to remove a permission, errors are
only reported when attempting to add one.

Also removes misleading `throws` declarations that may mask similar
issues of this type.

Closes bazelbuild#18004.

PiperOrigin-RevId: 523454138
Change-Id: Ia85db5e912d07af1ca6f7cd4b8e2d784395e7811
  • Loading branch information
fmeum authored and copybara-github committed Apr 11, 2023
1 parent bfdff54 commit 89a3d5e
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,9 @@ protected void setReadable(PathFragment path, boolean readable) throws IOExcepti
if (!file.exists()) {
throw new FileNotFoundException(path + ERR_NO_SUCH_FILE_OR_DIR);
}
file.setReadable(readable);
if (!file.setReadable(readable) && readable) {
throw new IOException(String.format("Failed to make %s readable", path));
}
}

@Override
Expand All @@ -185,7 +187,9 @@ public void setWritable(PathFragment path, boolean writable) throws IOException
if (!file.exists()) {
throw new FileNotFoundException(path + ERR_NO_SUCH_FILE_OR_DIR);
}
file.setWritable(writable);
if (!file.setWritable(writable) && writable) {
throw new IOException(String.format("Failed to make %s writable", path));
}
}

@Override
Expand All @@ -194,7 +198,9 @@ protected void setExecutable(PathFragment path, boolean executable) throws IOExc
if (!file.exists()) {
throw new FileNotFoundException(path + ERR_NO_SUCH_FILE_OR_DIR);
}
file.setExecutable(executable);
if (!file.setExecutable(executable) && executable) {
throw new IOException(String.format("Failed to make %s executable", path));
}
}

@Override
Expand Down Expand Up @@ -442,49 +448,50 @@ protected FileStatus stat(PathFragment path, boolean followSymlinks) throws IOEx
} catch (java.nio.file.FileSystemException e) {
throw new FileNotFoundException(path + ERR_NO_SUCH_FILE_OR_DIR);
}
FileStatus status = new FileStatus() {
@Override
public boolean isFile() {
return attributes.isRegularFile() || isSpecialFile();
}

@Override
public boolean isSpecialFile() {
return attributes.isOther();
}

@Override
public boolean isDirectory() {
return attributes.isDirectory();
}

@Override
public boolean isSymbolicLink() {
return attributes.isSymbolicLink();
}

@Override
public long getSize() throws IOException {
return attributes.size();
}

@Override
public long getLastModifiedTime() throws IOException {
return attributes.lastModifiedTime().toMillis();
}

@Override
public long getLastChangeTime() {
// This is the best we can do with Java NIO...
return attributes.lastModifiedTime().toMillis();
}

@Override
public long getNodeId() {
// TODO(bazel-team): Consider making use of attributes.fileKey().
return -1;
}
};
FileStatus status =
new FileStatus() {
@Override
public boolean isFile() {
return attributes.isRegularFile() || isSpecialFile();
}

@Override
public boolean isSpecialFile() {
return attributes.isOther();
}

@Override
public boolean isDirectory() {
return attributes.isDirectory();
}

@Override
public boolean isSymbolicLink() {
return attributes.isSymbolicLink();
}

@Override
public long getSize() {
return attributes.size();
}

@Override
public long getLastModifiedTime() {
return attributes.lastModifiedTime().toMillis();
}

@Override
public long getLastChangeTime() {
// This is the best we can do with Java NIO...
return attributes.lastModifiedTime().toMillis();
}

@Override
public long getNodeId() {
// TODO(bazel-team): Consider making use of attributes.fileKey().
return -1;
}
};

return status;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,12 @@ public boolean isSymbolicLink() {
}

@Override
public long getSize() throws IOException {
public long getSize() {
return attributes.size();
}

@Override
public long getLastModifiedTime() throws IOException {
public long getLastModifiedTime() {
return attributes.lastModifiedTime().toMillis();
}

Expand Down

0 comments on commit 89a3d5e

Please sign in to comment.