SBE Tool Fails Maven Multi-Module Builds

[dmitry-livchak-qco]

Context:

• SBE Tool is integrated in a Maven build
• in a multi-module project
• generates code for two independent SBE schemas in two different modules
• with build parallelism set high (e.g. mvn clean install --batch-mode -T 1.5C -U on an 8-core machine)

Error:

Settings passed on as System Properties, via pom.xml to module A are sometimes applied to module B. As an example, generated files may be written into another module's output folder.

Expected:

Settings applied per module as configured in the pom.xml of that module.

Suggestion:

Pass settings as Arguments to avoid concurrency issues caused by altering System Properties, or even better — load from a local file. Keep System Properties as a fallback for backwards compatibility.

[smandy]

I've arrived here as I'm having similar potential issue in a gradle kotlin DSL plugin. I'm calling SbeTool.main directly in my plugin ( rather than using a javaexec task), largely for efficiency reasons, so don't have to spawn a child JVM just to call this one method. So I'm having to call System.setProperty to inject properties into SbeTool.main. Would providing an overload that explicitly takes properties make sense? The main method would call this overload passing System.getProperties as an argument.

[smandy]

I've raised PR #1002 for consideration.

I believe this provides a workable solution for @dmitry-livchak-qco 's (and my!) problem

[vyazelenko]

Can't you fork the process that invokes the SbeTool? This way the code generator for each module will be independent.

[smandy]

Hmm... not a bad idea but I'm not familiar enough with gradle and/or how one would fork a java process (it's just never come up for me in java) . Would need to figure out a good way of ensuring the forked process had run to completion.

You'd also need to apply some locking before a fork as from what I understand you'd need to ensure you have correct system properties set prior to the fork. Otherwise other threads setting systemproperties prior to forking could potentially pollute forked processes that are forked from another thread. So basically you've got the same problem again.... happy to discuss though I'm maybe not understanding it that well. I've seldom used forking in the past :-(

[smandy]

Ug thinking about it I misspoke. You can set properties post-fork - duh sorry. ( See what I mean about not being good at forking ). The point about needing to know when the forked process terminates still stands though. You can't start compiling the output of SbeTool until you know for sure it has exited successfully,

[smandy]

Also another point. If you want to fork. Then just running a javaexec task will achieve the same (bit more performance overhead, but simpler implementation). The whole intention of the PR was so that I could avoid the overhead of spooling up a new jvm solely to call sbetool.

[npepinpe]

For those landing here like I did, here's the solution we ended up using for Maven. We have a multi-module project, with a parent POM, and a bunch of child POMs which inherit from it. We use the exec-maven-plugin to execute the tool. However, the exec:java goal doesn't support forking (AFAIK), so we switched to exec:exec, which is a little more cumbersome.

In the parent POM, define the plugin as:

<plugin>
  <groupId>org.codehaus.mojo</groupId>
  <artifactId>exec-maven-plugin</artifactId>
  <version>${plugin.version.exec}</version>
  <dependencies>
    <dependency>
      <groupId>uk.co.real-logic</groupId>
      <artifactId>sbe-tool</artifactId>
      <version>${version.sbe}</version>
    </dependency>
  </dependencies>
  <executions>
    <execution>
      <id>generate-sbe</id>
      <!-- not bound by default since it would execute in places where we don't want SBE generation -->
      <phase>none</phase>
      <goals>
        <goal>exec</goal>
      </goals>
      <configuration combine.children="override">
        <!--
          we have to use the goal exec with the java executable as otherwise the process
          will not fork, and this can cause thread safety issues in a parallel multi-module
          build
        -->
        <executable>java</executable>
        <includePluginDependencies>true</includePluginDependencies>
        <arguments combine.children="append">
          <argument>-Dsbe.output.dir=${project.build.directory}/generated-sources/sbe</argument>
          <argument>-Dsbe.java.generate.interfaces=true</argument>
          <argument>-Dsbe.decode.unknown.enum.values=true</argument>
          <argument>-Dsbe.xinclude.aware=true</argument>
          <argument>-classpath</argument>
          <!-- automatically creates the classpath using all project dependencies,
             also adding the project build directory -->
          <classpath/>
          <argument>uk.co.real_logic.sbe.SbeTool</argument>
        </arguments>
        <workingDirectory>${project.build.directory}/generated-sources</workingDirectory>
      </configuration>
    </execution>
  </executions>
</plugin>

[!Note] Note the usage of combine.children="override" on configuration, and combine.children="append" on arguments. This is on purpose to allow override settings, except arguments, where you want to append your schema files.

Then in a child POM, for example:

<plugin>
  <groupId>org.codehaus.mojo</groupId>
  <artifactId>exec-maven-plugin</artifactId>
  <executions>
    <execution>
      <id>generate-sbe</id>
      <phase>generate-sources</phase>
      <configuration>
        <arguments>
          <argument>${project.build.resources[0].directory}/schema.xml</argument>
        </arguments>
      </configuration>
    </execution>
  </executions>
</plugin>

Of course, ideally the exec:java goal of exec-maven-plugin would allow forking, but it does not, so :upside_down_face:

[smandy]

I still have a pull request open for this which I believe would fix this issue. But it's failing the codestyle checks so has been rejected for merge. #1002

There haven't been any comments either way but I think if fixing up the code style could get it over the line....