Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip execution for Maven projects with packaging "pom" #10

Closed
GoogleCodeExporter opened this issue Mar 14, 2015 · 10 comments
Closed

Skip execution for Maven projects with packaging "pom" #10

GoogleCodeExporter opened this issue Mar 14, 2015 · 10 comments

Comments

@GoogleCodeExporter
Copy link

I'm using the plugin in a multi-module Maven project. To apply it consistently 
to all modules, I configure it in the root/parent POM. However, this causes a 
problem because in this setup, the plugin is also executed when the root 
project itself is build. I think that the easiest way to avoid this is to 
automatically skip execution of the plugin in Maven projects with packaging 
"pom". Since these projects never have sources, there is no point in executing 
the plugin anyway.

I've attached a patch that modifies AbstractCheckMojo in the suggested way.

Original issue reported on code.google.com by [email protected] on 18 Sep 2013 at 10:41

Attachments:

@GoogleCodeExporter
Copy link
Author

Hi Andreas,
thank you for the patch! I am currently investigating other ways to prevent 
execution if there are no sources/classes, but your varaiant seems to be used 
quite often!

In general the forbidden-api checker should not fail when no sources are 
available, it just prints a warning if the "classes" directory is not available 
in the target. So the question is:
What is exactly your problem? Is it because you have no signatures configured, 
so it fails with that code:

      if (checker.hasNoSignatures()) {
        throw new MojoExecutionException("No API signatures found; use parameters 'signatures', 'bundledSignatures', and/or 'signaturesFiles' to define those!");
      }


Uwe

Original comment by uwe.h.schindler on 18 Sep 2013 at 1:16

@GoogleCodeExporter
Copy link
Author

One other question:
If packaging=pom, process-classes should never be executed, so what's special 
in your porject?

See: 
http://books.sonatype.com/mvnref-book/reference/lifecycle-sect-package-specific.
html

Original comment by uwe.h.schindler on 18 Sep 2013 at 1:38

@GoogleCodeExporter
Copy link
Author

Here is my use case. I've created a signature file with 
forbidden/internal/deprecated APIs for one of the projects I'm working on 
(Apache Axiom). That is packaged as a JAR file:

https://svn.apache.org/repos/asf/webservices/axiom/trunk/forbiddenapi-signatures
/

In a downstream project, I then configure the plugin in the root/parent POM as 
follows:

            <plugin>
                <groupId>de.thetaphi</groupId>
                <artifactId>forbiddenapis</artifactId>
                <version>1.3</version>
                <executions>
                    <execution>
                        <goals>
                            <goal>check</goal>
                            <goal>testCheck</goal>
                        </goals>
                    </execution>
                </executions>
                <configuration>
                    <bundledSignatures>
                        <bundledSignature>axiom</bundledSignature>
                    </bundledSignatures>
                </configuration>
                <dependencies>
                    <dependency>
                        <groupId>org.apache.ws.commons.axiom</groupId>
                        <artifactId>forbiddenapi-signatures</artifactId>
                        <version>${axiom.version}</version>
                    </dependency>
                </dependencies>
            </plugin>

The build of the root/parent POM fails with the following error:

[ERROR] Failed to execute goal de.thetaphi:forbiddenapis:1.3:check (default) on 
project Apache-Synapse: Parsing signatures failed: Loading of class 
org.apache.axiom.om.impl.builder.StAXBuilder failed: Not found -> [Help 1]

That is because Axiom is not declared as a dependency of the root/parent POM, 
but only in individual modules.

That being said, I have the same problem in some of the modules of the project 
because they also lack a dependency on Axiom. So in addition I will need an 
option to ignore signatures referring to classes that are not in the 
dependencies of the project. Looking at the code of the plugin, I don't think 
that this is currently supported. I will open another issue for that.

Original comment by [email protected] on 18 Sep 2013 at 3:14

@GoogleCodeExporter
Copy link
Author

Regarding your comment #2, you are misinterpreting the Maven documentation. 
Section 4.2.2 only lists the lifecycle _bindings_ for the POM packaging, i.e. 
the goals that are executed by default in each phase. The lifecycle itself 
doesn't depend on the packaging and always has the process-classes phase 
(unless one invokes the clean or site lifecycle), as shown in section 4.1.2.

Original comment by [email protected] on 18 Sep 2013 at 3:21

@GoogleCodeExporter
Copy link
Author

Hi Andreas,
thanks for the feedback! Interpreting comment #4, I think the whole exclusion 
is needed for some projects. I am not a Maven guy - everybody knows, I *hate* 
Maven :-), so I just trust you.

