Skip to content

Commit 1219cbd

Browse files
committed
Do not use separate agent directories when agent is invoked on main
This is a workaround for goal execution ordering problems with Maven. For tests, we can add a synthetic goal which is executed in the same phase and will merge the agent files generated by the test execution. Unfortunately, this approach doesn't work if we want to instrument the main execution, which is sometimes the case. This is not doable because we won't have a phase to hook into and a goal to execute in order to merge the files. Even if we synthetize a goal and try to add it to the session execution, this would cause ordering issues because we don't know in which order the plugins are defined in the build file. In addition, this would simply not work for multi-module builds. As a workaround, we simply disable the split agent output directories in case we instrument the main execution. This will hopefully work for most invocations.
1 parent a8f6f1c commit 1219cbd

File tree

2 files changed

+50
-20
lines changed

2 files changed

+50
-20
lines changed

native-maven-plugin/src/main/java/org/graalvm/buildtools/maven/MergeAgentFilesMojo.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,14 @@ public class MergeAgentFilesMojo extends AbstractMojo {
7878
public void execute() throws MojoExecutionException, MojoFailureException {
7979
String agentOutputDirectory = agentOutputDirectoryFor(target, NativeExtension.Context.valueOf(context));
8080
File baseDir = new File(agentOutputDirectory);
81-
Path nativeImageExecutable = Utils.getNativeImage();
82-
File mergerExecutable = tryInstall(nativeImageExecutable);
83-
List<File> sessionDirectories = sessionDirectoriesFrom(baseDir.listFiles()).collect(Collectors.toList());
84-
invokeMerge(mergerExecutable, sessionDirectories, baseDir);
81+
if (baseDir.exists()) {
82+
Path nativeImageExecutable = Utils.getNativeImage();
83+
File mergerExecutable = tryInstall(nativeImageExecutable);
84+
List<File> sessionDirectories = sessionDirectoriesFrom(baseDir.listFiles()).collect(Collectors.toList());
85+
invokeMerge(mergerExecutable, sessionDirectories, baseDir);
86+
} else {
87+
getLog().debug("Agent output directory " + baseDir + " doesn't exist. Skipping merge.");
88+
}
8589
}
8690

8791
private File tryInstall(Path nativeImageExecutablePath) {

native-maven-plugin/src/main/java/org/graalvm/buildtools/maven/NativeExtension.java

Lines changed: 42 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,26 @@ public class NativeExtension extends AbstractMavenLifecycleParticipant {
7878
* and within the Maven POM as values of the {@code name} attribute in
7979
* {@code <options name="...">}.
8080
*/
81-
enum Context { main, test };
81+
enum Context {main, test}
8282

8383
static String testIdsDirectory(String baseDir) {
8484
return baseDir + File.separator + "test-ids";
8585
}
8686

8787
static String buildAgentArgument(String baseDir, Context context, List<String> agentOptions) {
8888
List<String> options = new ArrayList<>(agentOptions);
89-
options.add("config-output-dir=" + agentOutputDirectoryFor(baseDir, context) + File.separator + SharedConstants.AGENT_SESSION_SUBDIR);
89+
String effectiveOutputDir = agentOutputDirectoryFor(baseDir, context);
90+
if (context == Context.test) {
91+
// We need to patch the config dir IF, and only IF, we are running tests, because
92+
// test execution can be forked into a separate process and there's a race condition.
93+
// We have to special case testing here instead of using a generic strategy THEN
94+
// invoke the merging tool, because there's no way in Maven to do something as easy
95+
// as finalizing a goal (that is, let me do the merge AFTER you're done executing tests,
96+
// or invoking exec, or whatever, because what I need to do actually participates into
97+
// the same unit of work !).
98+
effectiveOutputDir = effectiveOutputDir + File.separator + SharedConstants.AGENT_SESSION_SUBDIR;
99+
}
100+
options.add("config-output-dir=" + effectiveOutputDir);
90101
return "-agentlib:native-image-agent=" + String.join(",", options);
91102
}
92103

@@ -153,18 +164,24 @@ public void afterProjectsRead(MavenSession session) throws MavenExecutionExcepti
153164
Context context = exec.getGoals().stream().anyMatch("test"::equals) ? Context.test : Context.main;
154165
Xpp3Dom agentResourceDirectory = findOrAppend(configuration, "agentResourceDirectory");
155166
agentResourceDirectory.setValue(agentOutputDirectoryFor(target, context));
156-
List<String> goals = new ArrayList<>();
157-
goals.add("merge-agent-files");
158-
goals.addAll(exec.getGoals());
159-
exec.setGoals(goals);
160-
Xpp3Dom agentContext = findOrAppend(configuration, "context");
161-
agentContext.setValue(context.name());
167+
if (context == Context.test) {
168+
setupMergeAgentFiles(exec, configuration, context);
169+
}
162170
});
163171
}
164172
});
165173
}
166174
}
167175

