ZooKeeper
  1. ZooKeeper
  2. ZOOKEEPER-640

make build.xml more configurable to ease packaging for linux distros

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.2.1, 3.2.2
    • Fix Version/s: 3.3.0
    • Component/s: build
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Hi,

      I started packaging Zookeeper for Debian[1][2]. Thereby I had a problem excluding contrib/rest from the build without patching the upstream tarball. Could you please add some properties to your build.xml that allow me to (de)select contribs? In the example below I can easily override the properties:

      <project name="zookeepercontrib">

      <property name="contribfilesetincludes" value="*/build.xml" />
      <property name="contribfilesetexcludes" value="" />

      <fileset id="contribfileset"
      dir="."
      includes="$

      {contribfilesetincludes}

      "
      excludes="$

      {contribfilesetexcludes}

      "
      />

      <target name="compile">
      <subant target="jar">
      <fileset refid="contribfileset" />
      </subant>
      </target>

      Could you please also add a line to project.classpath:

      <path id="project.classpath">
      <fileset dir="$

      {additional.lib.dir}

      " includes="*.jar"/>

      For Debian I may not compile based on the jar files in lib but must use the jars already in Debian.

      [1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=561947
      [2] http://git.debian.org/?p=pkg-java/zookeeper.git

      Thank you!

        Activity

        Hide
        Patrick Hunt added a comment -

        Great to see this being packaged for Debian! Shouldn't be a problem. A few questions though:

        1) contrib include/exclude seems fine to me, good idea

        2) for the "additional.lib.dir" :

        a) you want this changed in build.xml at the toplevel and also for the contribs, right? Same variable name
        used in both ok?

        b) do you really want a dir or a path string instead, ie additional.lib.dir or additional.libs.path (where libs.path is a list of directories/jars)

        libs.path would give you more control (you would specify/order the jars explicitly vs just *.jar") This might be important
        for example if you have multiple jars with same capability but diff version... it would require you to specify the
        list of jars though.

        Show
        Patrick Hunt added a comment - Great to see this being packaged for Debian! Shouldn't be a problem. A few questions though: 1) contrib include/exclude seems fine to me, good idea 2) for the "additional.lib.dir" : a) you want this changed in build.xml at the toplevel and also for the contribs, right? Same variable name used in both ok? b) do you really want a dir or a path string instead, ie additional.lib.dir or additional.libs.path (where libs.path is a list of directories/jars) libs.path would give you more control (you would specify/order the jars explicitly vs just *.jar") This might be important for example if you have multiple jars with same capability but diff version... it would require you to specify the list of jars though.
        Hide
        Patrick Hunt added a comment -

        Thomas, checkout this patch and let me know.

        It was a bit tricky with additional.libs since ant doesn't support conditionals, but I got around
        it with a fairly clean hack. All you need to do is specify additional.lib.dir and turn off (set to "")
        the excludes for that same fileset.

        I tried various scenarios and it seemed ok for me. Give it a try, work for you? If so I'll
        work on getting it committed to 3.3.0

        Show
        Patrick Hunt added a comment - Thomas, checkout this patch and let me know. It was a bit tricky with additional.libs since ant doesn't support conditionals, but I got around it with a fairly clean hack. All you need to do is specify additional.lib.dir and turn off (set to "") the excludes for that same fileset. I tried various scenarios and it seemed ok for me. Give it a try, work for you? If so I'll work on getting it committed to 3.3.0
        Hide
        Patrick Hunt added a comment -

        Thomas, this good for you?

        Show
        Patrick Hunt added a comment - Thomas, this good for you?
        Hide
        Hadoop QA added a comment -

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

        +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 tests are needed for 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 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: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/38/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/38/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/38/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/12429777/ZOOKEEPER-640.patch against trunk revision 899383. +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 tests are needed for 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 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: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/38/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/38/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/38/console This message is automatically generated.
        Hide
        Mahadev konar added a comment -

        thomas, can you review the changes?

        Show
        Mahadev konar added a comment - thomas, can you review the changes?
        Hide
        Thomas Koch added a comment -

        Sorry for not replying earlier. I've finished packaging zookeeper now and reused only a part of your buildsystem. If everything works out, zookeeper should be in Debian unstable in a few days. I need to go on with hadoop now.
        I'm working on a document for java developers how they can help their downstream packagers:

        http://wiki.debian.org/Java/UpstreamHints

        Please be so kind to have a look there from time to time or even help me expanding it. You'll see, that I'm still seeking for best practices on how to advise upstream developers to ease Debian with packaging.

        Show
        Thomas Koch added a comment - Sorry for not replying earlier. I've finished packaging zookeeper now and reused only a part of your buildsystem. If everything works out, zookeeper should be in Debian unstable in a few days. I need to go on with hadoop now. I'm working on a document for java developers how they can help their downstream packagers: http://wiki.debian.org/Java/UpstreamHints Please be so kind to have a look there from time to time or even help me expanding it. You'll see, that I'm still seeking for best practices on how to advise upstream developers to ease Debian with packaging.
        Hide
        Patrick Hunt added a comment -

        @Thomas I subscribed to the page you listed so hopefully I'll be notified of any changes in future.

        the "no network no download" may be a problem. In 3.3.0 we've upgraded to ivy (already used by
        much of the rest of the hadoop projects) and as part of that change ivy will d/l any/all necessary
        dependencies during the build. I guess we'll have to cross that bridge at some point.

        so you are ok with the changes listed in this patch then? It addressed your current concern/problems?

        Show
        Patrick Hunt added a comment - @Thomas I subscribed to the page you listed so hopefully I'll be notified of any changes in future. the "no network no download" may be a problem. In 3.3.0 we've upgraded to ivy (already used by much of the rest of the hadoop projects) and as part of that change ivy will d/l any/all necessary dependencies during the build. I guess we'll have to cross that bridge at some point. so you are ok with the changes listed in this patch then? It addressed your current concern/problems?
        Hide
        Patrick Hunt added a comment -

        No update from Thomas - committer please review/commit this asap.

        Show
        Patrick Hunt added a comment - No update from Thomas - committer please review/commit this asap.
        Hide
        Thomas Koch added a comment -

        Thanks for your afford. I'm sorry, that I didn't answer earlier. In the end, I've not used the build.xml at all but scripted the build in the debian/rules file. Please chancel this issue.

        Show
        Thomas Koch added a comment - Thanks for your afford. I'm sorry, that I didn't answer earlier. In the end, I've not used the build.xml at all but scripted the build in the debian/rules file. Please chancel this issue.
        Hide
        Patrick Hunt added a comment -

        No worries. Seems like a fine idea anyway, commiter plz review/commit asap.

        Show
        Patrick Hunt added a comment - No worries. Seems like a fine idea anyway, commiter plz review/commit asap.
        Hide
        Mahadev konar added a comment -

        +1 the patch looks good. It seems useful enough to be in the build...

        Show
        Mahadev konar added a comment - +1 the patch looks good. It seems useful enough to be in the build...
        Hide
        Mahadev konar added a comment -

        I just committed this. thanks Pat!

        Show
        Mahadev konar added a comment - I just committed this. thanks Pat!
        Hide
        Hudson added a comment -

        Integrated in ZooKeeper-trunk #715 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/715/)
        . make build.xml more configurable to ease packaging for linux distros (phunt via mahadev)

        Show
        Hudson added a comment - Integrated in ZooKeeper-trunk #715 (See http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/715/ ) . make build.xml more configurable to ease packaging for linux distros (phunt via mahadev)

          People

          • Assignee:
            Patrick Hunt
            Reporter:
            Thomas Koch
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

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

                Development