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

Create mojo to test if incompatible dependencies are converged #7

Open
hinerm opened this issue Nov 7, 2014 · 11 comments
Open

Create mojo to test if incompatible dependencies are converged #7

hinerm opened this issue Nov 7, 2014 · 11 comments
Milestone

Comments

@hinerm
Copy link
Member

hinerm commented Nov 7, 2014

We need a goal (or enforcer rule?) that finds all instances of dependency convergence and checks if a resolution occurred between semver-incompatible dependencies.

We will probably need custom version API to account for terms like beta-XXX

@ctrueden
Copy link
Member

A related sort of "convergence" is when you extend a POM parent foo, and then scope import another POM parent bar that itself uses foo as its parent. If these parents contain Bills of Materials (i.e., dependencyManagement), then managed versions can be out of sync. So: let's warn/fail if a project inherits from version X of foo, imports bar extending version Y of foo, and X != Y.

Such a check may end up being too draconian if a new foo is produced but no new corresponding Y. So it may actually be sufficient to simply check dependency convergence in general, and not worry so much about always using identical parent POM versions.

Worth keeping in mind, though.

@ctrueden
Copy link
Member

We should definitely consider whether the semantic-versioning project could be of assistance!

@ctrueden
Copy link
Member

We may also be able to reuse all or part of the maven-dependency-versions-check-plugin.

@axtimwalde
Copy link

Enforcing identical parents is not only not required but leads to a hen and egg problem that can only be resolved when both parent and child are under the control of the same person by dirty and unintuitive hacks (release script). Here is my thinking of what the rule has to enforce and how the simplest version can do this. Consider each module and dependency a node with a parent and children (modules and dependencies):

  1. Make sure that the dependency graph has no circles or the following would not work.
  2. For each node, calculate the versions of its modules and dependencies, these versions can be locally specified or inherited from a parent (in Maven, the top-level parent rules, mid-level overriding does not work, so trace back the parent tree and consume the version from its top-most occurrence). Write the artifactID and the version in a Map<ArtifactId, Version>. If an entry <artifactID, Version> with a different version exists, we have a conflict, report it and stop. Otherwise, proceed.
  3. For each module and dependency, recursively do 2.
  4. Report success.

If no conflict was found, everything is consistent and can be released. If no maven-plugin for this exists, it can be implemented by moderately simple XML traversal.

This test enables to not require artifacts to reference the same parent (which is non-practical for the above mentioned hen-and-egg problem) and it can also deal with artifacts overriding individual dependency versions which is necessary when updated parents do not yet exist. The root node for a Fiji release would be pom-fiji.

@ctrueden
Copy link
Member

@axtimwalde The issue I described above is different than enforcing identical parents. I am talking about situations like fiji/fiji, which must extend pom-fiji but import pom-trakem2 and pom-bigdataviewer. But those pom-trakem2 and pom-bigdataviewer POMs themselves also extend pom-fiji—and if they do so at a different version than the one extended by fiji/fiji, there can be managed dependency version skew. Since fiji/fiji has sole control over which versions of pom-fiji, pom-bigdataviewer and pom-trakem2 that it references, we could do some kind of sanity check there. There is no "hen and egg" problem here, but there probably is an issue with scenarios where a new pom-fiji is released, but no new pom-bigdataviewer or pom-trakem2.

Anyway, like I said above, further thought and consideration needed, but just checking for dependency convergence may be sufficient. Another option I briefly discussed with @hinerm recently would be to put pom-trakem2.version and pom-bigdataviewer.version properties into pom-fiji, which avoids any explicit circular dependencies, and frees downstream projects like pom-fiji from hard-coding (and later manually updating) versions of such child BOM projects.

@ctrueden
Copy link
Member

@axtimwalde By the way, I agree with your algorithm above in broad strokes. It is not allowed in Maven for a dependency graph to have cycles (at least for release dependencies—didn't test with snapshots), so we are good there. As for entries with "different versions", yes, but I'd phrase it as "divergent versions"—i.e., versions with a different major version number. Or in the case of the beta releases, a different beta number. Or for 0.x.y releases, a different x.

@axtimwalde
Copy link

@ctrueden My point is that it is unrealistic to expect all modules to have identical parents and to have the same parent as the bom that builds them. In particular, it is not correct to expect pom-fiji to manage dependencies consistently. It's purpose is to define a consistent state but it can not be in time for every update upstream. The managed dependencies, in particular, can not be expected to have the correct pom-fiji version as parent. This is the hen-and-egg problem that I was talking about. Ergo we need a version consistency crawler as outlined above. I see your point of auto-accepting patches but I would argue that doing so would require user intervention, I would very much prefer this to be a question rather than an auto-execution. Keep in mind that dependencies use only a small subset of the managed dependencies in their parents. So typically, the divergent managing poms will be equal in this local subset. But it needs to be checked.

@ctrueden
Copy link
Member

My point is that it is unrealistic to expect all modules to have identical parents and to have the same parent as the bom that builds them.

I think you don't need to spend any more words trying to convince @hinerm or me—we completely agree with that assessment.

@hinerm
Copy link
Member Author

hinerm commented Mar 20, 2015

Hi @ctrueden and @axtimwalde

I made a mcve demonstrating the inheritance concerns @ctrueden was describing in his original Nov post:
https://github.com/hinerm/pom-mcve

The reason we thought requiring a single parent pom might be necessary is that we didn't know what would happen when the dependencyManagement versions themselves conflicted, in the case of inheriting from multiple versions of pom-fiji.

My mcve demonstrates[1][2] that the managed version of each dependency can still be known. So I believe we do not need to enforce a common parent pom version.

To summarize the goals of this ticket:

The algorithm described by @axtimwalde is, I believe, equivalent to the dependencyConvergence enforcer rule.

We want to use a modified version of this rule such that:

  • Differing versions of the same artifact are allowed if they are SemVer compliant
  • When a managed dependency's version is overridden (e.g. "version X managed from Y") we need to fail if:
    • X has a different major version than Y
    • X has a version < Y (Y could have added new API or required bug fix not present in X)

@axtimwalde
Copy link

Great! It would be good though to report warnings about diverging versions even when SemVer promises compatibility to signalize that implications were made. There are also projects that do not follow SemVer, we would need to track a list of those.

If the dependencyConvergence enforcer rule works recursively and parses and interprets inherited versions correctly it would be equivalent to the algorithm that I described. Is this certain?

@ctrueden
Copy link
Member

It would be good though to report warnings about diverging versions even when SemVer promises compatibility

If we do that, there will be a flood of warnings in every case. You can verify this for yourself by importing fiji/fiji into Eclipse and looking for instances of versions that are "omitted"—there are always many.

There are also projects that do not follow SemVer, we would need to track a list of those.

True. It is probably sufficient to divide dependencies into two groups, based on groupId. The SciJava group IDs are considered SemVer, and others not necessarily. I am not intuitively sure of the best way to handle third party version convergence. I guess we could have a whitelist as you suggest. But it could get quite tricky. Also, SemVer is not required, just the major digit portion of it; see also here.

@hinerm hinerm self-assigned this Mar 23, 2015
@hinerm hinerm added this to the m1 milestone Mar 23, 2015
ctrueden pushed a commit that referenced this issue Nov 13, 2021
Cleanup & add target/test-classes to package cycle check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants