Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.23.0
    • Fix Version/s: 0.23.0
    • Component/s: fs
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      CommandFormat currently allows options in any location within the args. This is not the intended behavior for FsShell commands. Prior to the redesign, the commands used to expect option processing to stop at the first non-option.

      CommandFormat was an existing class prior the redesign, but it was only used by "count" to find the -q flag. All commands were converted to using this class, thus inherited the unintended behavior.

      1. HADOOP-7341-3.patch
        17 kB
        Daryn Sharp
      2. HADOOP-7341-2.patch
        17 kB
        Daryn Sharp
      3. HADOOP-7341.patch
        16 kB
        Daryn Sharp

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #711 (See https://builds.apache.org/hudson/job/Hadoop-Common-trunk/711/)
          HADOOP-7341. Fix options parsing in CommandFormat. Contributed by Daryn Sharp.

          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1132505
          Files :

          • /hadoop/common/trunk/CHANGES.txt
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CopyCommands.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Touchz.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/MoveCommands.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/FsUsage.java
          • /hadoop/common/trunk/src/test/core/org/apache/hadoop/fs/TestCommandFormat.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Stat.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Test.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Mkdir.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Count.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Tail.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Delete.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/SetReplication.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Ls.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Display.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CommandFormat.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/FsShellPermissions.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #711 (See https://builds.apache.org/hudson/job/Hadoop-Common-trunk/711/ ) HADOOP-7341 . Fix options parsing in CommandFormat. Contributed by Daryn Sharp. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1132505 Files : /hadoop/common/trunk/CHANGES.txt /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CopyCommands.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Touchz.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/MoveCommands.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/FsUsage.java /hadoop/common/trunk/src/test/core/org/apache/hadoop/fs/TestCommandFormat.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Stat.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Test.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Mkdir.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Count.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Tail.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Delete.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/SetReplication.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Ls.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Display.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CommandFormat.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/FsShellPermissions.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #641 (See https://builds.apache.org/hudson/job/Hadoop-Common-trunk-Commit/641/)
          HADOOP-7341. Fix options parsing in CommandFormat. Contributed by Daryn Sharp.

          todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1132505
          Files :

          • /hadoop/common/trunk/CHANGES.txt
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CopyCommands.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Touchz.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/MoveCommands.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/FsUsage.java
          • /hadoop/common/trunk/src/test/core/org/apache/hadoop/fs/TestCommandFormat.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Stat.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Test.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Mkdir.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Count.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Tail.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Delete.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/SetReplication.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Ls.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Display.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CommandFormat.java
          • /hadoop/common/trunk/src/java/org/apache/hadoop/fs/FsShellPermissions.java
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #641 (See https://builds.apache.org/hudson/job/Hadoop-Common-trunk-Commit/641/ ) HADOOP-7341 . Fix options parsing in CommandFormat. Contributed by Daryn Sharp. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1132505 Files : /hadoop/common/trunk/CHANGES.txt /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CopyCommands.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Touchz.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/MoveCommands.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/FsUsage.java /hadoop/common/trunk/src/test/core/org/apache/hadoop/fs/TestCommandFormat.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Stat.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Test.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Mkdir.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Count.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Tail.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Delete.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/SetReplication.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Ls.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Display.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CommandFormat.java /hadoop/common/trunk/src/java/org/apache/hadoop/fs/FsShellPermissions.java
          Hide
          Todd Lipcon added a comment -

          Committed based on atm's +1 (he's still waiting on his ASF account to get set up)

          Show
          Todd Lipcon added a comment - Committed based on atm's +1 (he's still waiting on his ASF account to get set up)
          Hide
          Aaron T. Myers added a comment -

          +1, looks good to me.

          Show
          Aaron T. Myers added a comment - +1, looks good to me.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12481523/HADOOP-7341-3.patch
          against trunk revision 1131330.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/578//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/578//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/578//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12481523/HADOOP-7341-3.patch against trunk revision 1131330. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/578//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/578//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/578//console This message is automatically generated.
          Hide
          Daryn Sharp added a comment -

          Per Aaron, alter text related to deprecation.

          Show
          Daryn Sharp added a comment - Per Aaron, alter text related to deprecation.
          Hide
          Aaron T. Myers added a comment -

          +1, looks good to me.

          One nit: I'd recommend either removing the "@deprecated name is never used" from the comment, or changing the name of the parameter from "n" to "name". Outside of the context of this patch, the comment doesn't make much sense.

          Show
          Aaron T. Myers added a comment - +1, looks good to me. One nit: I'd recommend either removing the "@deprecated name is never used" from the comment, or changing the name of the parameter from "n" to "name". Outside of the context of this patch, the comment doesn't make much sense.
          Hide
          Daryn Sharp added a comment -

          Are there any remaining issues I need to address?

          Show
          Daryn Sharp added a comment - Are there any remaining issues I need to address?
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12481006/HADOOP-7341-2.patch
          against trunk revision 1129905.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/548//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/548//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/548//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12481006/HADOOP-7341-2.patch against trunk revision 1129905. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/548//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/548//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/548//console This message is automatically generated.
          Hide
          Daryn Sharp added a comment -

          clean up a few warnings

          Show
          Daryn Sharp added a comment - clean up a few warnings
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12480991/HADOOP-7341.patch
          against trunk revision 1129905.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          -1 javac. The applied patch generated 1072 javac compiler warnings (more than the trunk's current 1068 warnings).

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/547//testReport/
          Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/547//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/547//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12480991/HADOOP-7341.patch against trunk revision 1129905. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. -1 javac. The applied patch generated 1072 javac compiler warnings (more than the trunk's current 1068 warnings). +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/547//testReport/ Findbugs warnings: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/547//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/hudson/job/PreCommit-HADOOP-Build/547//console This message is automatically generated.
          Hide
          Daryn Sharp added a comment -

          Stop at first non-option, like before.
          Also stop at opt/arg demarcation "--".
          Remove unnecessary & unused first param of null.

          Show
          Daryn Sharp added a comment - Stop at first non-option, like before. Also stop at opt/arg demarcation "--". Remove unnecessary & unused first param of null.
          Hide
          Todd Lipcon added a comment -

          ah, ok. I'm fine with just restoring the old behavior.

          Show
          Todd Lipcon added a comment - ah, ok. I'm fine with just restoring the old behavior.
          Hide
          Daryn Sharp added a comment -

          Aaron:
          I completely agree the parser needs to be changed, but I don't currently have the time available. My goal here is a very small/quick change to revert how every FsShell command (except count) to how it used to work. If you're ok with that, I think another jira should switch the parser.

          Todd:
          I suppose it comes down to whether hadoop should favor posix or gnu extensions?

          $ mkdir a b
          
          $ ls a -l b
          a:
          total 0
          b:
          total 0
          
          $ POSIXLY_CORRECT=1 ls a -l b
          ls: -l: No such file or directory
          a:
          b:
          

          I was just planning to restore the prior behavior.

          Show
          Daryn Sharp added a comment - Aaron: I completely agree the parser needs to be changed, but I don't currently have the time available. My goal here is a very small/quick change to revert how every FsShell command (except count) to how it used to work. If you're ok with that, I think another jira should switch the parser. Todd: I suppose it comes down to whether hadoop should favor posix or gnu extensions? $ mkdir a b $ ls a -l b a: total 0 b: total 0 $ POSIXLY_CORRECT=1 ls a -l b ls: -l: No such file or directory a: b: I was just planning to restore the prior behavior.
          Hide
          Todd Lipcon added a comment -

          hmm – do you think it's correct behavior that we only support flags at the front of the args? Linux "ls" for example is fine with "ls / -l"

          Show
          Todd Lipcon added a comment - hmm – do you think it's correct behavior that we only support flags at the front of the args? Linux "ls" for example is fine with "ls / -l"
          Hide
          Aaron T. Myers added a comment -

          Now seems like a great time to scrap CommandFormat altogether, in favor of the capabilities provided by Commons CLI, a dependency which Hadoop already includes for use in the GenericOptionsParser.

          Thoughts?

          Show
          Aaron T. Myers added a comment - Now seems like a great time to scrap CommandFormat altogether, in favor of the capabilities provided by Commons CLI, a dependency which Hadoop already includes for use in the GenericOptionsParser . Thoughts?

            People

            • Assignee:
              Daryn Sharp
              Reporter:
              Daryn Sharp
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development