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

Remove mvnd.builder.rule* and mvnd.builder.rules.provider.* features #264

Closed
ppalaga opened this issue Dec 15, 2020 · 10 comments
Closed

Remove mvnd.builder.rule* and mvnd.builder.rules.provider.* features #264

ppalaga opened this issue Dec 15, 2020 · 10 comments
Milestone

Comments

@ppalaga
Copy link
Contributor

ppalaga commented Dec 15, 2020

By default mvnd build modules of multi-module source tree in parallel. The ordering of the modules is given by their dependencies. However, sometimes, in real world scenarios, modules may require another module to be built before itself, although there is no Maven dependency relationship between them. The properties mvnd.builder.rules, mvnd.builder.rule, mvnd.builder.rules.provider.url and mvnd.builder.rules.provider.script were introduced as an additional mean for the users to control the build order. After some time, we found out that "virtual" dependecies like the following can be used to reach the same goal:

        <dependency>
            <groupId>org.group</groupId>
            <artifactId>my-dependency</artifactId>
            <version>${project.version}</version>
            <type>pom</type>
            <scope>test</scope>
            <exclusions>
                <exclusion>
                    <groupId>*</groupId>
                    <artifactId>*</artifactId>
                </exclusion>
            </exclusions>
        </dependency>

We have no evidence that mvnd.builder.rules, mvnd.builder.rule, mvnd.builder.rules.provider.url and mvnd.builder.rules.provider.script have found any substantial adoption and hence we would like to deprecate them now and remove them in some future release in favor of the "virtual" dependecies.

@gnodet
Copy link
Contributor

gnodet commented Jan 7, 2021

@ppalaga
Copy link
Contributor Author

ppalaga commented Jan 7, 2021

There is perhaps a couple of left-overs that we need to replace with virtual deps, that we generally use already https://github.com/apache/camel-quarkus/blob/cb66cca3e978e49cb5f55d2d63d35332a8632b59/catalog/pom.xml#L62-L75

@gnodet
Copy link
Contributor

gnodet commented Jan 7, 2021

@ppalaga this is still 4000 lines of dependencies even if they are automatically generated...
Did the quarkus team agreed to go the same way with the stock maven build ?
The benefit of the rules is that they could be specified in the .mvn/mvnd.properties and be tuned without disturbing the main project like it had to be done with camel-quarkus.

@ppalaga
Copy link
Contributor Author

ppalaga commented Jan 7, 2021

@ppalaga this is still 4000 lines of dependencies even if they are automatically generated...

I agree that it is a lot of LoC. Maven XML is verbose in general. But still, the virtual deps kind of solution being stock Maven is a huge advantage in my eyes. It also solves the problem for stock Maven parallel builds.

Did the quarkus team agreed to go the same way with the stock maven build ?

That's where I have it from. I think it was @famod who introduced it there, so he may want to comment on how's the agreement there.

The benefit of the rules is that they could be specified in the .mvn/mvnd.properties and be tuned without disturbing the main project like it had to be done with camel-quarkus.

This info belongs to the project IMO, where both people and tools can see it.

@famod
Copy link
Contributor

famod commented Jan 7, 2021

I think it was @famod who introduced it there, so he may want to comment on how's the agreement there.

Yes, it was me. It is not pretty by any means but better not fight or bypass the build tool you've chosen.
(Most of) The Quarkus team members agreed to do it this way, mostly due to lack of good alternatives that have fewer drawbacks.
If choosing this approach I strongly suggest to establish some dev support like the enforcer rules I added to Quarkus.

@gnodet
Copy link
Contributor

gnodet commented Jan 7, 2021

@famod is the quarkus build supposed to support parallel build ?
It seems to fail for me when trying to build with mvnd from a clean repo.

INFO] Quarkus - Integration Tests - gRPC - Interceptors .. SKIPPED
[INFO] Quarkus - Integration Tests - gRPC - Proto v2 ...... SKIPPED
[INFO] Quarkus - Integration Tests - gRPC - Health ........ SKIPPED
[INFO] Quarkus - Integration Tests - Google Cloud Functions HTTP SKIPPED
[INFO] Quarkus - Integration Tests - Google Cloud Function  SKIPPED
[INFO] Quarkus - Documentation ............................ FAILURE [ 13.124 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  33.400 s (Wall Clock)
[INFO] Finished at: 2021-01-07T14:57:18+01:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.codehaus.mojo:exec-maven-plugin:3.0.0:java (default) on project quarkus-documentation: An exception occured while executing the Java class. Failed to resolve artifacts: The following artifacts could not be resolved: io.quarkus:quarkus-caffeine-deployment:jar:999-SNAPSHOT, io.quarkus:quarkus-smallrye-reactive-type-converters-deployment:jar:999-SNAPSHOT, io.quarkus:quarkus-spring-di-deployment:jar:999-SNAPSHOT, io.quarkus:quarkus-spring-boot-properties-deployment:jar:999-SNAPSHOT, io.quarkus:quarkus-kotlin-deployment:jar:999-SNAPSHOT, io.quarkus:quarkus-amazon-alexa-deployment:jar:999-SNAPSHOT, io.quarkus:quarkus-container-image-docker-deployment:jar:999-SNAPSHOT, io.quarkus:quarkus-container-image-jib-deployment:jar:999-SNAPSHOT, io.quarkus:quarkus-container-image-deployment:jar:999-SNAPSHOT, io.quarkus:quarkus-scala-deployment:jar:999-SNAPSHOT, io.quarkus:quarkus-jgit-deployment:jar:999-SNAPSHOT, io.quarkus:quarkus-jsch-deployment:jar:999-SNAPSHOT: Could not find artifact io.quarkus:quarkus-caffeine-deployment:jar:999-SNAPSHOT -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <args> -rf :quarkus-documentation

@ppalaga
Copy link
Contributor Author

ppalaga commented Jan 7, 2021

#309 is not urgent for me if you I'd prefer keeping the rule props, @gnodet
I'd prefer releasing ASAP with or without this one.

@gnodet
Copy link
Contributor

gnodet commented Jan 7, 2021

#309 is not urgent for me if you I'd prefer keeping the rule props, @gnodet
I'd prefer releasing ASAP with or without this one.

+1

@famod
Copy link
Contributor

famod commented Jan 7, 2021

@gnodet Not entirely, yet. The failure you are seeing will be fixed by quarkusio/quarkus#13218
Workaround until then: build singlethreaded once.

@ppalaga
Copy link
Contributor Author

ppalaga commented Jan 24, 2021

Fixed via 82df39d

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

Successfully merging a pull request may close this issue.

3 participants