ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-1178

Add eclipse target for supporting Apache IvyDE

    Details

    • Type: Improvement Improvement
    • Status: Patch Available
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: 3.5.0
    • Component/s: build
    • Labels:
      None
    • Environment:

      Mac OS X w/ Eclipse 3.7. However, I believe this will work in any Eclipse environment.

    • Release Note:
      Add support for Eclipse with Apache IvyDE extension

      Description

      This patch adds support for Eclipse with Apache IvyDE, which is the extension that integrates Ivy support into Eclipse. This allows the creation of what appear to be fully portable .eclipse and .classpath files. I will be posting a patch shortly.

        Issue Links

          Activity

          Hide
          Michi Mutsuzaki added a comment -

          ping

          Show
          Michi Mutsuzaki added a comment - ping
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12541011/ZOOKEEPER-1178.patch
          against trunk revision 1577756.

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

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

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1977//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1977//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1977//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/12541011/ZOOKEEPER-1178.patch against trunk revision 1577756. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 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 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1977//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1977//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1977//console This message is automatically generated.
          Hide
          Michi Mutsuzaki added a comment -

          Pat, is this patch good to go?

          Show
          Michi Mutsuzaki added a comment - Pat, is this patch good to go?
          Hide
          Warren Turkal added a comment -

          ping...Please?

          Show
          Warren Turkal added a comment - ping ...Please?
          Hide
          Warren Turkal added a comment -

          Ping again.

          Show
          Warren Turkal added a comment - Ping again.
          Hide
          Warren Turkal added a comment -

          ping

          Is there anything else I need to do to get this patch submitted?

          Show
          Warren Turkal added a comment - ping Is there anything else I need to do to get this patch submitted?
          Hide
          Warren Turkal added a comment -

          I think this one's ready for inclusion submission. Can a committer please pull the trigger?

          Thanks.

          Show
          Warren Turkal added a comment - I think this one's ready for inclusion submission. Can a committer please pull the trigger? Thanks.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12541011/ZOOKEEPER-1178.patch
          against trunk revision 1373156.

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

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

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1162//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1162//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1162//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/12541011/ZOOKEEPER-1178.patch against trunk revision 1373156. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 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 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1162//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1162//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1162//console This message is automatically generated.
          Hide
          Warren Turkal added a comment -

          Oops. Last patch generated with incorrect git config.

          Show
          Warren Turkal added a comment - Oops. Last patch generated with incorrect git config.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12541007/ZOOKEEPER-1178.patch
          against trunk revision 1373156.

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

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

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

          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1161//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/12541007/ZOOKEEPER-1178.patch against trunk revision 1373156. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1161//console This message is automatically generated.
          Hide
          Warren Turkal added a comment -

          Re-cut patch from tip of trunk.

          Show
          Warren Turkal added a comment - Re-cut patch from tip of trunk.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12538960/ZOOKEEPER-1178.patch
          against trunk revision 1373156.

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

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

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

          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1160//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/12538960/ZOOKEEPER-1178.patch against trunk revision 1373156. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1160//console This message is automatically generated.
          Hide
          Warren Turkal added a comment -

          Is there anything else that needs to be done before this can be submitted?

          Show
          Warren Turkal added a comment - Is there anything else that needs to be done before this can be submitted?
          Hide
          Warren Turkal added a comment -

          Added license headers.

          Show
          Warren Turkal added a comment - Added license headers.
          Hide
          Warren Turkal added a comment -

          The newest patch is of that form. I still need to add the license headers. I'll do that now.

          Show
          Warren Turkal added a comment - The newest patch is of that form. I still need to add the license headers. I'll do that now.
          Hide
          Patrick Hunt added a comment -

          Yes, that's what I was thinking - make it a directory.

          Show
          Patrick Hunt added a comment - Yes, that's what I was thinking - make it a directory.
          Hide
          Warren Turkal added a comment -

          Updated patch.

          Show
          Warren Turkal added a comment - Updated patch.
          Hide
          Warren Turkal added a comment -

          I completely misread that. I now think you were saying to make .eclipse_with_ivyde a directory. That seems reasonable. I'll update the patch.

          Show
          Warren Turkal added a comment - I completely misread that. I now think you were saying to make .eclipse_with_ivyde a directory. That seems reasonable. I'll update the patch.
          Hide
          Warren Turkal added a comment -

          Gah, I must have overlooked you commment while I was scanning over the ticket. I really don't like putting them in a hidden file at the top level if they aren't going to be used by eclipse directly. Ideally, the files would just be the toplevel .eclipse and .classpath files and we could just require that folks use the Apache IvyDE extension for Eclipse. If that's no good, I just want to put them in a reasonable and visible place so that they aren't hidden from folks. I also don't want them to clutter the root of the repo if they are not hidden.

          If conf isn't a good place, should I just create an /ide_support/eclipse_files directory and put the files there?

          Show
          Warren Turkal added a comment - Gah, I must have overlooked you commment while I was scanning over the ticket. I really don't like putting them in a hidden file at the top level if they aren't going to be used by eclipse directly. Ideally, the files would just be the toplevel .eclipse and .classpath files and we could just require that folks use the Apache IvyDE extension for Eclipse. If that's no good, I just want to put them in a reasonable and visible place so that they aren't hidden from folks. I also don't want them to clutter the root of the repo if they are not hidden. If conf isn't a good place, should I just create an /ide_support/eclipse_files directory and put the files there?
          Hide
          Patrick Hunt added a comment -

          what did you think of my previous comment? specifically:

          3) don't put the "eclipse_with_ivyde" into conf - that's for the main code configuration. Perhaps just put it at the toplevel (same level as conf) as ".eclipse_with_ivyde" ?

          Also, for the new files you are adding please add Apache license headers. Thx.

          Show
          Patrick Hunt added a comment - what did you think of my previous comment? specifically: 3) don't put the "eclipse_with_ivyde" into conf - that's for the main code configuration. Perhaps just put it at the toplevel (same level as conf) as ".eclipse_with_ivyde" ? Also, for the new files you are adding please add Apache license headers. Thx.
          Hide
          Warren Turkal added a comment -

          Where should these files go? They aren't really templates. They are the actual files that should be used by eclipse. If possible, I'd like to go ahead and tie this one off so that I can add the info about how to use this support to the UsingEclipse page on the wiki.

          Show
          Warren Turkal added a comment - Where should these files go? They aren't really templates. They are the actual files that should be used by eclipse. If possible, I'd like to go ahead and tie this one off so that I can add the info about how to use this support to the UsingEclipse page on the wiki.
          Hide
          Warren Turkal added a comment -

          Updated to trunk as of 2012-07-29.

          Show
          Warren Turkal added a comment - Updated to trunk as of 2012-07-29.
          Hide
          Patrick Hunt added a comment -

          I see pig calls it ".eclipse.templates" while hive calls it "eclipse.templates" so I doubt we'll get my objections - imo we should rename it to ".eclipse.templates" (I don't see any objections to my original suggestion either, so we're probably safe).

          Note - if you use git be sure to use the --no-prefix option when creating the patch. Regards.

          Show
          Patrick Hunt added a comment - I see pig calls it ".eclipse.templates" while hive calls it "eclipse.templates" so I doubt we'll get my objections - imo we should rename it to ".eclipse.templates" (I don't see any objections to my original suggestion either, so we're probably safe). Note - if you use git be sure to use the --no-prefix option when creating the patch. Regards.
          Hide
          Warren Turkal added a comment -

          Fixed the location of build class files.

          Show
          Warren Turkal added a comment - Fixed the location of build class files.
          Hide
          Warren Turkal added a comment -

          Please give this a new look when you get a chance. It's slightly improved from the last version, and it really improves the eclipse workflow when compared to the current way that the build.xml has to generate the eclipse metadata.

          Show
          Warren Turkal added a comment - Please give this a new look when you get a chance. It's slightly improved from the last version, and it really improves the eclipse workflow when compared to the current way that the build.xml has to generate the eclipse metadata.
          Hide
          Warren Turkal added a comment -

          This patch makes the eclipse-with-ivyde rule depend on building the generated code.

          Show
          Warren Turkal added a comment - This patch makes the eclipse-with-ivyde rule depend on building the generated code.
          Hide
          Warren Turkal added a comment -

          I changed the filename of the patch to ZOOKEEPER-1178.patch like you said.

          As for your other questions:
          1) This is better than using the ant-eclipse.jar as it doesn't just try to detect the project structure, you can actually have the real .project and .classpath files checked in. Using ant-eclipse.jar, you cannot do that as the .classpath file will contain absolute paths to some of the jar the are dependencies for the build, which means that .classpath contains local settings. Using this patch with the Apache IvyDE extension for eclipse allows the .classpath to refer to the Ivy deps in a way that doesn't involve putting absolute paths into the .classpath file. Compare the .classpath from a newly generated zookeeper eclipse project with "ant eclipse" vs. the .classpath in this patch. The line containing the string IVYDE is the important one in the new file in the patch. Also, having the real .project file checked in should allow us to have more complex configuration out-of-the-box for Eclipse users, such as having the code style pre-configured.
          2) I would love to do so.
          3) Ok, that kinda sets a new president for where files like this should go. Is that okay with others in the project?

          With regard to this patch, I honestly think that the .eclipse and .classpath files should eventually be moved to the root of the project so that anyone using eclipse can just import the project. We could even include a code style and other project settings that all eclipse users should have. The user specific .settings directory should not be included in the repo, of course. AFAICT, the only thing stopping that from being realistic today is having the ant-eclipse.jar generate a .classpath with absolute paths, which this patch addresses at the cost of requiring the IvyDE extension. However, I think that requiring Eclipse developers to use the IvyDE extension is a pretty small barrier since (a) zookeeper already has an ivy configuration and (b) it's an free and open source (Apache even) extension.

          Show
          Warren Turkal added a comment - I changed the filename of the patch to ZOOKEEPER-1178 .patch like you said. As for your other questions: 1) This is better than using the ant-eclipse.jar as it doesn't just try to detect the project structure, you can actually have the real .project and .classpath files checked in. Using ant-eclipse.jar, you cannot do that as the .classpath file will contain absolute paths to some of the jar the are dependencies for the build, which means that .classpath contains local settings. Using this patch with the Apache IvyDE extension for eclipse allows the .classpath to refer to the Ivy deps in a way that doesn't involve putting absolute paths into the .classpath file. Compare the .classpath from a newly generated zookeeper eclipse project with "ant eclipse" vs. the .classpath in this patch. The line containing the string IVYDE is the important one in the new file in the patch. Also, having the real .project file checked in should allow us to have more complex configuration out-of-the-box for Eclipse users, such as having the code style pre-configured. 2) I would love to do so. 3) Ok, that kinda sets a new president for where files like this should go. Is that okay with others in the project? With regard to this patch, I honestly think that the .eclipse and .classpath files should eventually be moved to the root of the project so that anyone using eclipse can just import the project. We could even include a code style and other project settings that all eclipse users should have. The user specific .settings directory should not be included in the repo, of course. AFAICT, the only thing stopping that from being realistic today is having the ant-eclipse.jar generate a .classpath with absolute paths, which this patch addresses at the cost of requiring the IvyDE extension. However, I think that requiring Eclipse developers to use the IvyDE extension is a pretty small barrier since (a) zookeeper already has an ivy configuration and (b) it's an free and open source (Apache even) extension.
          Hide
          Patrick Hunt added a comment -

          This looks fine to me, but some suggestions:

          1) what does this do for me and how do I use it?
          2) after this gets committed it would be great to add (1) to
          https://cwiki.apache.org/confluence/display/ZOOKEEPER/UsingEclipse
          3) don't put the "eclipse_with_ivyde" into conf - that's for the main code configuration. Perhaps just put it at the toplevel (same level as conf) as ".eclipse_with_ivyde" ?

          Show
          Patrick Hunt added a comment - This looks fine to me, but some suggestions: 1) what does this do for me and how do I use it? 2) after this gets committed it would be great to add (1) to https://cwiki.apache.org/confluence/display/ZOOKEEPER/UsingEclipse 3) don't put the "eclipse_with_ivyde" into conf - that's for the main code configuration. Perhaps just put it at the toplevel (same level as conf) as ".eclipse_with_ivyde" ?
          Hide
          Patrick Hunt added a comment -

          Thanks Warren, please do try to name the patch after the JIRA #, ie ZOOKEEPER-1178.patch in this case, see:
          https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute

          Show
          Patrick Hunt added a comment - Thanks Warren, please do try to name the patch after the JIRA #, ie ZOOKEEPER-1178 .patch in this case, see: https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute
          Hide
          Warren Turkal added a comment -

          Looks like this issue is still unassigned. Would anyone be willing to take it on. It's a really small patch.

          Show
          Warren Turkal added a comment - Looks like this issue is still unassigned. Would anyone be willing to take it on. It's a really small patch.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12494054/eclipse-apache-ivyde-support.patch
          against trunk revision 1166970.

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

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

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/524//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/524//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/524//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/12494054/eclipse-apache-ivyde-support.patch against trunk revision 1166970. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 1 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 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/524//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/524//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/524//console This message is automatically generated.
          Hide
          Warren Turkal added a comment -

          While I would prefer to commit a patch that includes a .eclipse and .classpath file in the $

          {basedir}

          since these don't depend on local configuration, that would require that everyone using Eclipse have no local project file modifications and have Apache IvyDE installed. I think that's probably a reasonable goal. However, I don't want that requirement to block this patch. This patch just adds a new ant target "eclipse-with-ivyde" that copies some templates files for .project and .classpath into place. Of course, the patch also includes the template files.

          Show
          Warren Turkal added a comment - While I would prefer to commit a patch that includes a .eclipse and .classpath file in the $ {basedir} since these don't depend on local configuration, that would require that everyone using Eclipse have no local project file modifications and have Apache IvyDE installed. I think that's probably a reasonable goal. However, I don't want that requirement to block this patch. This patch just adds a new ant target "eclipse-with-ivyde" that copies some templates files for .project and .classpath into place. Of course, the patch also includes the template files.

            People

            • Assignee:
              Warren Turkal
              Reporter:
              Warren Turkal
            • Votes:
              1 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:

                Time Tracking

                Estimated:
                Original Estimate - 1h
                1h
                Remaining:
                Remaining Estimate - 1h
                1h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development