I am not happy of hardcoding "pom" as packaging directly into the java code and 
don't run the goal for this special case - this looks too unfelxible to me. 
What happens if you define a new packaging type which also have no classes? Not 
even the Maven Compiler plugin uses this packaging, it just does not compile if 
no sources are available. I think the better way to fix this would be to 
reorder the execution of the goal to not parse the signatures before validating 
that the target directories are existing. If the signature parsing is done 
later and the DirectoryScanner filesets are build before, the task would 
succedd (it would just print the warning that no class files were found or the 
directory does not exits).

Your setup of the signatures is really funny to me, but very intelligent! I did 
not think about that way to do it. You have the signatures as a separate Maven 
module and just include it into the plugin's classpath via a dependency - very 
cool (that way you can use bundledSignatures). This is somehow a hack, but we 
could make a more configureable way to do this (not with the hardcoded package 
name). Maybe open an issue - bundledSignatures was not be meant as extension 
point.

About the signatures missing in classpath: Early versions of forbidden-apis 
used by Apache Lucene (before it was a googlecode project) were less fussy 
about signatures. Unfortuantely we made it strict, because otherwise there is 
no guarantee that the signatures are actually working.

In my opinion, its a bug in the project setup, if the testsed signatures 
contain references to classes not available on classpath. This is like trying 
to complie a project with missing dependencies: javac will complain. So I don't 
think, this should be allowed!

Original comment by uwe.h.schindler on 18 Sep 2013 at 3:45

@GoogleCodeExporter
Copy link
Author

> I am not happy of hardcoding "pom" as packaging directly into the java code 
and don't run the goal for this special case - this looks too unfelxible to me. 
What happens if you define a new packaging type which also have no classes? Not 
even the Maven Compiler plugin uses this packaging, it just does not compile if 
no sources are available. I think the better way to fix this would be to 
reorder the execution of the goal to not parse the signatures before validating 
that the target directories are existing. If the signature parsing is done 
later and the DirectoryScanner filesets are build before, the task would 
succedd (it would just print the warning that no class files were found or the 
directory does not exits).

Yes that would solve the issue in my case. In addition, it avoids loading 
signatures unnecessarily. One may still want to hardcode the "pom" packaging to 
avoid the warning message, because for this packaging type it is expected that 
there are no classes.

> Your setup of the signatures is really funny to me, but very intelligent! I 
did not think about that way to do it. You have the signatures as a separate 
Maven module and just include it into the plugin's classpath via a dependency - 
very cool (that way you can use bundledSignatures). This is somehow a hack, but 
we could make a more configureable way to do this (not with the hardcoded 
package name). Maybe open an issue - bundledSignatures was not be meant as 
extension point.

If people successfully use your code in ways that you didn't think of, then 
this is usually an indication of good design ;-)

I don't think it's a hack, but you are right that the resource location for the 
lookup should be something more standard, such as 
META-INF/forbiddenapi-signatures/<name>.txt.

> About the signatures missing in classpath: Early versions of forbidden-apis 
used by Apache Lucene (before it was a googlecode project) were less fussy 
about signatures. Unfortuantely we made it strict, because otherwise there is 
no guarantee that the signatures are actually working. In my opinion, its a bug 
in the project setup, if the testsed signatures contain references to classes 
not available on classpath. This is like trying to complie a project with 
missing dependencies: javac will complain. So I don't think, this should be 
allowed!

I understand the reason for that, but in my use case it's really not practical. 
It means that I would have to configure the plugin individually in each module 
that has the dependency, and that I have no way to enforce the check on the 
entire project (in particular for modules that are added later). Some of the 
project where I'm planning to use the plugin have a large number of modules 
(see e.g. https://builds.apache.org/job/Axis2/modules), so I really want to 
avoid this. What I'm proposing is to add an option that allows me to ignore 
signatures for classes that are unresolvable. The default behavior would still 
be strict.

Original comment by [email protected] on 18 Sep 2013 at 4:34

@GoogleCodeExporter
Copy link
Author

I committed both changes in r215:
- Add the "pom" packaging check early
- Move the file scanning up, so before parsing the signatures, the files are 
already located and we can exit early (with warning) if directory is empty/does 
not exist.

I added no new test.

Original comment by uwe.h.schindler on 11 Oct 2013 at 3:45

  • Changed state: Committed

@GoogleCodeExporter
Copy link
Author

I will open a new issue to move the bundled signatures out of the package 
structure and move them to META-INF.

Original comment by uwe.h.schindler on 11 Oct 2013 at 3:48

@GoogleCodeExporter
Copy link
Author

I opened issue #13.

Original comment by uwe.h.schindler on 11 Oct 2013 at 3:54

@GoogleCodeExporter
Copy link
Author

Original comment by uwe.h.schindler on 21 Nov 2013 at 11:37

  • Changed state: Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

1 participant