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

Declaring explicitly the build service in the QuarkusBuildTask #44399

Merged

Conversation

cdsap
Copy link
Contributor

@cdsap cdsap commented Nov 9, 2024

#44367 describes the issue of the task quarkusAppPartsBuild in a project applying quarkus in multiple modules.
The issue is reproducible in any project with the following configuration:

//module a
plugins {
    `java-library`
    id("io.quarkus")
    id("com.github.ben-manes.versions") version "0.51.0"
}

//module b
plugins {
    `java-library`
    id("io.quarkus")
    id("com.github.ben-manes.versions") version "0.51.0"
}

The property failing is additionalForcedProperties, which represents a BuildSerivice with the forced properties used by different tasks. The build service uses the @ServiceReference annotation that eliminates the need to declare the association between the task and the service explicitly. This is the part that is failing and I need to confirm with the Build Tool team If it's an expected behavior.
This PR implements the explicit declaration between the task and the service:

        task.getAdditionalForcedProperties().set(serviceProvider);
        task.usesService(serviceProvider)

https://docs.gradle.org/current/userguide/build_services.html#sec:service_references

@quarkus-bot quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/gradle Gradle labels Nov 9, 2024
@geoand
Copy link
Contributor

geoand commented Nov 11, 2024

cc @aloubyansky

@cdsap
Copy link
Contributor Author

cdsap commented Nov 12, 2024

I'm still working on a reproducer (without the QuarkusPlugin) for the Build Tool team, a simple:

public abstract class FooAbstractTask extends DefaultTask {
    @ServiceReference("forcedPropertiesService")
    abstract Property<ForcedPropertieBuildService> getAdditionalForcedProperties();

with the same configuration as the reproducer is not triggering the issue.

@cdsap cdsap force-pushed the build_service_provider_explicitly_task branch 2 times, most recently from d74c319 to d0ae6d4 Compare November 13, 2024 23:11
@cdsap cdsap marked this pull request as ready for review November 14, 2024 01:22
@cdsap
Copy link
Contributor Author

cdsap commented Nov 14, 2024

Hi @aloubyansky, after discussing this issue with the Build tool team I'm opening for review this PR.
The original issue is described here gradle/gradle#17559

When registering a build service, due to how classloaders are shared or not among projects (essentially, projects with the same set of plugins applied share a classloader, though project hierarchies add additional variations), the class backing a shared build service registered by a plugin against a project may be incompatible with the class available under some other project.

The original idea was to remove the BuildService, but first, we need to address the issue where the actions nativeArgs and manifest need to be extracted from QuarkusBuild and included in the Quarkus extension to later be wired as inputs (I will create an issue soon). This PR addresses the issue by declaring a build service for each project where the Quarkus plugin is applied and explicitly declaring the build service in the task.

Copy link
Member

@aloubyansky aloubyansky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @cdsap for all the details and the patch

@geoand geoand added triage/waiting-for-ci Ready to merge when CI successfully finishes triage/backport-3.16 triage/backport labels Nov 14, 2024

This comment has been minimized.

@aloubyansky
Copy link
Member

The CI seems to be stuck again

@geoand
Copy link
Contributor

geoand commented Nov 14, 2024

Yeah, really weird. I would just rebase and force push if I were you @aloubyansky

@aloubyansky aloubyansky force-pushed the build_service_provider_explicitly_task branch from d0ae6d4 to b6324cd Compare November 14, 2024 08:41
@geoand
Copy link
Contributor

geoand commented Nov 14, 2024

👍🏽

Seems to be good now

Copy link

quarkus-bot bot commented Nov 14, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit b6324cd.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@aloubyansky aloubyansky merged commit 8a57c8f into quarkusio:main Nov 14, 2024
21 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Nov 14, 2024
@quarkus-bot quarkus-bot bot added kind/bugfix and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Nov 14, 2024
@gsmet gsmet modified the milestones: 3.18 - main, 3.16.4 Nov 19, 2024
benkard pushed a commit to benkard/quarkus-googlecloud-jsonlogging that referenced this pull request Nov 27, 2024
…oud-jsonlogging!25)

