From nhat.min...@gmail.com on May 17, 2011 02:47:49

Hi, I think there are two issues here.

The first one, that you have noticed, is that Cofoja contracts get compiled when the annotation processor is running, that is, actually before the source files get compiled, which may cause headaches since it will report in a cryptic way errors that may otherwise be caught during the normal build. On large projects, it is probably a good idea to compile contracts in a separate step (proc:only) after the normal build.

That being said, it's not the reason why you are getting that error. Usually when you get an "error in contract" without any indication, it's an internal error, so it's our fault. :) Here, it seems the problem is createApproximateCommandLineArgumentString(java.lang.Object[]) is actually a variadic method, which is not translated correctly at the moment (I'm looking into it).

Status: Accepted

From nhat.min...@gmail.com on May 17, 2011 04:18:35

About the varargs problem, I've submitted a patch for review; you can try out my latest non-official build of my branch, dated from today (2011-05-17), see if it at least solves some of the cryptic errors. http://www.huoc.org/~minh/cofoja/ Or you can look at the patch yourself (see attached).

Attachment: 0002-Add-variadic-method-support.patch

From nhat.min...@gmail.com on May 17, 2011 07:32:13

Varargs issue should be fixed in r120 .

From chat...@google.com on May 17, 2011 07:32:35

Varargs problem fixed on r120 .
The Downloads page has been updated with the latest build. Please give it a try.

About ant, I'll provide an example today, but the basics are: add cofoja.jar to the classpath when invoking javac (it should find the annotation processor alone), and add the javaagent:cofoja option as a jvmarg.

Simply adding cofoja.jar to the classpath should make the annotation processor work. If you want to have a target that does NOT compile contracts, add to javac.

From depri...@broadinstitute.org on May 17, 2011 13:10:57

Just tried r120 , and can confirm a few things. First, it changed the error messages, but now they aren't informative at all:

gatk.compile:
[javac] Compiling 810 source files to /Users/depristo/Desktop/broadLocal/GATK_CoFoJa/trunk/build/java/classes
[javac] Note: error in contract: Note: Some input files use or override a deprecated API.
[javac] Note: error in contract: Note: Recompile with -Xlint:deprecation for details.
[javac] Note: error in contract: Note: Some input files use unchecked or unsafe operations.
[javac] Note: error in contract: Note: Recompile with -Xlint:unchecked for details.

Also, I can get cofoja working on a smaller, and less complex, set of java files used by the GATK. So locally, with the same ant configuration and build.xml I can get cofoja working on one project but not another.

I decided to revert to a clean version of our repository, modify the build.xml to try cofoja contacts, and created a tgz file for you to reproduce the problem. You can get the archive from http://www.broadinstitute.org/~depristo/GATK_CoFoJa.tgz After uncompressing, ant clean; ant in the trunk and you can see the build errors. We require Java 1.6 and ant 1.7 or later.

In the trunk, even after the build failure, the sublibrary will have built correctly and you can verify that cofoja worked there by running:
% java -Xmx1g -javaagent:lib/cofoja.jar -Dcom.google.java.contract.log.contract=true -jar tribble/dist/CountRecords.jar sites.vcfCreating the index and memory, then writing to disk for index file -> /Users/depristo/Desktop/broadLocal/GATK_CoFoJa/trunk/sites.vcf.idx
[com.google.java.contract:contract checking contract: org.broad.tribble.vcf.VCFCodec.com$google$java$contract$PH$org$broad$tribble$vcf$VCFCodec$readHeader: {"reader != null"}]
0 [main] DEBUG org.broad.tribble.index.DynamicIndexCreator - feature count == 0, number of bases seen == 0
1 [main] DEBUG org.broad.tribble.index.DynamicIndexCreator - density == NaN longest feature == 0
1 [main] DEBUG org.broad.tribble.index.DynamicIndexCreator - score for Interval == 75.0
1 [main] DEBUG org.broad.tribble.index.DynamicIndexCreator - score for Linear == NaN
2 [main] DEBUG org.broad.tribble.index.DynamicIndexCreator - creating index of type IntervalIndexCreator with bin size 75
[com.google.java.contract:contract checking contract: org.broad.tribble.vcf.VCFCodec.com$google$java$contract$PH$org$broad$tribble$vcf$VCFCodec$readHeader: {"reader != null"}]
We saw 0 record in file sites.vcf

Thanks again for creating this very useful library. We are looking forward to using it in our system.

From nhat.min...@gmail.com on May 17, 2011 17:06:04

The deprecation warnings shouldn't make your build fail, even though Cofoja spits "error in contract", it also does that for warnings; we should change that, I'll admit it's pretty confusing. Normally, they can be safely ignored and the build should complete anyway. Are you sure this actually makes your build fail and not something else?

From depri...@broadinstitute.org on May 17, 2011 17:38:02

I should have been more clear. The clean version of our repo causes several javac errors when contracts are enabled. These occur in files with outer-level, non-public (package only) classes in public classes. For example, BAMSchedule.java contains public class BAMSchedule {} and class BAMScheduleEntry {} in the java file. It causes some kind of compilation problem, when contracts are used in any file in the repo (GenomeAnalysisEngine.java for example). All of the errors should reproduce with the GATK source trunk with ant.

From nhat.min...@gmail.com on May 17, 2011 17:38:25

I've submitted a patch for review which should remove the spurious warnings (Cofoja is not supposed to relay warnings from the underlying compiler at all, in the first place, which explains the "error in contract" instead of "warning in contract").

Could you try out my latest build? http://www.huoc.org/~minh/cofoja/ccofoja-1.0-20110518.jar There may be other errors, though, but this should give us a better the picture.

