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

Support multiple invocations? #3

Open
cherron opened this issue Oct 20, 2010 · 9 comments
Open

Support multiple invocations? #3

cherron opened this issue Oct 20, 2010 · 9 comments

Comments

@cherron
Copy link

cherron commented Oct 20, 2010

Feature suggestion: it would be cool if it were possible to have multiple invocations of the thrift compiler, varying the param in order to generate source for multiple languages.

@dtrott
Copy link
Owner

dtrott commented Oct 25, 2010

I understand that the plug in itself may not be configurable enough to define all the options necessary for non-java code however I think it should be possible to invoke the compiler multiple times using standard maven syntax.

What is it that you are trying to do?

@cherron
Copy link
Author

cherron commented Oct 25, 2010

I apologize, I didn't realize that I could configure two Maven executions of the same plugin
with different config. My aim is to generate both Java and C++ source from my Thrift IDL.
I updated my pom to run the plugin twice like so:

        <plugin>
            <groupId>org.apache.thrift.tools</groupId>
            <artifactId>maven-thrift-plugin</artifactId>
            <version>0.1.8</version>
            <configuration>
                <thriftExecutable>/usr/local/bin/thrift</thriftExecutable>                    
            </configuration>
            <executions>
                <execution>
                    <id>thrift-java-generation</id>
                    <configuration>
                        <generator>java</generator>
                    </configuration>
                    <goals>
                        <goal>compile</goal>
                    </goals>
                </execution>
                <execution>
                    <id>thrift-cpp-generation</id>
                    <configuration>
                        <generator>cpp</generator>
                    </configuration>
                    <goals>
                        <goal>compile</goal>
                    </goals>
                </execution>
            </executions>
        </plugin> 

Running with cpp (whether alone or in addition to the java generation) results
in an NPE:

 java.lang.NullPointerException
at org.apache.thrift.maven.Thrift.moveGeneratedFiles(Thrift.java:115)
at org.apache.thrift.maven.Thrift.compile(Thrift.java:84)
at org.apache.thrift.maven.AbstractThriftMojo.execute(AbstractThriftMojo.java:158)
at org.apache.thrift.maven.ThriftCompileMojo.execute(ThriftCompileMojo.java:22)
at org.apache.maven.plugin.DefaultPluginManager.executeMojo(DefaultPluginManager.java:490)

... looking at the code, the Thrift class assumes that the generated source is placed in "gen-java". Can I suggest that the outputDirectory be made configurable and passed to the thrift compiler?

@cherron
Copy link
Author

cherron commented Oct 25, 2010

Clarification: outputDirectory is indeed configurable, but the method Thrift.moveGeneratedFiles() assumes genDir to be outputDirectory/GENERATED_JAVA.
It looks like the output subdirectory always matches the generator value, so perhaps it would be safe to assume a convention of / ?

@cherron
Copy link
Author

cherron commented Oct 25, 2010

Doh, markdown hijacked my last sentence. I meant to say: perhaps it would be safe to assume a convention of outputDirectory + generator for the genDir?

@cherron
Copy link
Author

cherron commented Oct 25, 2010

Turned out the issue was some file deletes and moves that assume only a single run for one generator configuration. I forked and committed some tweaks that allowed me to successfully run multiple executions and retain all generated source: 19dd708

@dtrott
Copy link
Owner

dtrott commented Oct 26, 2010

I have reviewed your fixes.
Originally the moves were put in to address an issue with some IDE's not liking the additional directory nesting (gen-java).

That said I agree that, the moving files "solution" was a hack, I am trying to think of a better solution, one option may be to have two directories:

Working directory - Where the thrift compiler outputs to.
Output directory - Which maven will add to the build path, to trigger the standard java compile cycle.

Instead of cleaning and moving at two points in the cycle we could just delete the output directory and overwrite with the entire contents of the workingdir/gen-java

