Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-1146

Sqoop dependencies break Ecpilse build on Linux

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None
    • Environment:

      Linux, Sun JDK6

    • Hadoop Flags:
      Reviewed

      Description

      Under Linux there's the error in the Eclipse "Problems" view:

      - "com.sun.tools cannot be resolved" at line 166 of  org.apache.hadoop.sqoop.orm.CompilationManager
      

      The problem doesn't appear on MacOS though

      1. MAPREDUCE-1146.patch
        2 kB
        Aaron Kimball
      2. MAPREDUCE-1146.4.patch
        4 kB
        Aaron Kimball
      3. MAPREDUCE-1146.3.patch
        3 kB
        Aaron Kimball
      4. MAPREDUCE-1146.2.patch
        2 kB
        Aaron Kimball

        Activity

        Hide
        Konstantin Boudnik added a comment -

        The issue could be workarounded by adding JDK5 tools.jar into the dependecies list.

        Show
        Konstantin Boudnik added a comment - The issue could be workarounded by adding JDK5 tools.jar into the dependecies list.
        Hide
        Aaron Greenhouse added a comment -

        Actually, it doesn't have to be JDK5 tools.jar. It can be the JDK6 tools.jar. Seems to work on Mac OS because classes.jar contains more stuff than does the rt.jar found on Linux.

        Show
        Aaron Greenhouse added a comment - Actually, it doesn't have to be JDK5 tools.jar. It can be the JDK6 tools.jar. Seems to work on Mac OS because classes.jar contains more stuff than does the rt.jar found on Linux.
        Hide
        Aaron Kimball added a comment -

        Attaching a patch that fixes the issue. There's not an easy way to get a reference to the current JDK's tools.jar inside Eclipse itself. (Nor, if the user is compiling with ecj, is there necessarily a compatible tools.jar available.)

        This patch uses the text replace feature as is done with the @PROJECT@ variable to substitute in the path from the user's $JAVA_HOME environment variable. Eclipse file generation will fail if this does not point to a suitable JDK. (The path may be specified by supplying ant with -Djdk.home)

        Show
        Aaron Kimball added a comment - Attaching a patch that fixes the issue. There's not an easy way to get a reference to the current JDK's tools.jar inside Eclipse itself. (Nor, if the user is compiling with ecj, is there necessarily a compatible tools.jar available.) This patch uses the text replace feature as is done with the @PROJECT@ variable to substitute in the path from the user's $JAVA_HOME environment variable. Eclipse file generation will fail if this does not point to a suitable JDK. (The path may be specified by supplying ant with -Djdk.home )
        Hide
        Aaron Kimball added a comment -

        To clarify "file generation will fail" means that the ant task will abort with an error message and refuse to create the true eclipse files from their templates.

        Show
        Aaron Kimball added a comment - To clarify "file generation will fail" means that the ant task will abort with an error message and refuse to create the true eclipse files from their templates.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12423387/MAPREDUCE-1146.patch
        against trunk revision 829529.

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

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

        -1 patch. The patch command could not apply the patch.

        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/95/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/12423387/MAPREDUCE-1146.patch against trunk revision 829529. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/95/console This message is automatically generated.
        Hide
        Aaron Kimball added a comment -

        New patch generated with --no-prefix this time

        Show
        Aaron Kimball added a comment - New patch generated with --no-prefix this time
        Hide
        Aaron Kimball added a comment -

        New patch that handles OS X as well. On most operating systems, the javac compiler is exposed through tools.jar in the JDK's lib/ directory. In OS X, this is rolled into the monolithic classes.jar that is already implicitly on the classpath. This version of the patch checks {{$

        {os.name}

        }} to see whether it's on OS X or not. If so, it comments out the tools.jar classpath entry, otherwise, it includes it. This is some unfortunately hairy ant-fu, but I'm not sure if there's a cleaner way to do it.

        Show
        Aaron Kimball added a comment - New patch that handles OS X as well. On most operating systems, the javac compiler is exposed through tools.jar in the JDK's lib/ directory. In OS X, this is rolled into the monolithic classes.jar that is already implicitly on the classpath. This version of the patch checks {{$ {os.name} }} to see whether it's on OS X or not. If so, it comments out the tools.jar classpath entry, otherwise, it includes it. This is some unfortunately hairy ant-fu, but I'm not sure if there's a cleaner way to do it.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12423478/MAPREDUCE-1146.3.patch
        against trunk revision 830531.

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

        +1 tests included. The patch appears to include 3 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 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 contrib tests. The patch failed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/101/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/101/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/101/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/101/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/12423478/MAPREDUCE-1146.3.patch against trunk revision 830531. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 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 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 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/101/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/101/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/101/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/101/console This message is automatically generated.
        Hide
        Aaron Kimball added a comment -

        Unrelated test failure (gridmix submission)

        Show
        Aaron Kimball added a comment - Unrelated test failure (gridmix submission)
        Hide
        Chris Douglas added a comment -

        This patch uses the text replace feature as is done with the @PROJECT@ variable to substitute in the path from the user's $JAVA_HOME environment variable [...]

        It's a pretty elaborate extension of that hack. Is there any harm in adding $JAVA_HOME/lib/tools.jar to the classpath when it doesn't exist on OSX? I'm not an eclipse user; what's broken, here?

        Show
        Chris Douglas added a comment - This patch uses the text replace feature as is done with the @PROJECT@ variable to substitute in the path from the user's $JAVA_HOME environment variable [...] It's a pretty elaborate extension of that hack. Is there any harm in adding $JAVA_HOME/lib/tools.jar to the classpath when it doesn't exist on OSX? I'm not an eclipse user; what's broken, here?
        Hide
        Aaron Kimball added a comment -

        If you add an external jar to the build path in eclipse and that jar is missing, eclipse will refuse to run the build process.

        Show
        Aaron Kimball added a comment - If you add an external jar to the build path in eclipse and that jar is missing, eclipse will refuse to run the build process.
        Hide
        Aaron Kimball added a comment -

        Chris: Eclipse requires everything in its build paths to exist; we can't "overfill" it. Do you have a cleaner proposal for how to create this list in an OS-dependent fashion than what I've implemented here?

        The other option I can think of is to put some token in the eclipse classpath file and attempt to write a bash script that uses various OS shibboleths to determine whether or not its on OS X, and then use sed to replace the token (or otherwise edit the classpath file). So that would shift the logic out of ant and into a bash script. Seeing as how ant already has OS identification built in, I don't see this as a win.

        Show
        Aaron Kimball added a comment - Chris: Eclipse requires everything in its build paths to exist; we can't "overfill" it. Do you have a cleaner proposal for how to create this list in an OS-dependent fashion than what I've implemented here? The other option I can think of is to put some token in the eclipse classpath file and attempt to write a bash script that uses various OS shibboleths to determine whether or not its on OS X, and then use sed to replace the token (or otherwise edit the classpath file). So that would shift the logic out of ant and into a bash script. Seeing as how ant already has OS identification built in, I don't see this as a win.
        Hide
        Chris Douglas added a comment -

        Do you have a cleaner proposal for how to create this list in an OS-dependent fashion than what I've implemented here?

        No, but I'm not the right person to ask about eclipse arcana. Have you tried their dev lists?

        Show
        Chris Douglas added a comment - Do you have a cleaner proposal for how to create this list in an OS-dependent fashion than what I've implemented here? No, but I'm not the right person to ask about eclipse arcana. Have you tried their dev lists?
        Hide
        Aaron Kimball added a comment -

        I don't follow – this isn't related to "Eclipse arcana." The requirements imposed by Eclipse are straightforward: we need to create a classpath file containing a list of exactly the jars necessary to compile the project. We are currently doing this via a template file and some substitutions executed by our ant buildfile. To my knowledge, this is the state of the art. The file format is XML and the schema is straightforward.

        Since Sqoop requires a certain jar found in a user-defined location, only on certain platforms, I implemented conditional logic in our buildfile to check the platform, and inject the correct jar dependency into the Eclipse project classpath file if it's being constructed in a platform which requires this jar.

        When you canceled the patch, it seemed to be because you didn't like the degree of complexity I placed in the ant script. If you have a simpler solution, I'm receptive, but I don't think there's a less-complicated way (given the constraints imposed by our build system of choice, ant) to implement this, short of using ant to invoke a bash script. But I think that adds its own subtleties and complexities that make that an even less-attractive choice.

        Show
        Aaron Kimball added a comment - I don't follow – this isn't related to "Eclipse arcana." The requirements imposed by Eclipse are straightforward: we need to create a classpath file containing a list of exactly the jars necessary to compile the project. We are currently doing this via a template file and some substitutions executed by our ant buildfile. To my knowledge, this is the state of the art. The file format is XML and the schema is straightforward. Since Sqoop requires a certain jar found in a user-defined location, only on certain platforms, I implemented conditional logic in our buildfile to check the platform, and inject the correct jar dependency into the Eclipse project classpath file if it's being constructed in a platform which requires this jar. When you canceled the patch, it seemed to be because you didn't like the degree of complexity I placed in the ant script. If you have a simpler solution, I'm receptive, but I don't think there's a less-complicated way (given the constraints imposed by our build system of choice, ant) to implement this, short of using ant to invoke a bash script. But I think that adds its own subtleties and complexities that make that an even less-attractive choice.
        Hide
        Chris Douglas added a comment -

        When you canceled the patch, it seemed to be because you didn't like the degree of complexity I placed in the ant script.

        Yes. The build scripts are already awkward to maintain, and what little maintenance occurs happens as in this issue: enough to fix the immediate problem. I don't think this imposes a high maintenance burden, as the eclipse build is hardly high-traffic, but I don't think we disagree that the current patch is an indirect fix. If that's all there is, then fine, but I'd like to be sure that this does not incur undue technical debt.

        I implemented conditional logic in our buildfile to check the platform, and inject the correct jar dependency into the Eclipse project classpath file if it's being constructed in a platform which requires this jar.

        More precisely, the patch injects XML comment characters into the Eclipse project classpath file to exclude a non-existent jar. Unless the eclipse-files target were rewritten to generate- rather than inject paths into- the classpath file, I agree, it's not clear how to effect what the current patch achieves.

        this isn't related to "Eclipse arcana." The requirements imposed by Eclipse are straightforward: we need to create a classpath file containing a list of exactly the jars necessary to compile the project.

        I suggested the eclipse dev list because this can't be the first time someone has thought about this issue. Before settling on injecting XML into an eclipse configuration file using ant properties, it's not unreasonable to ask if there's a better way.

        Show
        Chris Douglas added a comment - When you canceled the patch, it seemed to be because you didn't like the degree of complexity I placed in the ant script. Yes. The build scripts are already awkward to maintain, and what little maintenance occurs happens as in this issue: enough to fix the immediate problem. I don't think this imposes a high maintenance burden, as the eclipse build is hardly high-traffic, but I don't think we disagree that the current patch is an indirect fix. If that's all there is, then fine, but I'd like to be sure that this does not incur undue technical debt. I implemented conditional logic in our buildfile to check the platform, and inject the correct jar dependency into the Eclipse project classpath file if it's being constructed in a platform which requires this jar. More precisely, the patch injects XML comment characters into the Eclipse project classpath file to exclude a non-existent jar. Unless the eclipse-files target were rewritten to generate- rather than inject paths into- the classpath file, I agree, it's not clear how to effect what the current patch achieves. this isn't related to "Eclipse arcana." The requirements imposed by Eclipse are straightforward: we need to create a classpath file containing a list of exactly the jars necessary to compile the project. I suggested the eclipse dev list because this can't be the first time someone has thought about this issue. Before settling on injecting XML into an eclipse configuration file using ant properties, it's not unreasonable to ask if there's a better way.
        Hide
        Tom White added a comment -

        The Eclipse templates are very brittle, but I think we can and should improve this. See HADOOP-6407, where I propose using Ant-Eclipse (http://sourceforge.net/projects/ant-eclipse/) to automate the generation of Eclipse files.

        Could we avoid this issue entirely by using JavaCompiler in Sqoop (new in Java 6, see http://java.sun.com/javase/6/docs/api/javax/tools/JavaCompiler.html) rather then using com.sun.tools.javac.Main?

        Show
        Tom White added a comment - The Eclipse templates are very brittle, but I think we can and should improve this. See HADOOP-6407 , where I propose using Ant-Eclipse ( http://sourceforge.net/projects/ant-eclipse/ ) to automate the generation of Eclipse files. Could we avoid this issue entirely by using JavaCompiler in Sqoop (new in Java 6, see http://java.sun.com/javase/6/docs/api/javax/tools/JavaCompiler.html ) rather then using com.sun.tools.javac.Main?
        Hide
        Doug Cutting added a comment -

        Sending a question to an Eclipse user list and waiting a day seems easy enough. Would that suffice, Chris? I searched the web and found references to this kind of problem, but most suggest using the Eclipse GUI to manually fix the classpath and none mention an automated means to fix a tools.jar-containing .classpath file for OS-X. Maven will generate an Eclipse .classpath automatically, but folks seem to frequently have trouble with this on OS-X when the classpath contains tools.jar and no automated solution is offered.

        Looking at the patch, my preference would be to name things after what they do. So I'd name the markers in the file SKIP_TOOLS_BEGIN and SKIP_TOOLS_END, and the ant target "set-eclipse-skip-tools-jar", and the ant properties "eclipse.skip.tools.jar.

        {begin,end}

        ". These will currently only be set on OS-X.

        Show
        Doug Cutting added a comment - Sending a question to an Eclipse user list and waiting a day seems easy enough. Would that suffice, Chris? I searched the web and found references to this kind of problem, but most suggest using the Eclipse GUI to manually fix the classpath and none mention an automated means to fix a tools.jar-containing .classpath file for OS-X. Maven will generate an Eclipse .classpath automatically, but folks seem to frequently have trouble with this on OS-X when the classpath contains tools.jar and no automated solution is offered. Looking at the patch, my preference would be to name things after what they do. So I'd name the markers in the file SKIP_TOOLS_BEGIN and SKIP_TOOLS_END, and the ant target "set-eclipse-skip-tools-jar", and the ant properties "eclipse.skip.tools.jar. {begin,end} ". These will currently only be set on OS-X.
        Hide
        Doug Cutting added a comment -

        > Could we avoid this issue entirely by using JavaCompiler [ ... ]

        Good suggestion, Tom! I'd forgotten about that API.

        Show
        Doug Cutting added a comment - > Could we avoid this issue entirely by using JavaCompiler [ ... ] Good suggestion, Tom! I'd forgotten about that API.
        Hide
        Aaron Kimball added a comment -

        Tom,

        Thanks for the suggestion about that API – I will take a look at that and see if that obviates this issue. Running a quick test program, the interfaces appear to be loaded from rt.jar, but the implementations still come from tools.jar on Linux. So we shouldn't need tools.jar on the classpath for building Sqoop itself. Then tools.jar should be implicitly loaded by the JVM to satisfy ToolProvider.getJavaCompiler() if necessary. A quick test on OS X shows that it loads the same JavacTool class from classes.jar there, so we don't need to special-case anything with our classpath.

        Show
        Aaron Kimball added a comment - Tom, Thanks for the suggestion about that API – I will take a look at that and see if that obviates this issue. Running a quick test program, the interfaces appear to be loaded from rt.jar, but the implementations still come from tools.jar on Linux. So we shouldn't need tools.jar on the classpath for building Sqoop itself. Then tools.jar should be implicitly loaded by the JVM to satisfy ToolProvider.getJavaCompiler() if necessary. A quick test on OS X shows that it loads the same JavacTool class from classes.jar there, so we don't need to special-case anything with our classpath.
        Hide
        Aaron Kimball added a comment -

        New patch that rewrites CompilationManager to use javax.tools.JavaCompiler and friends. This should eliminate the hard dependency on Sun's tools.jar and solve this issue.

        No new tests because the existing unit tests cover the use of CompilationManager.

        Show
        Aaron Kimball added a comment - New patch that rewrites CompilationManager to use javax.tools.JavaCompiler and friends. This should eliminate the hard dependency on Sun's tools.jar and solve this issue. No new tests because the existing unit tests cover the use of CompilationManager.
        Hide
        Tom White added a comment -

        +1 Looks good.

        Show
        Tom White added a comment - +1 Looks good.
        Hide
        Chris Douglas added a comment -

        +1

        Show
        Chris Douglas added a comment - +1
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12428239/MAPREDUCE-1146.4.patch
        against trunk revision 891524.

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

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +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 warnings.

        -1 release audit. The applied patch generated 162 release audit warnings (more than the trunk's current 0 warnings).

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

        -1 contrib tests. The patch failed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/209/testReport/
        Release audit warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/209/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/209/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/209/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/209/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/12428239/MAPREDUCE-1146.4.patch against trunk revision 891524. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +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 warnings. -1 release audit. The applied patch generated 162 release audit warnings (more than the trunk's current 0 warnings). +1 core tests. The patch passed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/209/testReport/ Release audit warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/209/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/209/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/209/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Mapreduce-Patch-h3.grid.sp2.yahoo.net/209/console This message is automatically generated.
        Hide
        Tom White added a comment -

        I've just committed this. Thanks Aaron!

        (There were actually no release audit warnings introduced by this patch. Also, the test failures were unrelated.)

        Show
        Tom White added a comment - I've just committed this. Thanks Aaron! (There were actually no release audit warnings introduced by this patch. Also, the test failures were unrelated.)
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #196 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk/196/)

        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #196 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk/196/ )

          People

          • Assignee:
            Aaron Kimball
            Reporter:
            Konstantin Boudnik
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development