From nhat.min...@gmail.com on May 17, 2011 17:41:08

Hum, seems more complicated than I thought. I'll investigate this later, when I have more time, then.

From depri...@broadinstitute.org on May 17, 2011 17:41:30

Perhaps we overlapped on our post. There are multiple errors when compiling the GATK, which you can easily recreate in the provided source tree. There's cojojaDir property that you can set to your local build, and cycle through the bugs quickly. Might be more efficient than me downloading your jar, but am happy to if you think that's best.

From chat...@google.com on May 18, 2011 03:27:52

Quick update:
We generate the contracts correctly, the problem is compiling them. BAMSchedulerEntry and JEXLMap are package-private classes and for some reason they are not found.
If these two classes are made public it works correctly (don't worry, I'm NOT asking you to do this :)).

From depri...@broadinstitute.org on May 18, 2011 04:26:26

When I say I reverted to our original repository, it was to restore the package-private classes, as I had already created public versions of those files to make the system compile. Even with that change, the contracts do not run correctly, unfortunately.

Also, I forgot to revert one file that caused another compilation error (here's the diff)

--- java/src/org/broadinstitute/sting/gatk/walkers/Reference.java ( revision 5813 )
+++ java/src/org/broadinstitute/sting/gatk/walkers/Reference.java (working copy)
@@ -43,5 +43,5 @@
* Specifies the window expansion for the current walker.
* @return The window to which the reference should be expanded. Defaults to [0,0](no expansion).
*/

• public Window window() default @window;
• public Window window();
  }

That default @window causes the build to fail with the annotation processor.

From nhat.min...@gmail.com on May 18, 2011 05:16:24

OK, so let's first fix that build problem, then we can see how come contracts are not running. I've had the time to look at your archive file and here's my explanation of the problem, followed by a few suggestions/solutions.

Problem: The problem is that Cofoja needs to compile the contracts in the context where they appear, which means it needs to be able to pull all dependencies your normal class would. Having said that, Cofoja will only compile contracts for these classes that actually have contracts (either directly declared on them or inherited), which makes sense and allows you to let classes that have constructs Cofoja doesn't understand (though we try hard to support all Java constructs) compile side-by-side with classes that have contracts. It also saves time when your project is only sparsely contracted. Now, the problem with compiling only a few select files is that some dependencies may be missing; Cofoja uses the system Java compiler internally so it has the exact same resolution algorithm: if it fails to find a class in the current compilation batch, it will look for classes by name in the current class and source paths. Hence, it fails in the same way to find classes that do not live in a file of their own (but you'd have the same problem if you tried to compile only part of your files without contracts).

Solution 1: Simple solution for new code. If you put one class per file, named after the class, everything should just work fine. But that requires refactoring on your part and may not be in line with your coding standard, which is fine.

Solution 2: Split your build according to dependencies; that is, here you should compile the files with internal classes separately, then build the rest. Has better build time than solution 3 but does not scale well since it's not the Ant model (it would work with a make-like build system though, that orders all artefacts by dependency).

Solution 3: I recommend this. Do contract generation in a post-processing phase. It's also what we do at Google (unless it has changed since I left). Also, with this solution you'll get better error reporting as I mentioned in an earlier post, because contracts will be compiled after your code, which means any error in your classes will be reported before Cofoja runs and you won't see them appear as cryptic "error in contract" that actually have nothing to do with contracts but stem from the other parts of the code. Downside is the setup is more complex, so you'll want to make a macro for this.

To do a separate contract compilation, starting from a single rule:

1. Duplicate your rule. Make sure the output directories are different for each new instance of the rule, otherwise Ant won't let you compile "twice".
2. Add the compiler option -proc:none to the first instance.
3. Add -proc:only to the second instance.
4. Add the class output directory of your first invocation to the class path of your second instance so it can find its dependencies from your own project. This is the important step.

See the attached file for an example modification of your build.xml that does just that. You'll have to modify your packaging rules to add the contracts files to your JARs if you wish, though. Note that you should be able to (though I've never done so myself) package the contracts in their own JAR also if you want; just make sure it's in the class path so the agent can find them.

Attachment: build.xml

From depri...@broadinstitute.org on May 18, 2011 12:14:27

Thank you very much. Things are working now, and I'm adding contracts to a few of our core classes for testing. I'll let you know if we encounter any more issues. I would recommend adding the pre and post ant example to your wiki, as this seems to have been key to getting this work for our codebase.

From nhat.min...@gmail.com on May 18, 2011 13:26:16

Great! Thanks for helping us test Cofoja on a real project of some significant size. We realize that the difficult setup does not help and is a big barrier for new users. And we will try to package it better for an eventual release. At the moment, the main development effort, driven by Leo, is geared towards integration within Google, and unfortunately cannot benefit our open source users directly. Besides, we are still learning with new scenarios and build cases, since actually the internal system at Google has a very rigid and singleminded way of doing things, so we did not encounter all these different configurations before. So all in all, it helps us a lot to have feedback from real world users on projects that use different build strategies and have different code patterns!

I'll leave this issue open till I get around to fixing the documentation.

Status: Started

From chat...@google.com on May 19, 2011 04:26:16

I added a quick example of an ant buildfile to the wiki. You can check it here: https://code.google.com/p/cofoja/wiki/AntCofoja Please report any error or suggestion :)

From nhat.min...@gmail.com on May 20, 2011 09:10:19

LGTM, I'm closing this ticket then, since we have some up-to-date docs now. We can probably make it better, but it'll do for now.

Status: Done