Three questions:
1.) Would something like that work for you?
2.) Is there anything specical you need for output directories from multiple runs?
3.) Any chance you want to take at coding it ;-)

PS I know that this sounds a bit convoluted but I am trying not to break anyone who is already using the plug-in.

@cherron
Copy link
Author

cherron commented Oct 27, 2010

  1. Yes, that might work.
  2. Not really, as long as there's a way to avoid one run clobbering another's output.
  3. I had a go at coding it: cherro/maven-thrift-plugin@068b89c71ea284682966fad0650492cadd4f506d
    (hopefully my GitHub markdown is correct!)

Some notes:
I assumed it was reasonable to use a temporary directory for the working directory, in order to avoid having to require another user-specified parameter.

I added two boolean configuration flags:

  • cleanBeforeOutput, which is true by default. This would delete the specified outputDirectory before adding the generated source. Setting to false would accommodate the scenario where there are other things that need to be retained (e.g. writing directly into ${project.basedir}/src/main/java in order to share the generated source via the VCS).
  • compileOutput, which is true by default. This would cause the outputDirectory to be added Setting to false would accommodate the scenario where we don't want to compile, or maven cannot compile the generated source (e.g. C++).

The behavior for each invocation is:

  1. Create a uniquely named temporary directory for the workDirectory.
  2. Pass that directory path to the thrift executable
  3. After the thrift execution, expect to find a single directory in the workDirectory, with the prefix "gen-".
  4. Move all contents beneath that single "gen-" parent directory into the outputDirectory.
  5. If compileOutput = true, then add the outputDirectory to the maven build path.

Would you mind reviewing, in particular to see if there's any obvious impact to other use cases? If the changes are reasonable, there would need to be some more cleanup (JavaDoc etc.) to reflect the broader usage beyond Java-only generation.

@cherron
Copy link
Author

cherron commented Oct 27, 2010

Here is some sample config for two runs:

<properties>
    <thrift.executable>/usr/local/bin/thrift</thrift.executable>
    <maven-thrift-plugin.base.output.dir>
        ${project.basedir}/target/generated-sources/thrift
    </maven-thrift-plugin.base.output.dir>
    <maven-thrift-plugin.thrift.src.dir>
        ${project.basedir}/src/main/resources
    </maven-thrift-plugin.thrift.src.dir>
</properties>

<build>
    <plugins>
        <plugin>
            <groupId>org.apache.maven.plugins</groupId>
            <artifactId>maven-compiler-plugin</artifactId>
            <configuration>
                <source>1.6</source>
                <target>1.6</target>
            </configuration>
        </plugin>
        <plugin>
            <groupId>org.apache.thrift.tools</groupId>
            <artifactId>maven-thrift-plugin</artifactId>
            <version>0.1.9-SNAPSHOT</version>
            <configuration>
                <thriftExecutable>${thrift.executable}</thriftExecutable>
                <thriftSourceRoot>${maven-thrift-plugin.thrift.src.dir}
                </thriftSourceRoot>
                <cleanBeforeOutput>true</cleanBeforeOutput>
            </configuration>
            <executions>
                <execution>
                    <id>thrift-cpp-generation</id>
                    <configuration>
                        <generator>cpp</generator>
                        <outputDirectory>
                            ${maven-thrift-plugin.base.output.dir}/cpp
                        </outputDirectory>
                        <compileOutput>false</compileOutput>
                    </configuration>
                    <goals>
                        <goal>compile</goal>
                    </goals>
                </execution>
                <execution>
                    <id>thrift-java-generation</id>
                    <configuration>
                        <generator>java</generator>
                        <outputDirectory>
                            ${maven-thrift-plugin.base.output.dir}/java
                        </outputDirectory>
                        <compileOutput>true</compileOutput>
                    </configuration>
                    <goals>
                        <goal>compile</goal>
                    </goals>
                </execution>
            </executions>
        </plugin>
    </plugins>
</build>

@nwparker
Copy link

This would be nice to have

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

No branches or pull requests

3 participants