Reviewer needed: fix for class RegularFileObject + added new regression test
Dr Andrew John Hughes
ahughes at redhat.com
Mon Mar 28 10:34:50 PDT 2011
On 17:23 Mon 28 Mar , Pavel Tisnovsky wrote:
> Hi all,
>
> I've prepared patch which fixes behaviour of RegularFileObject class.
> There's a difference between IcedTea6-1.9 and IcedTea6-1.10|HEAD in
> path+object(file) name returned by the method JavaFileObject.getName()
> which is caused by changes made in OpenJDK's sources by this quite big
> and IMHO unclear changeset:
> http://hg.openjdk.java.net/jdk6/jdk6/langtools/rev/5c2858bccb3f
>
Yes, I agree that changeset is hideous. An example of why you shouldn't
roll more than one change into a single changeset.
> Here are results for three various IcedTea versions:
>
> IcedTea6-1.9.x output:
>
> Source location: [./data]
> JavaFileObject.getName() = Test.html
> JavaFileObject.getName() = Test.java
> JavaFileObject.getName() = Test.class
> JavaFileObject.getName() = Test.txt
>
> Unpatched IcedTea6-HEAD output:
>
> Source location: [./data]
> JavaFileObject.getName() = ./test-data/Test.html
> JavaFileObject.getName() = ./test-data/Test.java
> JavaFileObject.getName() = ./test-data/Test.class
> JavaFileObject.getName() = ./test-data/Test.txt
>
> Pathes IcedTea6-HEAD output:
>
> Source location: [./data]
> JavaFileObject.getName() = Test.html
> JavaFileObject.getName() = Test.java
> JavaFileObject.getName() = Test.class
> JavaFileObject.getName() = Test.txt
>
>
>
> This patch also contains new regression test which checks the proper
> behaviour of JavaFileObject and JavaFileManager classes, ie. whether
> correct path is returned for four types of file objects:
>
> - java source
> - java class (I mean source compiled to bytecode)
> - HTML file
> - other file (in this case text file)
>
> (three of this files are included in patch, fourth - .class - is
> automatically generated by JTreg tool)
>
> FYI: this patch also fixes one TCK failure.
>
> hg diff created against recent IcedTea6 HEAD version is stored in
> attachment.
>
> Here's ChangeLog entry:
>
> 2011-03-28 Pavel Tisnovsky <ptisnovs at redhat.com>
>
> * Makefile.am: Add new patch.
> * patches/regular-file-object-patch:
> Patch which fixes the behaviour of JavaFileManager
> and JavaFileObject classes. This patch also contains
> regression test for check it the fix is correct.
>
>
> Can anybody review this change please?
>
> Thank you in advance,
> Pavel
I think a better name for the patch would be revert-6885123.patch
(6885123 being the getName() bug that was fixed by this change, but
causes the behaviour of a standard 1.6 API to change).
With that change, it's good to go in.
I would guess you want this in the 1.10.x series too. If so, can you
post a version for 1.10 and I'll approve it for the branch too? Thanks.
> diff -r 6fb1dbbbd0d3 Makefile.am
> --- a/Makefile.am Sat Mar 26 08:30:50 2011 +0100
> +++ b/Makefile.am Mon Mar 28 16:22:55 2011 +0200
> @@ -331,7 +331,8 @@
> patches/openjdk/7027667-AAShapePipeRegTest.patch \
> patches/openjdk/7019861-AA-regression-fix.patch \
> patches/openjdk/6768387-jtable_not_serializable.patch \
> - patches/mark_sun_toolkit_privileged_code.patch
> + patches/mark_sun_toolkit_privileged_code.patch \
> + patches/regular-file-object.patch
>
> if WITH_ALT_HSBUILD
> ICEDTEA_PATCHES += \
> diff -r 6fb1dbbbd0d3 patches/regular-file-object.patch
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/patches/regular-file-object.patch Mon Mar 28 16:22:55 2011 +0200
> @@ -0,0 +1,310 @@
> +--- openjdk.old/langtools/src/share/classes/com/sun/tools/javac/file/RegularFileObject.java 2011-02-28 17:07:28.000000000 +0100
> ++++ openjdk/langtools/src/share/classes/com/sun/tools/javac/file/RegularFileObject.java 2011-03-25 10:16:13.646180000 +0100
> +@@ -78,7 +78,7 @@
> +
> + //@Override
> + public String getName() {
> +- return file.getPath();
> ++ return this.name;
> + }
> +
> + //@Override
> +--- /dev/null 1970-01-01 01:00:00.000000000 +0100
> ++++ openjdk/jdk/test/javax/tools/JavaFileManager/JavaFileManagerTest.java 2011-03-28 15:51:43.000000000 +0200
> +@@ -0,0 +1,119 @@
> ++/*
> ++ * Copyright 2011 Red Hat, Inc. All Rights Reserved.
> ++ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> ++ *
> ++ * This code is free software; you can redistribute it and/or modify it
> ++ * under the terms of the GNU General Public License version 2 only, as
> ++ * published by the Free Software Foundation.
> ++ *
> ++ * This code is distributed in the hope that it will be useful, but WITHOUT
> ++ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> ++ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> ++ * version 2 for more details (a copy is included in the LICENSE file that
> ++ * accompanied this code).
> ++ *
> ++ * You should have received a copy of the GNU General Public License version
> ++ * 2 along with this work; if not, write to the Free Software Foundation,
> ++ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
> ++ */
> ++
> ++
> ++import java.util.*;
> ++import java.io.*;
> ++import java.nio.charset.*;
> ++
> ++import javax.tools.*;
> ++import javax.tools.JavaFileManager.*;
> ++import javax.tools.JavaFileObject.*;
> ++
> ++public class JavaFileManagerTest {
> ++
> ++ /**
> ++ * Test data files are stored in a subdirectory named "test_data".
> ++ */
> ++ private static final String TEST_DATA_DIRECTORY = "test_data";
> ++
> ++ /**
> ++ * Charset used by JavaFileManager.
> ++ */
> ++ private static final Charset FILE_MANAGER_CHARSET = Charset.forName("US-ASCII");
> ++
> ++ /**
> ++ * Locale used by JavaFileManager.
> ++ */
> ++ private static final Locale FILE_MANAGER_LOCALE = (java.util.Locale)null;
> ++
> ++ /**
> ++ * Location of file objects being tested.
> ++ */
> ++ private static final Location FILE_OBJECTS_LOCATION = javax.tools.StandardLocation.SOURCE_PATH;
> ++
> ++ public static DiagnosticListener<JavaFileObject> listener = new DiagnosticCollector<JavaFileObject>();
> ++
> ++ private void doTest() throws IOException {
> ++ StandardJavaFileManager fileManager = getStandardFileManager();
> ++ fileManager.setLocation(FILE_OBJECTS_LOCATION, Collections.singleton(getSourceDirPathFile()));
> ++
> ++ checkFileManagerLocation(fileManager.getLocation(FILE_OBJECTS_LOCATION));
> ++ checkFileManagerGetName(fileManager);
> ++ }
> ++
> ++ private StandardJavaFileManager getStandardFileManager() {
> ++ return ToolProvider.getSystemJavaCompiler().getStandardFileManager(
> ++ listener, FILE_MANAGER_LOCALE, FILE_MANAGER_CHARSET);
> ++ }
> ++
> ++ public static File getSourceDirPathFile() {
> ++ return new File(new File("."), TEST_DATA_DIRECTORY);
> ++ }
> ++
> ++ /**
> ++ * Check if there's "./test_data" location included in a locations argument.
> ++ * @param locations set of locations returned by fileManager.getLocation()
> ++ */
> ++ private void checkFileManagerLocation(Iterable<? extends File> locations) {
> ++ for (File file : locations) {
> ++ if (".".equals(file.getParent()) && TEST_DATA_DIRECTORY.equals(file.getName())) {
> ++ System.out.println("checkFileManagerLocation: ok");
> ++ return;
> ++ }
> ++ }
> ++ throw new RuntimeException("fileManager.getLocation() returns wrong locations! " + locations.toString());
> ++ }
> ++
> ++ /**
> ++ * Check if method JavaFileManager.getName() returns correct file name.
> ++ * @param fileManager
> ++ * @throws IOException
> ++ */
> ++ private void checkFileManagerGetName(JavaFileManager fileManager) throws IOException {
> ++ Map<Kind, String> kinds = new HashMap<Kind, String>();
> ++ kinds.put(Kind.SOURCE, "Test.java");
> ++ kinds.put(Kind.CLASS, "Test.class");
> ++ kinds.put(Kind.HTML, "Test.html");
> ++ kinds.put(Kind.OTHER, "Test.txt");
> ++ for (Map.Entry<Kind, String> kind : kinds.entrySet()) {
> ++ System.out.println("Testing JavaFileManager for kind " + kind.getKey());
> ++ Iterable<JavaFileObject> fileObjects = fileManager.list(
> ++ FILE_OBJECTS_LOCATION, "", Collections.singleton(kind.getKey()), false);
> ++ checkIfFileObjectsContainName(fileObjects, kind.getValue());
> ++ }
> ++ }
> ++
> ++ private void checkIfFileObjectsContainName(Iterable<JavaFileObject> fileObjects, String name) {
> ++ //System.out.println(fileObjects);
> ++ for (JavaFileObject fileObject : fileObjects) {
> ++ //System.out.println(fileObject.getName());
> ++ if (name.equals(fileObject.getName())) {
> ++ System.out.println("JavaFileObject.getName(): ok ('" + fileObject.getName() + "')");
> ++ return;
> ++ }
> ++ }
> ++ throw new RuntimeException("JavaFileObject.getName() returns wrong name!");
> ++ }
> ++
> ++ public static void main(String argv[]) throws IOException {
> ++ new JavaFileManagerTest().doTest();
> ++ }
> ++
> ++}
> +--- /dev/null 1970-01-01 01:00:00.000000000 +0100
> ++++ openjdk/jdk/test/javax/tools/JavaFileManager/JavaFileManagerTest.sh 2011-03-28 15:47:23.000000000 +0200
> +@@ -0,0 +1,153 @@
> ++#!/bin/sh
> ++
> ++#
> ++# Copyright 2011 Red Hat, Inc. All Rights Reserved.
> ++# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> ++#
> ++# This code is free software; you can redistribute it and/or modify it
> ++# under the terms of the GNU General Public License version 2 only, as
> ++# published by the Free Software Foundation.
> ++#
> ++# This code is distributed in the hope that it will be useful, but WITHOUT
> ++# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> ++# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> ++# version 2 for more details (a copy is included in the LICENSE file that
> ++# accompanied this code).
> ++#
> ++# You should have received a copy of the GNU General Public License version
> ++# 2 along with this work; if not, write to the Free Software Foundation,
> ++# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
> ++
> ++#
> ++# This script is the actual launcher of JavaFileManagerTest.java
> ++#
> ++
> ++# @test
> ++# @summary Test that check proper behaviour of JavaFileManager and
> ++# JavaFileObject classes.
> ++
> ++
> ++
> ++# check arguments set by JTreg
> ++function checkArgs() {
> ++ if [ "${TESTSRC}" = "" ]
> ++ then
> ++ echo "TESTSRC not set. Test cannot execute. Failed."
> ++ exit 1
> ++ fi
> ++ echo "TESTSRC=${TESTSRC}"
> ++ if [ "${TESTJAVA}" = "" ]
> ++ then
> ++ echo "TESTJAVA not set. Test cannot execute. Failed."
> ++ exit 1
> ++ fi
> ++ echo "TESTJAVA=${TESTJAVA}"
> ++ if [ "${TESTCLASSES}" = "" ]
> ++ then
> ++ echo "TESTCLASSES not set. Test cannot execute. Failed."
> ++ exit 1
> ++ fi
> ++}
> ++
> ++
> ++
> ++# print paths to test sources and test classes
> ++function printPaths() {
> ++ echo "TESTCLASSES=${TESTCLASSES}"
> ++ echo "CLASSPATH=${CLASSPATH}"
> ++}
> ++
> ++
> ++
> ++# copy of all necessarry files into work directory
> ++function copyWorkFiles() {
> ++ cp -r "${TESTSRC}/test_data" "${TESTCLASSES}/"
> ++ result=$?
> ++ if [ $result -eq 0 ]
> ++ then
> ++ echo "Copy of work files was successful."
> ++ else
> ++ echo "copy of work files have failed."
> ++ exit $result
> ++ fi
> ++
> ++ cp "${TESTSRC}/JavaFileManagerTest.java" "${TESTCLASSES}"
> ++ result=$?
> ++ if [ $result -eq 0 ]
> ++ then
> ++ echo "Copy of work files was successful."
> ++ else
> ++ echo "copy of work files have failed."
> ++ exit $result
> ++ fi
> ++}
> ++
> ++
> ++
> ++# compilation of Test class which is stored in test_data subdirectory
> ++function compileTestClass() {
> ++ COMPILE="${TESTJAVA}/bin/javac ${TESTCLASSES}/test_data/Test.java"
> ++ echo ${COMPILE}
> ++ ${COMPILE}
> ++ result=$?
> ++
> ++ if [ $result -eq 0 ]
> ++ then
> ++ echo "Compilation of the test class was successful."
> ++ else
> ++ echo "Compilation of the test class have failed."
> ++ # Cleanup
> ++ rm -rf ${TESTCLASSES}/
> ++ exit $result
> ++ fi
> ++}
> ++
> ++
> ++
> ++# compilation of the Test itself
> ++function compileTest() {
> ++ COMPILE="${TESTJAVA}/bin/javac ${TESTCLASSES}/JavaFileManagerTest.java"
> ++ echo ${COMPILE}
> ++ ${COMPILE}
> ++ result=$?
> ++
> ++ if [ $result -eq 0 ]
> ++ then
> ++ echo "Compilation of the test case was successful."
> ++ else
> ++ echo "Compilation of the test case failed."
> ++ # Cleanup
> ++ rm -rf ${TESTCLASSES}/
> ++ exit $result
> ++ fi
> ++}
> ++
> ++
> ++
> ++# run the test
> ++function runTest() {
> ++ cd ${TESTCLASSES}
> ++ RUNCMD="${TESTJAVA}/bin/java JavaFileManagerTest"
> ++ echo ${RUNCMD}
> ++ ${RUNCMD}
> ++ result=$?
> ++
> ++ if [ $result -eq 0 ]
> ++ then
> ++ echo "Execution successful"
> ++ else
> ++ echo "Execution of the test case failed."
> ++ fi
> ++ rm -rf ${TESTCLASSES}/
> ++ exit $result
> ++}
> ++
> ++
> ++
> ++checkArgs
> ++printPaths
> ++copyWorkFiles
> ++compileTestClass
> ++compileTest
> ++runTest
> ++
> +--- /dev/null 1970-01-01 01:00:00.000000000 +0100
> ++++ openjdk/jdk/test/javax/tools/JavaFileManager/test_data/Test.html 2011-03-25 18:19:45.000000000 +0100
> +@@ -0,0 +1,5 @@
> ++<html>
> ++<body>
> ++42
> ++</body>
> ++</html>
> +--- /dev/null 1970-01-01 01:00:00.000000000 +0100
> ++++ openjdk/jdk/test/javax/tools/JavaFileManager/test_data/Test.java 2011-03-28 14:53:50.000000000 +0200
> +@@ -0,0 +1,5 @@
> ++package test_data;
> ++
> ++public class Test {
> ++ // nothing here
> ++}
> +--- /dev/null 1970-01-01 01:00:00.000000000 +0100
> ++++ openjdk/jdk/test/javax/tools/JavaFileManager/test_data/Test.txt 2011-03-28 16:15:09.000000000 +0200
> +@@ -0,0 +1,2 @@
> ++42
> ++
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
More information about the distro-pkg-dev
mailing list