Bug 50610 - Add ant tasks for running POI against a workbook
Summary: Add ant tasks for running POI against a workbook
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: 3.8-dev
Hardware: All All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-18 12:12 UTC by Jon Svede
Modified: 2011-02-23 11:50 UTC (History)
0 users



Attachments
patch with the new classes for the Ant tasks (53.54 KB, text/plain)
2011-01-18 12:13 UTC, Jon Svede
Details
source files for the patch (8.71 KB, application/x-gzip)
2011-01-18 12:20 UTC, Jon Svede
Details
patch containing documentation and an example (19.42 KB, patch)
2011-01-20 15:58 UTC, Jon Svede
Details | Diff
updates to the example build (6.71 KB, patch)
2011-01-21 12:15 UTC, Jon Svede
Details | Diff
the spreadsheet and jar file dependencies. (39.26 KB, application/x-compressed)
2011-01-21 12:16 UTC, Jon Svede
Details
patch against https://github.com/jsvede/excelant/ (6.36 KB, patch)
2011-02-07 08:07 UTC, Yegor Kozlov
Details | Diff
new files to add to https://github.com/jsvede/excelant/ (832 bytes, application/gzip)
2011-02-07 08:08 UTC, Yegor Kozlov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jon Svede 2011-01-18 12:12:19 UTC
This ticket is for the ExcelAnt Ant tasks which allow users to write Ant tasks to test a workbook using POI.
Comment 1 Jon Svede 2011-01-18 12:13:24 UTC
Created attachment 26510 [details]
patch with the new classes for the Ant tasks

Adding the patch file, next I'll add the source files.
Comment 2 Jon Svede 2011-01-18 12:20:30 UTC
Created attachment 26511 [details]
source files for the patch

This is the tar.gz with the source files for the Ant tasks.

I am looking for feedback on the code as well suggestions/direction on what to do next with respect to:

 -  where to put example ant files and spreadsheets
 - tests for which classes to cover and to what depth
 - review of the util.ExcelAntWorkbookUtil and Factory classes

Any and all feedback gladly welcomed.

Thanks in advance!

Jon
Comment 3 Yegor Kozlov 2011-01-20 12:31:05 UTC
How did you test it? 

This seems a good patch, but it is not complete without samples and documentation. 

Can you upload some use-cases, I mean a build.xml and spreadsheets?

Regards,
Yegor
Comment 4 Jon Svede 2011-01-20 14:37:23 UTC
Yes, I agree it is far from complete.  I thought I'd follow the advice of "post patches early and often", so please let me know what works best.

Basically I've been testing it by running it.  There is only one class that has any POI related functions, the ExcelAntWorkbookUtil class, which manages all POI interactions.  The rest of it is all Ant related, for the most part.

I am happy to write a complete test suite if required.

I am working on docs right now, I can post what I have, I'll do that later today.

As part of the docs I was going to create a sample workbook and build file with targets, that should suffice but if not let me know what would work.  

As far as a build goes, what is required?  I can post a standalone build and then someone can advise me how best to integrate it?  Or is there some other approach that might work better?

Thanks,

Jon
Comment 5 Jon Svede 2011-01-20 15:58:35 UTC
Created attachment 26523 [details]
patch containing documentation and an example

Docs and examples patch.  Still needs work though.
Comment 6 Jon Svede 2011-01-21 12:15:13 UTC
Created attachment 26530 [details]
updates to the example build

This patch contains better examples, ones that actually work.  Attaching the tar.gz with the binaries next.
Comment 7 Jon Svede 2011-01-21 12:16:51 UTC
Created attachment 26531 [details]
the spreadsheet and jar file dependencies.

This file contains the dependencies for the example file.  You should be able to unzip this into the root of the project and then go to the excelant examples directory and run the two sample tests.
Comment 8 Jon Svede 2011-01-21 12:20:25 UTC
The last 2 attachments should be used for the examples.

