ayushtkn commented on a change in pull request #2061:
URL: https://github.com/apache/hadoop/pull/2061#discussion_r436536429
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1202,19 +1202,18 @@ public FileStatus getFileStatus(Path f) throws
IOException {
INodeLink<FileSystem> link = (INodeLink<FileSystem>) inode;
try {
String linkedPath = link.getTargetFileSystem().getUri().getPath();
- if("".equals(linkedPath)) {
+ if ("".equals(linkedPath)) {
linkedPath = "/";
}
FileStatus status =
- ((ChRootedFileSystem)link.getTargetFileSystem())
- .getMyFs().getFileStatus(new Path(linkedPath));
- result[i++] = new FileStatus(status.getLen(), false,
- status.getReplication(), status.getBlockSize(),
- status.getModificationTime(), status.getAccessTime(),
- status.getPermission(), status.getOwner(), status.getGroup(),
- link.getTargetLink(),
- new Path(inode.fullPath).makeQualified(
- myUri, null));
+ ((ChRootedFileSystem) link.getTargetFileSystem()).getMyFs()
+ .getFileStatus(new Path(linkedPath));
+ result[i++] = new FileStatus(status.getLen(), status.isDirectory(),
+ status.getReplication(), status.getBlockSize(),
+ status.getModificationTime(), status.getAccessTime(),
+ status.getPermission(), status.getOwner(), status.getGroup(),
+ link.getTargetLink(),
+ new Path(inode.fullPath).makeQualified(myUri, null));
Review comment:
I think the intent behind that assertion is little different.
An entity can be either a directory, file or a symlink. In linux while
listing, directory is denoted as d, link by l and file by -, So there are three
categories (Directory/File/Link) not just two (Directory/File)
```
drwxrwxr-x 2 ayush ayush 4096 Jun 8 04:25 dir/
lrwxrwxrwx 1 ayush ayush 4 Jun 8 04:25 sym_dir -> dir//
-rw-rw-r-- 1 ayush ayush 0 Jun 8 11:05 txt
```
A symlink can point to both directory and file, but isn't itself a directory
or file. It is link, and has its own identity.
In Hadoop, to distinguish between the three categories, the logic seems like
:
isDir==true --> It is a Directory
isDir==false --> Can be a file or Symlink. So to conclude further whether a
file or link
isDir==false and link==null --> it is file
isDir==false and link!=null --> it is a symlink
The assertion message "A directory can not be a symlink" also tries to
depict this only. Since if isDir is true that means it is a directory so it
can't be a symlink, it can be a symlink when it isn't neither directory nor
file.
Now regarding changing it here in ViewFS, allowing isDir to be true and as
well as having a link.
If someone is having an application code with
```
if(sDir==false and link!=null) --> it is a symlink
do work considering symlink.
```
His code will break, Since this check won't return true post this change.
Would this change incompatible?
Now regarding inconsistency between getFileStatus and listStatus, if we are
considering a mount entry as link, so getFileStatus should follow the similar
semantics as listStatus, A same entity can not be treated differently. If we
want to treat it as link, we can think of makin getFileStatus adapt..
Just my thoughts, not much idea about the actual discussions what was the
intent then..
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
