Support different agent modes in the native-image Gradle plugin by gradinac · Pull Request #230 · graalvm/native-build-tools

gradinac · 2022-05-06T15:09:09Z

This PR reworks agent support by decoupling the agent task instrumentation from image building and introduces the concept of agent modes.

melix

The changes look good to me, great work @gradinac ! So I think what's missing now is test coverage and documentation of the breaking changes. Potentially we could document the different agent modes too. I guess you'll have to discuss with @alvarosanchez for the Maven support to be added (fine if the Gradle plugin supports it first).

melix · 2022-05-23T12:56:50Z

native-gradle-plugin/src/main/java/org/graalvm/buildtools/gradle/dsl/GraalVMExtension.java

+    AgentOptions getAgent();
+
+    @Nested
+    CopyMetadataOptions getCopyMetadata();


this should probably live under AgentOptions

I agree, I've moved this to AgentOptions and renamed it to metadataCopy to remain consistent with the naming of other tasks

melix · 2022-05-23T12:58:25Z

samples/java-application-with-reflection/settings.gradle

    }
    repositories {
        mavenCentral()
+        mavenLocal()


Will need to remove this from draft PR

Ah yes, I've kept this to make it easier to test locally on multiple machines, I will remove all changes related to local testing

Rename copyMetadata to metadataCopy and move it into the agent block. Run the merge action from inside the MetadataCopyTask. Introduce a marker for agent options that is substituted with the output directory.

…st-processing lists tweakable.

melix

This is looking good, let's get this to the finish line!

melix · 2022-05-24T08:32:38Z

native-gradle-plugin/src/main/java/org/graalvm/buildtools/gradle/dsl/agent/AgentOptions.java

+     *
+     * @return .
+     */
+    @Input


This should be @Internal, it's not an input in the sense of task inputs.

That's a good point, I'll change it to @Internal

This is the last remaining issue AFAICT.

Snap, sorry for that, I missed this when refactoring, will push an updated version

melix · 2022-05-24T08:35:46Z

...lugin/src/main/java/org/graalvm/buildtools/gradle/tasks/actions/CleanupAgentFilesAction.java

+
+public class CleanupAgentFilesAction implements Action<Task> {
+
+    private final Provider<List<String>> directoriesToCleanup;


Should probably be a ListProvider or, even better, a ConfigurableFileCollection

Hmm, we could introduce a ListProperty, but we would manually have to instantiate it and pass it around. We could also go with a ConfigurableFileCollection, though in this context these files are either passed in from a task input, or created internally by mapping another property

Here's how they are created in this case: https://github.com/graalvm/native-build-tools/pull/230/files#diff-3ca19e7e49fe64feca3a1e88279c961b56e03450666583025c711da44cfb73bbR614-R615

Let's keep it as is for now, I'll take a look when I have time

melix · 2022-05-24T08:36:29Z

samples/java-application-with-reflection/build.gradle


 repositories {
    mavenCentral()
+    mavenLocal()


Should be removed

I agree, I will remove everything related to local testing from the PR

melix

Given that the "conditional" mode is the recommended mode for libraries, I wonder if we should make it the default when the java-library plugin is applied?

gradinac · 2022-05-27T12:56:00Z

Given that the "conditional" mode is the recommended mode for libraries, I wonder if we should make it the default when the java-library plugin is applied?

That's a good point - we could do this if the agent mode is not explicitly set on the command line and the user has not specified a `defaultMode in the DSL.

I've noticed plugins behave very similar to tasks in terms of lazyness - there's a withType call that's called back only when a plugin actually executes. For the agent, we hook into project evaluation so I don't think we can use that mechanism.
The alternative is to simply check if the JavaLibraryPlugin is included in the project.

One downside of doing this would be non-apparent changes in the build configuration that depend on included plugins

melix · 2022-05-27T13:52:30Z

mmm, not sure I follow you. You can do:

project.getPlugins().withType(JavaLibraryPlugin.class) { configure agent convention }

gradinac · 2022-05-27T13:58:48Z

mmm, not sure I follow you. You can do:
project.getPlugins().withType(JavaLibraryPlugin.class) { configure agent convention }

Ah yes, this would set the convention, but that's the bit i'm unsure about - would this execute before or after project evaluation?

melix · 2022-05-27T14:09:56Z

It would execute as soon as the java-library plugin is applied. That is to say immediately if it's already applied when this code executes, or later if it's not yet applied.

gradinac · 2022-05-27T14:23:01Z

It would execute as soon as the java-library plugin is applied. That is to say immediately if it's already applied when this code executes, or later if it's not yet applied.

Makes sense, thank you! :) I'll push an update to the PR

Aleksandar Gradinac added 2 commits April 28, 2022 17:27

Cleanup unused imports

6d8c46c

Support different agent modes in the native-image Gradle plugin

21d497b

graalvmbot added the oracle-emp label May 6, 2022

WIP

e2e54c4

gradinac force-pushed the gradinac/agent-modes-support branch from e2e54c4 to 21d497b Compare May 10, 2022 19:33

WIP

5bb0697

gradinac force-pushed the gradinac/agent-modes-support branch from ecff683 to 5bb0697 Compare May 19, 2022 15:25

melix requested changes May 23, 2022

View reviewed changes

Aleksandar Gradinac added 3 commits May 23, 2022 19:50

Multiple improvements.

00947ba

Rename copyMetadata to metadataCopy and move it into the agent block. Run the merge action from inside the MetadataCopyTask. Introduce a marker for agent options that is substituted with the output directory.

Post-process agent files in place

134d3d9

Properly re-run instrumented tasks if the agent mode changes. Make po…

c8635d1

…st-processing lists tweakable.

melix reviewed May 24, 2022

View reviewed changes

Aleksandar Gradinac added 2 commits May 25, 2022 04:53

Update agent functional tests

5b104dd

Agent doc refactor and support for additional options

1257d59

gradinac force-pushed the gradinac/agent-modes-support branch from 3722f5a to 1257d59 Compare May 27, 2022 01:47

melix approved these changes May 27, 2022

View reviewed changes

Change default agent mode to conditional when using JavaLibraryPlugin

b8d4464

gradinac marked this pull request as ready for review May 28, 2022 23:58

melix approved these changes May 29, 2022

View reviewed changes

gradinac merged commit 91e22c6 into master May 29, 2022

This was referenced May 31, 2022

Cannot configure "trace-output" agent option because plugins add "config-output-dir" option by default #189

Closed

Do not add option "config-output-dir" when conflicting options are pr… #190

Closed

lazar-mitrovic mentioned this pull request Jun 15, 2022

Rework or remove agent configuration merging #208

Closed

michael-simons mentioned this pull request Jun 27, 2022

build(deps): Bump native-maven-plugin from 0.9.11 to 0.9.12 michael-simons/neo4j-migrations#537

Merged

lazar-mitrovic mentioned this pull request Jun 29, 2022

Add agent modes to Maven plugin #260

Closed


		public class CleanupAgentFilesAction implements Action<Task> {

		private final Provider<List<String>> directoriesToCleanup;

Conversation

gradinac commented May 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

melix left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

melix left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

melix left a comment

Choose a reason for hiding this comment

Uh oh!

gradinac commented May 27, 2022

Uh oh!

melix commented May 27, 2022

Uh oh!

gradinac commented May 27, 2022

Uh oh!

melix commented May 27, 2022

Uh oh!

gradinac commented May 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gradinac commented May 6, 2022 •

edited

Loading