176+
private static void setupMergeAgentFiles(PluginExecution exec, Xpp3Dom configuration, Context context) {
177+
List<String> goals = new ArrayList<>();
178+
goals.add("merge-agent-files");
179+
goals.addAll(exec.getGoals());
180+
exec.setGoals(goals);
181+
Xpp3Dom agentContext = findOrAppend(configuration, "context");
182+
agentContext.setValue(context.name());
183+
}
184+
168185
private static void withPlugin(Build build, String artifactId, Consumer<? super Plugin> consumer) {
169186
build.getPlugins()
170187
.stream()
@@ -204,6 +221,7 @@ private static String getSelectedOptionsName(MavenSession session) {
204221
* {@code <options>} elements whose names match the name of the supplied {@code context}
205222
* or {@code selectedOptionsName} and for unnamed, shared {@code <options>} elements,
206223
* and returns a list of the collected agent options.
224+
*
207225
* @param nativePlugin the {@code native-maven-plugin}; never null
208226
* @param context the current execution context; never null
209227
* @param selectedOptionsName the name of the set of custom agent options activated
@@ -260,9 +278,12 @@ private static void processOptionNodes(Xpp3Dom options, List<String> optionsList
260278
private static boolean parseBoolean(String description, String value) {
261279
value = assertNotEmptyAndTrim(value, description + " must have a value").toLowerCase();
262280
switch (value) {
263-
case "true": return true;
264-
case "false": return false;
265-
default: throw new IllegalStateException(description + " must have a value of 'true' or 'false'");
281+
case "true":
282+
return true;
283+
case "false":
284+
return false;
285+
default:
286+
throw new IllegalStateException(description + " must have a value of 'true' or 'false'");
266287
}
267288
}
268289

@@ -304,15 +325,20 @@ private static void configureJunitListener(Plugin surefirePlugin, String testIds
304325

305326
private static void updatePluginConfiguration(Plugin plugin, BiConsumer<PluginExecution, ? super Xpp3Dom> consumer) {
306327
plugin.getExecutions().forEach(exec -> {
307-
Xpp3Dom configuration = (Xpp3Dom) exec.getConfiguration();
308-
if (configuration == null) {
309-
configuration = new Xpp3Dom("configuration");
310-
exec.setConfiguration(configuration);
311-
}
328+
Xpp3Dom configuration = configurationBlockOf(exec);
312329
consumer.accept(exec, configuration);
313330
});
314331
}
315332

333+
private static Xpp3Dom configurationBlockOf(PluginExecution exec) {
334+
Xpp3Dom configuration = (Xpp3Dom) exec.getConfiguration();
335+
if (configuration == null) {
336+
configuration = new Xpp3Dom("configuration");
337+
exec.setConfiguration(configuration);
338+
}
339+
return configuration;
340+
}
341+
316342
private static Xpp3Dom findOrAppend(Xpp3Dom parent, String childName) {
317343
Xpp3Dom child = parent.getChild(childName);
318344
if (child == null) {

0 commit comments

Comments
 (0)