You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/06/06 15:36:09 UTC
[GitHub] [maven] timboudreau opened a new pull request, #750: Allow Maven to resolve a parent it is about to build
timboudreau opened a new pull request, #750:
URL: https://github.com/apache/maven/pull/750
Fixes a long-standing issue when using Maven with git submodules:
* You have a library project in a git repository
* That library is part of a larger set of projects, but should be buildable with just a checkout of the library (so contributors interested in that particular corner of the project don't need to check out a huge amount of stuff to contribute to that one area)
* That library and its siblings share configuration via a shared superpom
* That means the library _must not_ make assumptions about what is in the parent directory - it must use `<relativePath/>` for its parent, so the parent will be resolved from the local repository or central
* You have an aggregation project that builds the superpom and _all_ of the siblings together, for a number of reasons
* So full time developers of the project can easily detect if they've broken compatibility with a sibling when they make changes
* So continuous builds can obtain a consistent view of the entire set or projects and build them together
So you wind up with an aggregation project that looks like:
* `Aggregator/`
* `pom.xml` - just a bill of materials, no parent
* `superpom-project` - a git submodule
* `pom.xml` - shared configuration, no parent
* `project-1` - another git submodule, for a Java project
* `pom.xml` - parents off of `superpom-project` with `<relativePath/>` so an isolated checkout is always buildable
* `project-2` - another git submodule for another Java project
* `pom.xml` - same setup as `project-1/pom.xml`
On a fresh checkout, with nothing locally built yet, building the aggregator will fail with _Non-resolvable parent POM_…and 'parent.relativePath' points at no local POM_ - if the superpom cannot yet be downloaded. *This is exactly the situation every time you bump the version of the superpom*.
After `superpom-project/pom.xml` has been manually built at least once, building the `Aggregator/pom.xml` will succeed.
This creates a number of problems:
1. Such projects always have to come with instructions or a "first time setup" script that explicitly builds the superpom to make the checkout usable for development
* The entire promise of Maven is that builds are "batteries included" - you check it out, type `mvn install` and it Just Works™
2. A "works for me" scenario, where the sources are the same, but those who have (sometimes accidentally) built the superpom have a build that works, and others do not, though the sources both have checked out are identical. This is yet another potent source of Maven hatred.
3. If it does work, you are actually building against a historical copy of `superpom/pom.xml` instead of the (possibly changed) copy in source, and won't know it unless something goes wrong
4. What you deploy may not be what you think it is. Say that I
* Build `superpom/pom.xml` locally
* Change the version of a library used by `project-1/pom.xml` from 1.0 to 1.1 _in the superpom source only_, build the aggregator and deploy to maven central
* The tests for `project-1` will have passed, but they are ran against version 1.0 of that dependency, read from `~/.m2/repository`
5. Distrust of Maven in general - Maven _knows perfectly well_ that it is going to build `superpom/pom.xml` (the fact that this patch succeeds in fixing the behavior proves _that_), but it claims not to be able to find a file it has literally already read and built a model for
This small patch solves that. The name of the interface `WorkspaceModelResolver` and its signature strongly suggest that it was created to solve this exact problem - but there are no implementations of the interface _at all_ within Maven's sources. I have no idea _why_ it was never implemented, but simply attempting to resolve the `Model` against the set of models already read is a sane and straightforward way to do it. This patch provides one that can resolve models against projects which have already been, themselves, resolved.
* The patch is sensitive to the order of `<module>` entries in the aggregator POM - if the superpom is not listed _before anything that depends on it_ the build will fail in the usual way
* While that's less than ideal, it has far less severe consequences than the existing behavior
* It could be made more robust by pre-sorting `<module>` entries so that those modules which have packaging `pom` and no `<module>` entries of their own are built first. That would probably require reading the `pom.xml` files of aggregator projects a little earlier than they currently are, in order to have enough information to perform that sort.
* The version argument to `WorkspaceModelResolver`'s methods is named `versionConstraint`, suggesting it could be a version range; the patch looks only for an exact string match. I expect this is harmless, because:
* It is not legal to specify a _version range_ for your parent POM
* If, say, `project-2` depends on a version range of `project-1` and both are built in the same reactor, the test will fail and `WorkspaceModelResolverImpl` will not resolve it, but the fallback is to the existing behavior which _will_ resolve it just as it would have without this patch, so the behavior does not change
Following this checklist to help us incorporate your
contribution quickly and easily:
- [ ] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] Format the pull request title like `[MNG-XXX] SUMMARY`, where you replace `MNG-XXX`
and `SUMMARY` with the appropriate JIRA issue. Best practice is to use the JIRA issue
title in the pull request title and in the first line of the commit message.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [ ] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [ ] You have run the [Core IT][core-its] successfully.
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
[core-its]: https://maven.apache.org/core-its/core-it-suite/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] timboudreau commented on pull request #750: Allow Maven to resolve a parent it is about to build
Posted by GitBox <gi...@apache.org>.
timboudreau commented on PR #750:
URL: https://github.com/apache/maven/pull/750#issuecomment-1147589016
Working on a backport to the 3.9.x branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] timboudreau commented on pull request #750: Allow Maven to resolve a parent it is about to build
Posted by GitBox <gi...@apache.org>.
timboudreau commented on PR #750:
URL: https://github.com/apache/maven/pull/750#issuecomment-1148177204
This may have been addressed in another way in the 4.x branch. Going to close this and open a patch for 3.9.x.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] timboudreau closed pull request #750: Allow Maven to resolve a parent it is about to build
Posted by GitBox <gi...@apache.org>.
timboudreau closed pull request #750: Allow Maven to resolve a parent it is about to build
URL: https://github.com/apache/maven/pull/750
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] jglick commented on pull request #750: Allow Maven to resolve a parent it is about to build
Posted by GitBox <gi...@apache.org>.
jglick commented on PR #750:
URL: https://github.com/apache/maven/pull/750#issuecomment-1147841396
> I have no idea _why_ it was never implemented
For M2Eclipse perhaps?
https://github.com/apache/maven-integration-testing would be the place to write a test AFAIK.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] timboudreau commented on pull request #750: Allow Maven to resolve a parent it is about to build
Posted by GitBox <gi...@apache.org>.
timboudreau commented on PR #750:
URL: https://github.com/apache/maven/pull/750#issuecomment-1147596279
Example repositories I know of with the problem this solves:
* [Mastfrog Parent](https://github.com/timboudreau/mastfrog-parent)
* [Telenav Build](https://github.com/telenav/telenav-build)
but given the number of questions about this exact behavior and how to solve the problem on StackOverflow, far from the only ones.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org