Skip to content

Conversation

@cstamas
Copy link
Member

@cstamas cstamas commented Sep 17, 2025

Do not collect management data for scope and optional transitively, as those are "inherited" in graph (contextualized from parent).

Changes:

  • fixed transitive manager to properly handle "scope" and "optional" (only from root)
  • removed "transitive" capability from classic manager (added in 2.0.x), it was a mistake (along with UT)
  • documented in Javadoc what each do and how
  • fixed suppliers to supply what is really expected

Do not collect management data for scope and optional, as
those are "inherited" in graph (contextualized from parent).

These two properties are special, only root (POM) ones
should count.
@cstamas cstamas marked this pull request as ready for review September 17, 2025 20:42
@cstamas
Copy link
Member Author

cstamas commented Sep 17, 2025

To check, create this POM on disk:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
  <modelVersion>4.0.0</modelVersion>

  <groupId>org.apache.maven</groupId>
  <artifactId>transitive-reproducer</artifactId>
  <version>1.0-SNAPSHOT</version>
  <name>Transitive Reproducer</name>

  <properties>
    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
  </properties>
  
  <dependencies>
    <dependency>
      <groupId>org.apache.spark</groupId>
      <artifactId>spark-sql_2.13</artifactId>
      <version>4.0.1</version>
      <scope>provided</scope>
    </dependency>
  </dependencies>
</project>

And using Maven 4.0.0-rc-4 and patched (master + this PR) execute this command:

mvn toolbox:tree -Dscope=compile

The org.apache.spark:spark-sql_2.13:jar:4.0.1 [provided] direct dependency is in scope "provided", hence all children should be "provided" as well, but in 4.0.0-rc-4 they are not (bug).

To make Maven 4.0.0-rc-4 "behave", you must use -Dmaven.resolver.dependencyManagerTransitivity=false but then you lose transitive management completely.

@cstamas cstamas added this to the 2.0.12 milestone Sep 17, 2025
@cstamas cstamas added the bug Something isn't working label Sep 17, 2025
@cstamas cstamas self-assigned this Sep 17, 2025
cstamas and others added 5 commits September 19, 2025 11:42
Enhanced all dependency manager implementations with comprehensive documentation,
better code organization, and improved maintainability:

AbstractDependencyManager (base class):
- Restructured Javadoc with clear sections and HTML formatting
- Added helper methods to reduce code duplication in property management
- Implemented input validation with descriptive error messages
- Enhanced field and method documentation
- Improved code organization and maintainability

ClassicDependencyManager:
- Enhanced documentation explaining Maven 2.x compatibility behavior
- Detailed explanation of depth=1 "hop" and its critical importance
- Clear usage recommendations and comparisons with other managers
- Comprehensive method documentation for deriveChildManager override

DefaultDependencyManager:
- Added clear warnings about incompatibility with Maven/ModelBuilder
- Detailed explanation of aggressive management behavior
- Clear guidance on when to use vs. when NOT to use
- Enhanced constructor documentation with behavior warnings

TransitiveDependencyManager:
- Marked as recommended manager for modern Maven usage
- Detailed explanation of ModelBuilder compatibility
- Comprehensive documentation of inheritance handling logic
- Enhanced isInheritedDerived() method documentation

NoopDependencyManager:
- Clear documentation of use cases and thread safety guarantees
- Enhanced singleton pattern documentation
- Detailed method documentation explaining no-op behavior
- Performance characteristics explanation

Key improvements:
- Reduced code duplication through helper methods
- Added parameter validation with meaningful error messages
- Consistent documentation structure across all implementations
- Clear manager selection guidance for developers
- Cross-references between related classes
- Enhanced developer experience with usage recommendations
@cstamas cstamas merged commit 7a384ca into apache:master Sep 19, 2025
19 of 20 checks passed
@cstamas cstamas deleted the inherited-managed branch September 19, 2025 13:10
cstamas added a commit to cstamas/maven-resolver that referenced this pull request Sep 19, 2025
Do not collect management data for scope and optional transitively, as those are "inherited" in graph (contextualized from parent).

Changes:
* fixed transitive manager to properly handle "scope" and "optional" (only from root)
* removed "transitive" capability from classic manager (added in 2.0.x), it was a mistake (along with UT)
* documented in Javadoc what each do and how
* fixed suppliers to supply what is really expected
@cstamas cstamas mentioned this pull request Sep 20, 2025
cstamas added a commit that referenced this pull request Sep 20, 2025
PR #1581 left some typos in, that makes javadoc puke:
```
[ERROR] /home/cstamas/Worx/apache-maven/maven-resolver/maven-resolver-test-util/src/main/java/org/eclipse/aether/internal/test/util/TestFileUtils.java:161: error: unterminated inline tag
[ERROR]      * @deprecated use {@code @tempdir) (JUnit 5) Or {@code TemporaryFolder} (JUnit 4) instead
```

PR #1589 used forbidden chars.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants