[ 
https://issues.apache.org/jira/browse/HADOOP-9483?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13647177#comment-13647177
 ] 

Ivan Mitic commented on HADOOP-9483:
------------------------------------

Thanks Arpit for addressing the comments!

bq. 2. I am not sure it is possible to get a path longer than MAX_PATH as input 
to Readlink since winutils is invoked via the shell. The situation you have 
handled elsewhere is when the path is passed in via JNI. Please let me know if 
I am wrong.
Yes, we are doing this for both JNI and command line calls, please fix this.

bq. 5. Deliberate for consistency with POSIX readlink. All extra arguments are 
ignored.
We do a strong param checks today with all other winutils cmd line options. 
Let's do the same here, otherwise, people might think that the parameter is 
actually doing something for them.

bq. 8. It seems to complicates the state, especially since we don't really care 
about the exact error code for most Win32 calls here. However I don't feel too 
strongly about it so let me know what you think.
Fine then. I would have done it the other way around (it can help when you're 
stepping thru with a debugger, if you don't know about !gle :)), but it's not a 
big deal.

bq. 9. That is intended to be fixed in HADOOP-9527.
Sounds good.

Two additional comments:
10. I see what you're trying to do with the {{for}} loop. However, this won't 
work when you get ERROR_MORE_DATA back, right? I say, let's just allocate a 
1024 size array on stack, and if the this is not enough, have the function 
fail. If in the future this turns out to be a problem, we can fix it (what is 
very unlikely to happen). Make sense?

11. Can you add a comment in code on CreateFile flags you are passing (or a 
reference)


                
> winutils support for readlink command
> -------------------------------------
>
>                 Key: HADOOP-9483
>                 URL: https://issues.apache.org/jira/browse/HADOOP-9483
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: util
>    Affects Versions: 3.0.0
>            Reporter: Chris Nauroth
>            Assignee: Arpit Agarwal
>         Attachments: HADOOP-9483.003.patch, HADOOP-9483.patch, 
> HADOOP-9483.patch
>
>
> The current codebase relies on the Unix readlink command to determine the 
> target of a symlink on the local file system.  winutils currently does not 
> support this functionality on Windows.  Adding the command to winutils will 
> prevent the need to use GnuWin32 or Cygwin for readlink support.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to