I am looking for advice on the following:

 - tests - how much and deep should the tests be?
 - docs - are these complete enough or do we need more?
 - build - I don't have any build integration yet.  It was originally suggested that these tasks should be in their own jar.  I'll point out that they require some of the POI jars to work which begs the question, does it make sense to separate them?

Any other comments, suggestions or ideas are welcome.

Thanks,

Jon
Comment 9 Nick Burch 2011-01-29 14:14:53 UTC
(In reply to comment #8)
>  - build - I don't have any build integration yet.  It was originally suggested
> that these tasks should be in their own jar.  I'll point out that they require
> some of the POI jars to work which begs the question, does it make sense to
> separate them?

The scratchpad jar needs the core one, and the ooxml jar needs core+scratchpad, so I don't think saying "the ant jar needs core" is a big step. My worry about putting the ant task into an existing jar is that it needs the ant jars to work, and I'd rather not require them for people not interested in using the ant functionality.

So, I'd say the source should live at the same level in the tree as scratchpad and ooxml, and be compiled to its own jar in the same way. Does that make sense?
Comment 10 Jon Svede 2011-01-29 14:51:26 UTC
(In reply to comment #9)

> So, I'd say the source should live at the same level in the tree as scratchpad
> and ooxml, and be compiled to its own jar in the same way. Does that make
> sense?

Yep, that makes sense.  Let me take a shot at that integration and post another patch.  I'll also write a few tests and include them as well.

Jon
Comment 11 Yegor Kozlov 2011-02-07 08:05:46 UTC
I checked out the latest sources from https://github.com/jsvede/excelant/. 

Firstly, I see two packages with duplicate contents: org.excelant.* and org.apache.poi.ss.excelant.*. Was org.excelant.* added by mistake? 

Secondly, I tweaked build.xml and added a target to run junit tests. See the attached diff. This configuration requires junit-4.7.jar in the lib directory.

The tests need to be converted to junit 3.8 because that's what POI uses
(changing junit versions is probably something we'd decide to do project wide). This means no annotations. Sorry. 

 The test classes should be renamed to follow POI conventions: All tests should start with Test* and extend TestCase. Also, POI expects tests to be silent (not write to std-out or any log) when successful. 

Thirdly, Ant provides several ways of registering custom tasks and itt would be good if ExcelAnt supports all of them:

 (1) explicit definition of taskdefs in build.xml (supported)

 (2) reference to a configuration file bundled in the jar

  <typedef resource="org/apache/poi/ss/excelant/antlib.xml"
                   classpath="YOUR-PATH-TO/poi-exelant.jar"/>


Where the antlib file is defined as follows:

<antlib>

  <taskdef name="excelant" classname="org.excelant.ExcelAntTask" />
  <taskdef name="test" classname="org.excelant.ExcelAntTest" />
  <taskdef name="setDouble" classname="org.excelant.ExcelAntSetDoubleCell" />
  <taskdef name="evaluate" classname="org.excelant.ExcelAntEvaluateCell" />

</antlib>


3. Similar to (2), but assigning a namespace URI:

  <typedef uri="poi:org.apache.poi.ss.excelant"
           resource="org/apache/poi/ss/excelant/antlib.xml"
                   classpath="YOUR-PATH-TO/poi-exelant.jar"/>

4.Using Ant's autodiscovery. Run Ant as

  ant -lib DIR_CONTAINING_EXCELANT_JAR targetName

 or copy the excelant jar into ANT_HOME/lib - and then in your build file, simply declare the namespace on the project tag:

                <project xmlns:poi="antlib:org.apache.poi.ss.excelant">
              
 And all tasks of this library will automatically be available in the poi namespace without any typedef.


I added support for (2)-(3), see my patch. (4) needs to be tested. 

Regards,
Yegor
Comment 12 Yegor Kozlov 2011-02-07 08:07:36 UTC
Created attachment 26616 [details]
patch against https://github.com/jsvede/excelant/
Comment 13 Yegor Kozlov 2011-02-07 08:08:12 UTC
Created attachment 26617 [details]
new files to add to https://github.com/jsvede/excelant/
Comment 14 Jon Svede 2011-02-07 12:54:43 UTC
(In reply to comment #11)
> 
> Firstly, I see two packages with duplicate contents: org.excelant.* and
> org.apache.poi.ss.excelant.*. Was org.excelant.* added by mistake? 


No, it was that I had checked the code in org.excelant but then later moved everything to the POI suggested packaging.  I didn't delete the old code until I was sure I didn't need it.  I thought had deleted the org.excelant code but looking in the master on github, that delete didn't make it up.  I will fix that, the correct codeline is the org.apache.* packages.
 
> Secondly, I tweaked build.xml and added a target to run junit tests. See the
> attached diff. This configuration requires junit-4.7.jar in the lib directory.
> 
> The tests need to be converted to junit 3.8 because that's what POI uses
> (changing junit versions is probably something we'd decide to do project wide).
> This means no annotations. Sorry. 

Not a problem, I can refactor the existing tests.
 
>  The test classes should be renamed to follow POI conventions: All tests should
> start with Test* and extend TestCase. Also, POI expects tests to be silent (not
> write to std-out or any log) when successful. 

I can refactor accordingly, not a problem.

> 
> Thirdly, Ant provides several ways of registering custom tasks and itt would be
> good if ExcelAnt supports all of them:
> 
>  (1) explicit definition of taskdefs in build.xml (supported)
> 
>  (2) reference to a configuration file bundled in the jar
> 
>   <typedef resource="org/apache/poi/ss/excelant/antlib.xml"
>                    classpath="YOUR-PATH-TO/poi-exelant.jar"/>
> 
> 
> Where the antlib file is defined as follows:
> 
> <antlib>
> 
>   <taskdef name="excelant" classname="org.excelant.ExcelAntTask" />
>   <taskdef name="test" classname="org.excelant.ExcelAntTest" />
>   <taskdef name="setDouble" classname="org.excelant.ExcelAntSetDoubleCell" />
>   <taskdef name="evaluate" classname="org.excelant.ExcelAntEvaluateCell" />
> 
> </antlib>
> 
> 
> 3. Similar to (2), but assigning a namespace URI:
> 
>   <typedef uri="poi:org.apache.poi.ss.excelant"
>            resource="org/apache/poi/ss/excelant/antlib.xml"
>                    classpath="YOUR-PATH-TO/poi-exelant.jar"/>
> 
> 4.Using Ant's autodiscovery. Run Ant as
> 
>   ant -lib DIR_CONTAINING_EXCELANT_JAR targetName
> 
>  or copy the excelant jar into ANT_HOME/lib - and then in your build file,
> simply declare the namespace on the project tag:
> 
>                 <project xmlns:poi="antlib:org.apache.poi.ss.excelant">
> 
>  And all tasks of this library will automatically be available in the poi
> namespace without any typedef.
> 
> 
> I added support for (2)-(3), see my patch. (4) needs to be tested. 

Yes, I had wanted to add support for #4, I wasn't aware of the methodology you've shown in #2 and #3.

My goal was to get it all working in the github repository and once we had it all worked out, migrate the code the POI structure and update the build file.   

I'll take a look at your patch to github repository and play around with it.

Thanks for reviewing it all providing feedback!

Sincerely,

Jon
Comment 15 Jon Svede 2011-02-18 23:00:38 UTC
I tried doing #2 and #4 and neither seemed to work.  I think the issue is that the ExcelAnt jar file would need to have it's dependencies fully satisfied to use either approach.  

I tried just using the excelant-****.jar but I get errors complaining about the dependencies; in both cases the dependency is spelled out in the path element in the build.xml I am using to test with.  

I am going to try to package the all the dependencies and test again.

Jon
Comment 16 Jon Svede 2011-02-19 00:04:40 UTC
Adding all the POI dependencies into the ExcelAnt jar file makes #2 start working but eventually it fails.   This has to do with the classpath.  In the original way I set this up, I assumed I could define all the dependencies in one path element and that doing so would allow the classloader to resolve all the dependencies.

For option #2 I was able to define the dependencies in the <typedef> tag and get it to resolve but because the ExcelAnt task doesn't have the ability to accept classpath elements and it needs to.   I will have to read up on how to enable this - unless this is a default feature somehow.

Jon
Comment 17 Yegor Kozlov 2011-02-19 01:14:43 UTC
Oops. There was an error in build.xml and antlib.xml was not included in the jar. That was the reason why neither of (2)-(4) did not work. 

Lines 47-50 should be 

      <copy todir="${build.dir}/src">
          <fileset dir="${resource.dir}"/>
      </copy>

instead of

      <copy todir="${build.dir}">
          <fileset dir="${resource.dir}"/>
      </copy>

I created a branch in svn (https://svn.apache.org/repos/asf/poi/branches/excelant/) and ported the code from GitHub into it. Please check out, I would like further work to be done in POI svn, not in GitHub. Actually, we are nearly ready to merge this branch with trunk.

ExcelAnt is included in the build cycle. The new build artifact is named poi-excelant. There are compile-excelant and test-excelant targets to compile / test only that module or you can do "ant compile" and "and test" to build everything. The "ant build" target builds distribution packages and poi-excelant is included. Please review, I'm going to include ExcelAnt in 3.8-beta which we plan in early March.

Note that I added a new junuit test, see https://svn.apache.org/repos/asf/poi/branches/excelant/src/excelant/testcases/org/apache/poi/ss/excelant/TestBuildFile.java

"pure java" junit tests are OK, but they are too low-level. It would be good to test against a real build file to ensure that the code works from Ant perspective.

The test extends BuildFileTest.java. Most of Ant's internal tests are based on this class, I just followed the code patterns when I wrote my TestBuildFile.java

Unfortunately BuildFileTest.java is in the test area and not included in the ant.jar, I had copy it from Ant's sources. 

Regards,
Yegor
Comment 18 Jon Svede 2011-02-21 23:04:14 UTC
I am fine to take down the GitHub repo and work out of SVN, thank you for creating that branch as well as adding the ant targets for building it.

I am worried about the inclusion of ExcelAnt in the beta only because of the classpath issue with the tasks. 

Right now the tasks work because when you set up your path in the build file you add all the dependencies for POI and any other custom classes, for instance the the FreeRefFunction implementations you might contribute.  In the case where you put the excelant jar in the Ant lib directory, you lose that benefit.

To fix that, the top level excelant class needs to be able to append entries to the classpath.  I haven't had time to read up on how to accomplish this.

Jon
Comment 19 Nick Burch 2011-02-22 06:19:19 UTC
(In reply to comment #18)
> To fix that, the top level excelant class needs to be able to append entries to
> the classpath.  I haven't had time to read up on how to accomplish this.

If the classpath is a path reference, then that's really easy to do. Get the path reference object, call createPathElement on that, then call setLocation on the path element with the file object pointing to your new jar / class
Comment 20 Yegor Kozlov 2011-02-23 11:43:25 UTC
I merged excelant into trunk in r1073707. 

> 
> I am worried about the inclusion of ExcelAnt in the beta only because of the
> classpath issue with the tasks. 
> 

No worries. What we already have is very, very cool. Support for the nested classpath element would be great, but even without it ExcelAnt is functional and it is certainly OK to include it in the beta. 

> Right now the tasks work because when you set up your path in the build file
> you add all the dependencies for POI and any other custom classes, for instance
> the the FreeRefFunction implementations you might contribute.  In the case
> where you put the excelant jar in the Ant lib directory, you lose that benefit.
> 
> To fix that, the top level excelant class needs to be able to append entries to
> the classpath.  I haven't had time to read up on how to accomplish this.
> 

I plan to spend some time to implement support for the classpath element. Hope to get it done by April 1, the scheduled date of 3.8-final.

Regards,
Yegor
Comment 21 Yegor Kozlov 2011-02-23 11:50:13 UTC
I think we are done with this patch. ExcelAnt is in trunk and and further improvements will be submitted in new tickets. 

Yegor