You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by sz...@apache.org on 2009/10/30 19:22:31 UTC
svn commit: r831413 - in /hadoop/common/branches/branch-0.21: ./
src/java/org/apache/hadoop/fs/ src/java/org/apache/hadoop/util/
src/test/core/org/apache/hadoop/fs/ src/test/core/org/apache/hadoop/util/
Author: szetszwo
Date: Fri Oct 30 18:22:31 2009
New Revision: 831413
URL: http://svn.apache.org/viewvc?rev=831413&view=rev
Log:
HADOOP-6334. Fix GenericOptionsParser and Path to have a better URI support. Contributed by Amareshwari Sriramadasu
Added:
hadoop/common/branches/branch-0.21/src/test/core/org/apache/hadoop/util/TestGenericOptionsParser.java
Modified:
hadoop/common/branches/branch-0.21/CHANGES.txt
hadoop/common/branches/branch-0.21/src/java/org/apache/hadoop/fs/Path.java
hadoop/common/branches/branch-0.21/src/java/org/apache/hadoop/util/GenericOptionsParser.java
hadoop/common/branches/branch-0.21/src/test/core/org/apache/hadoop/fs/TestPath.java
Modified: hadoop/common/branches/branch-0.21/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.21/CHANGES.txt?rev=831413&r1=831412&r2=831413&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.21/CHANGES.txt (original)
+++ hadoop/common/branches/branch-0.21/CHANGES.txt Fri Oct 30 18:22:31 2009
@@ -1096,6 +1096,10 @@
HADOOP-6318. Upgrade to Avro 1.2.0. (cutting)
+ HADOOP-6334. Fix GenericOptionsParser to understand URI for -files,
+ -libjars and -archives options and fix Path to support URI with fragment.
+ (Amareshwari Sriramadasu via szetszwo)
+
Release 0.20.2 - Unreleased
NEW FEATURES
Modified: hadoop/common/branches/branch-0.21/src/java/org/apache/hadoop/fs/Path.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.21/src/java/org/apache/hadoop/fs/Path.java?rev=831413&r1=831412&r2=831413&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.21/src/java/org/apache/hadoop/fs/Path.java (original)
+++ hadoop/common/branches/branch-0.21/src/java/org/apache/hadoop/fs/Path.java Fri Oct 30 18:22:31 2009
@@ -63,13 +63,13 @@
if (!(parentPath.equals("/") || parentPath.equals("")))
try {
parentUri = new URI(parentUri.getScheme(), parentUri.getAuthority(),
- parentUri.getPath()+"/", null, null);
+ parentUri.getPath()+"/", null, parentUri.getFragment());
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
URI resolved = parentUri.resolve(child.uri);
initialize(resolved.getScheme(), resolved.getAuthority(),
- normalizePath(resolved.getPath()));
+ normalizePath(resolved.getPath()), resolved.getFragment());
}
private void checkPathArg( String path ) {
@@ -123,7 +123,7 @@
// uri path is the rest of the string -- query & fragment not supported
String path = pathString.substring(start, pathString.length());
- initialize(scheme, authority, path);
+ initialize(scheme, authority, path, null);
}
/**
@@ -136,12 +136,13 @@
/** Construct a Path from components. */
public Path(String scheme, String authority, String path) {
checkPathArg( path );
- initialize(scheme, authority, path);
+ initialize(scheme, authority, path, null);
}
- private void initialize(String scheme, String authority, String path) {
+ private void initialize(String scheme, String authority, String path,
+ String fragment) {
try {
- this.uri = new URI(scheme, authority, normalizePath(path), null, null)
+ this.uri = new URI(scheme, authority, normalizePath(path), null, fragment)
.normalize();
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
@@ -253,6 +254,10 @@
path = path.substring(1); // remove slash before drive
buffer.append(path);
}
+ if (uri.getFragment() != null) {
+ buffer.append("#");
+ buffer.append(uri.getFragment());
+ }
return buffer.toString();
}
@@ -309,6 +314,7 @@
String scheme = pathUri.getScheme();
String authority = pathUri.getAuthority();
+ String fragment = pathUri.getFragment();
if (scheme != null &&
(authority != null || defaultUri.getAuthority() == null))
@@ -324,7 +330,14 @@
authority = "";
}
}
-
- return new Path(scheme+":"+"//"+authority + pathUri.getPath());
+
+ URI newUri = null;
+ try {
+ newUri = new URI(scheme, authority ,
+ normalizePath(pathUri.getPath()), null, fragment);
+ } catch (URISyntaxException e) {
+ throw new IllegalArgumentException(e);
+ }
+ return new Path(newUri);
}
}
Modified: hadoop/common/branches/branch-0.21/src/java/org/apache/hadoop/util/GenericOptionsParser.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.21/src/java/org/apache/hadoop/util/GenericOptionsParser.java?rev=831413&r1=831412&r2=831413&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.21/src/java/org/apache/hadoop/util/GenericOptionsParser.java (original)
+++ hadoop/common/branches/branch-0.21/src/java/org/apache/hadoop/util/GenericOptionsParser.java Fri Oct 30 18:22:31 2009
@@ -21,8 +21,11 @@
import java.io.IOException;
import java.io.PrintStream;
import java.net.URI;
+import java.net.URISyntaxException;
import java.net.URL;
import java.net.URLClassLoader;
+import java.util.ArrayList;
+import java.util.List;
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.CommandLineParser;
@@ -111,16 +114,20 @@
* Create an options parser with the given options to parse the args.
* @param opts the options
* @param args the command line arguments
+ * @throws IOException
*/
- public GenericOptionsParser(Options opts, String[] args) {
+ public GenericOptionsParser(Options opts, String[] args)
+ throws IOException {
this(new Configuration(), new Options(), args);
}
/**
* Create an options parser to parse the args.
* @param args the command line arguments
+ * @throws IOException
*/
- public GenericOptionsParser(String[] args) {
+ public GenericOptionsParser(String[] args)
+ throws IOException {
this(new Configuration(), new Options(), args);
}
@@ -133,8 +140,10 @@
*
* @param conf the <code>Configuration</code> to modify.
* @param args command-line arguments.
+ * @throws IOException
*/
- public GenericOptionsParser(Configuration conf, String[] args) {
+ public GenericOptionsParser(Configuration conf, String[] args)
+ throws IOException {
this(conf, new Options(), args);
}
@@ -148,8 +157,10 @@
* @param conf the configuration to modify
* @param options options built by the caller
* @param args User-specified arguments
+ * @throws IOException
*/
- public GenericOptionsParser(Configuration conf, Options options, String[] args) {
+ public GenericOptionsParser(Configuration conf,
+ Options options, String[] args) throws IOException {
parseGeneralOptions(options, conf, args);
this.conf = conf;
}
@@ -240,7 +251,7 @@
* @param line User-specified generic options
*/
private void processGeneralOptions(Configuration conf,
- CommandLine line) {
+ CommandLine line) throws IOException {
if (line.hasOption("fs")) {
FileSystem.setDefaultUri(conf, line.getOptionValue("fs"));
}
@@ -254,29 +265,25 @@
conf.addResource(new Path(value));
}
}
- try {
- if (line.hasOption("libjars")) {
- conf.set("tmpjars",
- validateFiles(line.getOptionValue("libjars"), conf));
- //setting libjars in client classpath
- URL[] libjars = getLibJars(conf);
- if(libjars!=null && libjars.length>0) {
- conf.setClassLoader(new URLClassLoader(libjars, conf.getClassLoader()));
- Thread.currentThread().setContextClassLoader(
- new URLClassLoader(libjars,
- Thread.currentThread().getContextClassLoader()));
- }
- }
- if (line.hasOption("files")) {
- conf.set("tmpfiles",
- validateFiles(line.getOptionValue("files"), conf));
- }
- if (line.hasOption("archives")) {
- conf.set("tmparchives",
- validateFiles(line.getOptionValue("archives"), conf));
+ if (line.hasOption("libjars")) {
+ conf.set("tmpjars",
+ validateFiles(line.getOptionValue("libjars"), conf));
+ //setting libjars in client classpath
+ URL[] libjars = getLibJars(conf);
+ if(libjars!=null && libjars.length>0) {
+ conf.setClassLoader(new URLClassLoader(libjars, conf.getClassLoader()));
+ Thread.currentThread().setContextClassLoader(
+ new URLClassLoader(libjars,
+ Thread.currentThread().getContextClassLoader()));
}
- } catch (IOException ioe) {
- System.err.println(StringUtils.stringifyException(ioe));
+ }
+ if (line.hasOption("files")) {
+ conf.set("tmpfiles",
+ validateFiles(line.getOptionValue("files"), conf));
+ }
+ if (line.hasOption("archives")) {
+ conf.set("tmparchives",
+ validateFiles(line.getOptionValue("archives"), conf));
}
if (line.hasOption('D')) {
String[] property = line.getOptionValues('D');
@@ -302,12 +309,14 @@
return null;
}
String[] files = jars.split(",");
- URL[] cp = new URL[files.length];
- for (int i=0;i<cp.length;i++) {
- Path tmp = new Path(files[i]);
- cp[i] = FileSystem.getLocal(conf).pathToFile(tmp).toURI().toURL();
+ List<URL> cp = new ArrayList<URL>();
+ for (String file : files) {
+ Path tmp = new Path(file);
+ if (tmp.getFileSystem(conf).equals(FileSystem.getLocal(conf))) {
+ cp.add(FileSystem.getLocal(conf).pathToFile(tmp).toURI().toURL());
+ }
}
- return cp;
+ return cp.toArray(new URL[0]);
}
/**
@@ -320,7 +329,8 @@
* @param files
* @return
*/
- private String validateFiles(String files, Configuration conf) throws IOException {
+ private String validateFiles(String files, Configuration conf)
+ throws IOException {
if (files == null)
return null;
String[] fileArr = files.split(",");
@@ -328,8 +338,13 @@
for (int i =0; i < fileArr.length; i++) {
String tmp = fileArr[i];
String finalPath;
- Path path = new Path(tmp);
- URI pathURI = path.toUri();
+ URI pathURI;
+ try {
+ pathURI = new URI(tmp);
+ } catch (URISyntaxException e) {
+ throw new IllegalArgumentException(e);
+ }
+ Path path = new Path(pathURI);
FileSystem localFs = FileSystem.getLocal(conf);
if (pathURI.getScheme() == null) {
//default to the local file system
@@ -349,9 +364,6 @@
throw new FileNotFoundException("File " + tmp + " does not exist.");
}
finalPath = path.makeQualified(fs).toString();
- try {
- fs.close();
- } catch(IOException e){};
}
finalArr[i] = finalPath;
}
@@ -367,7 +379,7 @@
* @return Command-specific arguments
*/
private String[] parseGeneralOptions(Options opts, Configuration conf,
- String[] args) {
+ String[] args) throws IOException {
opts = buildGeneralOptions(opts);
CommandLineParser parser = new GnuParser();
try {
Modified: hadoop/common/branches/branch-0.21/src/test/core/org/apache/hadoop/fs/TestPath.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.21/src/test/core/org/apache/hadoop/fs/TestPath.java?rev=831413&r1=831412&r2=831413&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.21/src/test/core/org/apache/hadoop/fs/TestPath.java (original)
+++ hadoop/common/branches/branch-0.21/src/test/core/org/apache/hadoop/fs/TestPath.java Fri Oct 30 18:22:31 2009
@@ -18,7 +18,12 @@
package org.apache.hadoop.fs;
-import java.util.*;
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import org.apache.hadoop.conf.Configuration;
+
import junit.framework.TestCase;
public class TestPath extends TestCase {
@@ -28,6 +33,8 @@
toStringTest("/foo/bar");
toStringTest("foo");
toStringTest("foo/bar");
+ toStringTest("/foo/bar#boo");
+ toStringTest("foo/bar#boo");
boolean emptyException = false;
try {
toStringTest("");
@@ -43,6 +50,8 @@
toStringTest("c:foo/bar");
toStringTest("c:foo/bar");
toStringTest("c:/foo/bar");
+ toStringTest("C:/foo/bar#boo");
+ toStringTest("C:foo/bar#boo");
}
}
@@ -148,5 +157,25 @@
assertEquals("foo://bar/baz", new Path("foo://bar/","/baz").toString());
}
+ public void testURI() throws URISyntaxException, IOException {
+ URI uri = new URI("file:///bar#baz");
+ Path path = new Path(uri);
+ assertTrue(uri.equals(new URI(path.toString())));
+
+ FileSystem fs = path.getFileSystem(new Configuration());
+ assertTrue(uri.equals(new URI(fs.makeQualified(path).toString())));
+
+ // uri without hash
+ URI uri2 = new URI("file:///bar/baz");
+ assertTrue(
+ uri2.equals(new URI(fs.makeQualified(new Path(uri2)).toString())));
+ assertEquals("foo://bar/baz#boo", new Path("foo://bar/", new Path(new URI(
+ "/baz#boo"))).toString());
+ assertEquals("foo://bar/baz/fud#boo", new Path(new Path(new URI(
+ "foo://bar/baz#bud")), new Path(new URI("fud#boo"))).toString());
+ // if the child uri is absolute path
+ assertEquals("foo://bar/fud#boo", new Path(new Path(new URI(
+ "foo://bar/baz#bud")), new Path(new URI("/fud#boo"))).toString());
+ }
}
Added: hadoop/common/branches/branch-0.21/src/test/core/org/apache/hadoop/util/TestGenericOptionsParser.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.21/src/test/core/org/apache/hadoop/util/TestGenericOptionsParser.java?rev=831413&view=auto
==============================================================================
--- hadoop/common/branches/branch-0.21/src/test/core/org/apache/hadoop/util/TestGenericOptionsParser.java (added)
+++ hadoop/common/branches/branch-0.21/src/test/core/org/apache/hadoop/util/TestGenericOptionsParser.java Fri Oct 30 18:22:31 2009
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.util;
+
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.net.URI;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+
+import junit.framework.TestCase;
+
+public class TestGenericOptionsParser extends TestCase {
+ private static File testDir =
+ new File(System.getProperty("test.build.data", "/tmp"), "generic");
+
+ public void testFilesOption() throws Exception {
+ Configuration conf = new Configuration();
+ File tmpFile = new File(testDir, "tmpfile");
+ FileSystem localFs = FileSystem.getLocal(conf);
+ Path tmpPath = new Path(tmpFile.toString());
+ localFs.create(tmpPath);
+ String[] args = new String[2];
+ // pass a files option
+ args[0] = "-files";
+ args[1] = tmpFile.toString();
+ new GenericOptionsParser(conf, args);
+ String files = conf.get("tmpfiles");
+ assertNotNull("files is null", files);
+ assertEquals("files option does not match",
+ localFs.makeQualified(tmpPath).toString(), files);
+
+ // pass file as uri
+ Configuration conf1 = new Configuration();
+ URI tmpURI = new URI(tmpFile.toString() + "#link");
+ args[0] = "-files";
+ args[1] = tmpURI.toString();
+ new GenericOptionsParser(conf1, args);
+ files = conf1.get("tmpfiles");
+ assertNotNull("files is null", files);
+ assertEquals("files option does not match",
+ localFs.makeQualified(new Path(tmpURI)).toString(), files);
+
+ // pass a file that does not exist.
+ // GenericOptionParser should throw exception
+ Configuration conf2 = new Configuration();
+ args[0] = "-files";
+ args[1] = "file:///xyz.txt";
+ Throwable th = null;
+ try {
+ new GenericOptionsParser(conf2, args);
+ } catch (Exception e) {
+ th = e;
+ }
+ assertNotNull("throwable is null", th);
+ assertTrue("FileNotFoundException is not thrown",
+ th instanceof FileNotFoundException);
+ files = conf2.get("tmpfiles");
+ assertNull("files is not null", files);
+ testDir.delete();
+ }
+
+}