This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [io.quarkus:quarkus-extension-processor](https://github.com/quarkusio/quarkus) |  | minor | `3.16.3` -> `3.17.0` |
| [io.quarkus:quarkus-extension-maven-plugin](https://github.com/quarkusio/quarkus) | build | minor | `3.16.3` -> `3.17.0` |
| [io.quarkus:quarkus-bom](https://github.com/quarkusio/quarkus) | import | minor | `3.16.3` -> `3.17.0` |
| [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | minor | `3.16.3` -> `3.17.0` |
| [org.jboss.logmanager:jboss-logmanager](https://jboss.org) ([source](https://github.com/jboss-logging/jboss-logmanager)) | optional | minor | `3.0.6.Final` -> `3.1.0.Final` |

---

### Release Notes

<details>
<summary>quarkusio/quarkus</summary>

### [`v3.17.0`](quarkusio/quarkus@3.16.4...3.17.0)

[Compare Source](quarkusio/quarkus@3.16.4...3.17.0)

### [`v3.16.4`](https://github.com/quarkusio/quarkus/releases/tag/3.16.4)

[Compare Source](quarkusio/quarkus@3.16.3...3.16.4)

##### Complete changelog

-   [#&#8203;37040](quarkusio/quarkus#37040) - The flyway extension generates Kubernetes resources as if quarkus.flyway.enabled was a runtime property
-   [#&#8203;42446](quarkusio/quarkus#42446) - Add explanation/concept for extension maturity model
-   [#&#8203;44367](quarkusio/quarkus#44367) - Gradle 3.16 fails with missing required property `additionalForcedProperties`
-   [#&#8203;44399](quarkusio/quarkus#44399) - Declaring explicitly the build service in the QuarkusBuildTask
-   [#&#8203;44433](quarkusio/quarkus#44433) - Reflection free serializers ArrayIndexOutOfBoundsException
-   [#&#8203;44438](quarkusio/quarkus#44438) - Gradle `buildForkOptions` no longer used since quarkus 3.16.1
-   [#&#8203;44457](quarkusio/quarkus#44457) - Support for short and uncommon field names like set, get, and is
-   [#&#8203;44468](quarkusio/quarkus#44468) - Use `QUARKUS_FLYWAY_ACTIVE` instead of `QUARKUS_FLYWAY_ENABLED` env in Kubernetes resources
-   [#&#8203;44472](quarkusio/quarkus#44472) - Kotlin native Jackson serialization regression: EmptyList & EmptyMap missing
-   [#&#8203;44480](quarkusio/quarkus#44480) - Fix nullpointer on null code websockets-next
-   [#&#8203;44493](quarkusio/quarkus#44493) - Using BuildForkOptions in QuarkusBuildTask
-   [#&#8203;44494](quarkusio/quarkus#44494) - Register Kotlin's empty list and map for reflection
-   [#&#8203;44505](quarkusio/quarkus#44505) - Log in smallrye-jwt and oauth2 extensions when no bearer access token is available
-   [#&#8203;44507](quarkusio/quarkus#44507) - Fixed Timestamp not being set for otel log signals
-   [#&#8203;44509](quarkusio/quarkus#44509) - Updates to Infinispan 15.0.11.Final
-   [#&#8203;44515](quarkusio/quarkus#44515) - Coordinated Vert.x 4.5.11 upgrades
-   [#&#8203;44531](quarkusio/quarkus#44531) - Correct image file name to resolve broken image
-   [#&#8203;44537](quarkusio/quarkus#44537) - Update smallrye-jwt to 4.6.1
-   [#&#8203;44545](quarkusio/quarkus#44545) - Wrong index of ParameterizedType argument of Map when register type to be generated in JacksonCodeGenerator
-   [#&#8203;44571](quarkusio/quarkus#44571) - Update `CacheJsonRPCService.java` reference
-   [#&#8203;44574](quarkusio/quarkus#44574) - Grammar corrections for en-us

</details>

<details>
<summary>jboss-logging/jboss-logmanager</summary>

### [`v3.1.0.Final`](https://github.com/jboss-logging/jboss-logmanager/releases/tag/v3.1.0.Final): 3.1.0.Final

[Compare Source](jboss-logging/jboss-logmanager@3.0.6.Final...v3.1.0.Final)

#### What's Changed

-   \[LOGMGR-345] Ensure logger FQCN is correct for system logger by [@&#8203;dmlloyd](https://github.com/dmlloyd) in jboss-logging/jboss-logmanager#457
-   Migrate tests to keep the log files that were created. Put the log fi… by [@&#8203;jamezp](https://github.com/jamezp) in jboss-logging/jboss-logmanager#459
-   Bump org.junit:junit-bom from 5.10.1 to 5.10.2 by [@&#8203;dependabot](https://github.com/dependabot) in jboss-logging/jboss-logmanager#461
-   \[LOGMGR-346] Bump org.jboss.modules:jboss-modules from 2.1.2.Final to 2.1.3.Final by [@&#8203;dependabot](https://github.com/dependabot) in jboss-logging/jboss-logmanager#462
-   \[LOGMGR-347] Do not use deprecated SmallRye Common OS `Process` by [@&#8203;dmlloyd](https://github.com/dmlloyd) in jboss-logging/jboss-logmanager#464
-   \[LOGMGR-349] Bump org.eclipse.parsson:parsson from 1.1.5 to 1.1.6 by [@&#8203;dependabot](https://github.com/dependabot) in jboss-logging/jboss-logmanager#466
-   \[LOGMGR-351] Fix periodic file rotation by week, month, year. by [@&#8203;alex-pumpkin](https://github.com/alex-pumpkin) in jboss-logging/jboss-logmanager#468
-   Bump org.jboss.modules:jboss-modules from 2.1.3.Final to 2.1.5.Final by [@&#8203;dependabot](https://github.com/dependabot) in jboss-logging/jboss-logmanager#467
-   \[LOGMGR-350] Avoid TCCL when configuring the log manager by [@&#8203;dmlloyd](https://github.com/dmlloyd) in jboss-logging/jboss-logmanager#469
-   \[LOGMGR-351] Remove the deprecated per-deployment logging options. by [@&#8203;jamezp](https://github.com/jamezp) in jboss-logging/jboss-logmanager#471
-   Bump org.junit:junit-bom from 5.10.2 to 5.10.3 by [@&#8203;dependabot](https://github.com/dependabot) in jboss-logging/jboss-logmanager#478
-   Bump org.jboss.byteman:byteman-bmunit5 from 4.0.22 to 4.0.23 by [@&#8203;dependabot](https://github.com/dependabot) in jboss-logging/jboss-logmanager#476
-   Bump org.junit:junit-bom from 5.10.3 to 5.11.2 by [@&#8203;dependabot](https://github.com/dependabot) in jboss-logging/jboss-logmanager#488
-   Bump org.junit:junit-bom from 5.11.2 to 5.11.3 by [@&#8203;dependabot](https://github.com/dependabot) in jboss-logging/jboss-logmanager#490
-   \[LOGMGR-354] Avoid expensive JLine setup on JDK 23+ by [@&#8203;dmlloyd](https://github.com/dmlloyd) in jboss-logging/jboss-logmanager#491
-   Save head encoding on sanitized String(s) by [@&#8203;franz1981](https://github.com/franz1981) in jboss-logging/jboss-logmanager#492
-   Use `NO_FORMAT` when using parameterless log methods by [@&#8203;dmlloyd](https://github.com/dmlloyd) in jboss-logging/jboss-logmanager#493
-   Switch to formal module descriptor by [@&#8203;dmlloyd](https://github.com/dmlloyd) in jboss-logging/jboss-logmanager#494
-   Module descriptor updates by [@&#8203;dmlloyd](https://github.com/dmlloyd) in jboss-logging/jboss-logmanager#496
-   Bump org.jboss.modules:jboss-modules from 2.1.5.Final to 2.1.6.Final by [@&#8203;dependabot](https://github.com/dependabot) in jboss-logging/jboss-logmanager#495
-   Add smart service provider method by [@&#8203;dmlloyd](https://github.com/dmlloyd) in jboss-logging/jboss-logmanager#497

#### New Contributors

-   [@&#8203;alex-pumpkin](https://github.com/alex-pumpkin) made their first contribution in jboss-logging/jboss-logmanager#468
-   [@&#8203;franz1981](https://github.com/franz1981) made their first contribution in jboss-logging/jboss-logmanager#492

**Full Changelog**: jboss-logging/jboss-logmanager@3.0.4.Final...v3.1.0.Final

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

👻 **Immortal**: This MR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/gradle Gradle kind/bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gradle 3.16 fails with missing required property additionalForcedProperties
4 participants