jbosstools / m2e-apt

Maven integration with Eclipse JDT Annotation Processor Toolkit
39 stars 19 forks source link

Do not load the complete project classpath in the Factory path #13

Open fbricon opened 12 years ago

fbricon commented 12 years ago

We should explore a way to only load relevant jars to the JDT APT Factory Path. See if adding only the processor dependencies is enough to get JDT APT working.

VsevolodGolovanov commented 9 years ago

Our .factorypath files look like this:

<factorypath>
    <factorypathentry kind="VARJAR" id="M2_REPO/org/hibernate/hibernate-jpamodelgen/4.3.7.Final/hibernate-jpamodelgen-4.3.7.Final.jar" enabled="true" runInBatchMode="false"/>
    <factorypathentry kind="PLUGIN" id="org.eclipse.jst.ws.annotations.core" enabled="false" runInBatchMode="false"/>
</factorypath>

And JDT APT works. (As to why our factorypaths look like this, see comment in issue #28).

JoshIsOnIt commented 8 years ago

Hi Fred,

We would really like to see this feature, especially with the new functionality in the compiler plugin (3.5) to specify annotation paths. When the entire class path is loaded into the factoryPath, we end up getting some strange build errors, and we have to end up pairing it down manually, which is not ideal (in fact it is quite a pain, because everytime we update the maven project, we have to do it).

Is this something that is difficult to do?

Thanks for all your work on this plugin!

fbricon commented 8 years ago

I remember having to add the project dependencies to the factory path because in some occasions, annotation processing would fail with CNFE exceptions.

Can you test the last build from http://download.jboss.org/jbosstools/builds/staging/m2e-apt/all/repo/ with annotation paths? In that case project dependencies are not added to the factory path. It's not consistent with the current behavior, because I wanted to see if we'd get into trouble.

JoshIsOnIt commented 8 years ago

So I tested the latest build. It definitely tightened up the list in the factory path, but it still brought in a bunch of dependencies. Again, I would expect if I list two jars in the annotationProcessorPaths tag, that they would be the only entries in the factory path. Does that make sense?

fbricon commented 8 years ago

Apparently, in some cases, APT won't work if the dependencies are missing, ask @agudian

agudian commented 8 years ago

If I remember correctly, hibernate-jpamodelgen declares some transitive dependencies. I'm on my phone now, but I can check that tomorrow in more detail.

If you ask me, implementors of annotation processors should provide only fat-jars, with any additional dependencies shaded and included in the jar. But it's not what we see in some cases ;-).

Btw, my own testing on the annotationProcessorPath in maven-compiler-plugin 3.5 wasn't thorough enough and only after the release I found that I also add the project's dependencies to the processor path - which is a bug there and will be fixed soonish. Just so you know that the current behavior in the maven-build isn't the right frame of reference when comparing to m2e-apt (which I think is correct in the snapshot-repo).

agudian commented 8 years ago

I checked it again: hibernate-jpamodelgen 4.3.7.Final comes with dependencies to jboss-logging and jboss-logging-annotations. So those three should be in the .factorypath. And they are, and it works fine - just tried it out with a very simple example:

    <build>
        <plugins>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-compiler-plugin</artifactId>
                <version>3.5</version>
                <configuration>
                    <source>1.8</source>
                    <target>1.8</target>
                    <annotationProcessorPaths>
                        <annotationProcessorPath>
                            <groupId>org.hibernate</groupId>
                            <artifactId>hibernate-jpamodelgen</artifactId>
                            <version>4.3.7.Final</version>
                        </annotationProcessorPath>
                    </annotationProcessorPaths>
                </configuration>
            </plugin>
        </plugins>
    </build>
    <dependencies>
        <dependency>
            <groupId>org.hibernate.javax.persistence</groupId>
            <artifactId>hibernate-jpa-2.1-api</artifactId>
            <version>1.0.0.Final</version>
        </dependency>
    </dependencies>

Resulting .factorypath:

<factorypath>
    <factorypathentry kind="VARJAR" id="M2_REPO/org/hibernate/hibernate-jpamodelgen/4.3.7.Final/hibernate-jpamodelgen-4.3.7.Final.jar" enabled="true" runInBatchMode="false"/>
    <factorypathentry kind="VARJAR" id="M2_REPO/org/jboss/logging/jboss-logging/3.1.3.GA/jboss-logging-3.1.3.GA.jar" enabled="true" runInBatchMode="false"/>
    <factorypathentry kind="VARJAR" id="M2_REPO/org/jboss/logging/jboss-logging-annotations/1.2.0.Beta1/jboss-logging-annotations-1.2.0.Beta1.jar" enabled="true" runInBatchMode="false"/>
</factorypath>

:+1:

JoshIsOnIt commented 8 years ago

Everything works great for me with this patch. Much cleaner than before, and more along the lines of what behaviour I would expect. Thanks again!

mickaelistria commented 2 years ago

m2e-apt's code is now included in https://github.com/eclipse-m2e/m2e-core , please consider reporting issue to https://github.com/eclipse-m2e/m2e-core/issues if it's still relevant.