Maybe I'm misreading it, but the code and regex appear to indicate the left-hand-side is classnames?

Does this fp get closed? maybe use with?

Is it safe to assume that all resource paths returned here will always be contained within the artifact_root?

Tests?

Just a little terminology clarification.  I think you mean "annotation processors" here instead of "APTs"
APT stands for Annotation Processing Tool, which is a standalone task to run Annotation Processors.  'apt' as a standalone tool is obsolete as of Java 7 and javac can just run annotation processors by itself.
http://docs.oracle.com/javase/7/docs/technotes/guides/apt/
This protocol is only implemented to my knowledge by com.twitter.tools.args.apt.CmdLineProcessor and it isn't enabled by default in OSS pants. You might want to mention that here.
https://github.com/twitter/commons/blob/71a06f9c15f0bd2db67a63b32440278543864f18/src/java/com/twitter/common/args/apt/CmdLineProcessor.java

the approach seems plausible to me, will wait for tests to do a 2nd pass.

early exit here would clean this up instead of the whole logic being in an if condition

add a check here for if line contains a space

lets log this error - its unused right now.

This is a pretty monsterous loop.  It should be broken up a bit if at all possible.

This whole parsing loop seems like it should be broken out into a helper class that just takes the full string contents of the file and does the parsing in the constructor or a factory classmethod.

Ditto below--shouldn't this just hard fail?

Isn't this an error condition?

What is this for exactly? This duplicates information in jmake's pdb file, no?

Seems like a bad idea to continue? I prefer a hard crash on such errors, since we expect them to never happen.

General comment on the design: I don't think write_to_artifact_cache is the right place for this. For example, it doesn't address bundling these files when creaing jars.
We already have a general mechanism for this sort of thing, i.e., the 'products' mechanism. In fact we already propagate target->resources mappings under the 'resources_by_target' product (e.g., so they can get bundled into relevant jars).
It would uniform to simply add these extra mappings into that product, and ensure that the artifact cache picks them up from there when writing.
This way you get the right bundling behavior for free.

nit: jvm_compile.py is getting pretty big.    It seems like ResourceMapping could be a standalone module.

it would be nice to add a unit test that could exercise these conditions.

I think this is irrelevant for testing resource mapping.

I think this is really a test project for the resource mapping feature.

This looks like you copied examples/src/java/pants/examples/annotation/processor/ExampleProcessor.java and added a few lines of code to it.

1) Could you rename it? ResourceMappingProcessor would be a better name.
2) Could you rip out the stuff from ExampleProcessor that is irrelevant?
3) Could you update the javadoc to explain what this processor is used for?

If this Task requires resources_by_target then you should add this as a requirement in prepare(..) as round_manager.require_date('resources_by_target')
and then you wont need/shoudnt create it here. In fact, a creation here actually indicates that this Task returns a resources_by_target (which should in turn be reflected in product_types(..)) which is not your intention afaict.

I may be missing the point of this change, so I'm not sure if my comments make sense.  Is the idea to create dependencies on resources without actually declaring them as resources in BUILD files?  How are these resources created?

This project doesn't really test annotations in general, it is testing your resource mapping logic, which could be created in other ways than using an annotation processor.  How about putting it in
testprojects/src/java/com/pants/testproject/resourcemapping/...

I don't think you want to use this, you want to make up a new annotation, like @Resource("com/pants/testproject/resourcemapping/foo.txt")

I'd suggest creating a static resource and adding a static dependency here.

I think you could simplify this code some more.
I don't know what the internal Twitter code looks like, but I doubt it is the annotation processor that is creating the file.  I'll bet that Twitter has an internal annotation that takes a resource path as an argument.  I'm thinking that is the file you should be referencing in your mapping, not a file created by the annotation processor.

Don't use 'Deprecated' here, create a new annotation that takes a filename argument.

You should be able to get the filename from the annotations passed.  If you don't want to do this, then justify it and add a big comment explaining what's going on.

The last version of this had a bunch of code that looked over the annotated classes and did stuff with them, but you thought that was too complicated.  So I ripped it all out and replaced it with something trivial.  The point of this Java code isn't to be useful in any way; it's to demonstrate that if an annotation processor creates a resource resource and a resource mapping, then pants can process it.  So it doesn't really matter what this code does or how it does it; the same bits of pants are going to be tested.

From this perspective, re-using Deprecated is not crazy; I could imagine having an annotation processor that creates a "deprecation report" (e.g. so that you could interrogate your JAR files to see which of them are using deprecated APIs).

Ship It!

Ship It!