Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-1035

Generate Eclipse's .classpath file from Ivy config

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.22.0
    • Component/s: build
    • Labels:
    • Release Note:
      Added support to auto-generate the Eclipse .classpath file from ivy.

      Description

      HDFS companion issue for HADOOP-6407.

      1. HDFS-1035.patch
        8 kB
        Nigel Daley
      2. HDFS-1035.patch
        9 kB
        Nigel Daley

        Issue Links

          Activity

          Hide
          Nigel Daley added a comment -

          I just committed this and updated the wiki doc for Eclipse.

          Show
          Nigel Daley added a comment - I just committed this and updated the wiki doc for Eclipse.
          Hide
          Tom White added a comment -

          +1 works for me

          Show
          Tom White added a comment - +1 works for me
          Hide
          Konstantin Boudnik added a comment -

          Yes, the valid point.

          +1 patch looks good.

          Show
          Konstantin Boudnik added a comment - Yes, the valid point. +1 patch looks good.
          Hide
          Nigel Daley added a comment -

          Why would you make reference to a file in build.xml that no longer exists? A properly applied patch by the committer will remove this file.

          Show
          Nigel Daley added a comment - Why would you make reference to a file in build.xml that no longer exists? A properly applied patch by the committer will remove this file.
          Hide
          Konstantin Boudnik added a comment -

          Well, apparently this is a good info. However, if .eclipse.templates/.classpath is not in use any more (and apparently it isn't, because the generation is done to .classpath file) then it has to be excluded from the copying in my opinion. Otherwise, freshly generated filed will be overridden with whatever junk might happen to be there.

          Show
          Konstantin Boudnik added a comment - Well, apparently this is a good info. However, if .eclipse.templates/.classpath is not in use any more (and apparently it isn't, because the generation is done to .classpath file) then it has to be excluded from the copying in my opinion. Otherwise, freshly generated filed will be overridden with whatever junk might happen to be there.
          Hide
          Nigel Daley added a comment -

          Ah, you need to pass -E to the patch command when applying the patch so that the emptied file is removed. This is how all Hadoop patches should be applied.

          Show
          Nigel Daley added a comment - Ah, you need to pass -E to the patch command when applying the patch so that the emptied file is removed. This is how all Hadoop patches should be applied.
          Hide
          Konstantin Boudnik added a comment -

          Ok, looks like I have found out what's going on: the patch copies .eclipse.templates/.classpath if it exists. That explains why I was getting zero-sized .classpath file after this patch application. To test the assumption I have created

          cat > .eclipse.templates/.classpath
          Hello dude!
          ^D
          

          and then ran ant eclipse. No wonder the result file had the same content.

          I think

          +       <copy todir="." overwrite="true">
          

          needs to exclude more than just README.txt.

          Show
          Konstantin Boudnik added a comment - Ok, looks like I have found out what's going on: the patch copies .eclipse.templates/.classpath if it exists. That explains why I was getting zero-sized .classpath file after this patch application. To test the assumption I have created cat > .eclipse.templates/.classpath Hello dude! ^D and then ran ant eclipse . No wonder the result file had the same content. I think + <copy todir="." overwrite="true"> needs to exclude more than just README.txt.
          Hide
          Konstantin Boudnik added a comment -

          I guess not including contrib is the right thing to do. However.. all I see is that generated .classpath in my hdfs workspace has zero-length. I believe it isn't suppose to be this way.

          Show
          Konstantin Boudnik added a comment - I guess not including contrib is the right thing to do. However.. all I see is that generated .classpath in my hdfs workspace has zero-length. I believe it isn't suppose to be this way.
          Hide
          Nigel Daley added a comment -

          Cos, here's a new patch. It doesn't include contrib components in the classpath as there doesn't seem to be a good way to resolve the contrib component classpaths via ivy from this top level build.xml file. Not sure if excluding the contrib's (hdfsproxy and thriftfs) from the Eclipse classpath matters to anyone. Comments?

          Show
          Nigel Daley added a comment - Cos, here's a new patch. It doesn't include contrib components in the classpath as there doesn't seem to be a good way to resolve the contrib component classpaths via ivy from this top level build.xml file. Not sure if excluding the contrib's (hdfsproxy and thriftfs) from the Eclipse classpath matters to anyone. Comments?
          Hide
          Konstantin Boudnik added a comment -
          1. Applied the patch
          2. removed .classpath file
          3. run ant eclipse
          4. rw-rr- 1 xxx users 0 2010-10-22 09:11 .classpath

          Have I done something wrong?

          Feel free to fix them and upload a new patch

          Awesome! As soon as this one will perform

          Show
          Konstantin Boudnik added a comment - Applied the patch removed .classpath file run ant eclipse rw-r r - 1 xxx users 0 2010-10-22 09:11 .classpath Have I done something wrong? Feel free to fix them and upload a new patch Awesome! As soon as this one will perform
          Hide
          Nigel Daley added a comment -

          Good call on needing to update the document. The reason you didn't notice any change is that it produces the exact same .classpath file from ivy deps vs the one you previously had from the template file. If you blow away your .classpath file and run this, it should create the exact same file.

          I agree that the rest of the comments are nits. Feel free to fix them and upload a new patch. Otherwise I'll commit this and MAPREDUCE-1592 this weekend.

          Show
          Nigel Daley added a comment - Good call on needing to update the document. The reason you didn't notice any change is that it produces the exact same .classpath file from ivy deps vs the one you previously had from the template file. If you blow away your .classpath file and run this, it should create the exact same file. I agree that the rest of the comments are nits. Feel free to fix them and upload a new patch. Otherwise I'll commit this and MAPREDUCE-1592 this weekend.
          Hide
          Konstantin Boudnik added a comment -

          Patch looks good. A couple of nits:

          • it might make sense to have a separate property for the location of ant-eclipse binary: it'd be easier to change over time
          • shall eclipse target depends on ivy-retrieve-system so system test framework related artifacts are also will be included?
          • should ant-eclipse-download be a top level target? IMO it shouldn't be called directly at all. Shall it be called -ant-eclipse-download instead and its description removed?
          • the patch replaces eclipse-files target which is covered by Eclipse environment document. It needs to be updated as well.
          • I have ran ant eclipse from the patch and didn't notice that anything had changed in my workspace. What is the desired effect of the execution?
          Show
          Konstantin Boudnik added a comment - Patch looks good. A couple of nits: it might make sense to have a separate property for the location of ant-eclipse binary: it'd be easier to change over time shall eclipse target depends on ivy-retrieve-system so system test framework related artifacts are also will be included? should ant-eclipse-download be a top level target? IMO it shouldn't be called directly at all. Shall it be called -ant-eclipse-download instead and its description removed? the patch replaces eclipse-files target which is covered by Eclipse environment document. It needs to be updated as well. I have ran ant eclipse from the patch and didn't notice that anything had changed in my workspace. What is the desired effect of the execution?
          Hide
          Nigel Daley added a comment -

          Tom, can you review?

          Show
          Nigel Daley added a comment - Tom, can you review?
          Hide
          Nigel Daley added a comment -

          Here's a patch for HDFS.

          Show
          Nigel Daley added a comment - Here's a patch for HDFS.

            People

            • Assignee:
              Nigel Daley
              Reporter:
              Tom White
            • Votes:
              3 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development