Indirect static required modules should stay on classpath

[rfscholte]

Based on https://issues.apache.org/jira/browse/SUREFIRE-1667 it makes sense to have 2 different types of resolution. By doing this on the LocationManager you can use the request for the different resolution results.

[sbordet]

@rfscholte these changes are an improvement, but not quite there yet.

Your changes fixed the transitive dependency due to requires static - now they are not anymore in the module-path.

However, there is another case:

moduleA {
  exports a;
  requires moduleB;
  requires static moduleD;
}

moduleB {
  exports b;
  requires static moduleC;
}

moduleC {
  exports c;
}

moduleD {
  exports d;
}

Before your changes, the module-path was: moduleA:moduleB:moduleC:moduleD and the problem was that moduleC should not have been there.

With your current changes the module-path is: moduleA:moduleB and the problem is that moduleD is now missing.

The module-path should really be: moduleA:moduleB:moduleD.

So the check on requires static should only be done for transitive dependencies, not for direct dependencies.

I believe that this is already what is being done with Maven's optional=true: the optional dependency is added to the class-path if it is a direct dependency, but not added if it is a transitive dependency.

Makes sense?

[rfscholte]

I've added another unittest reflecting your testcase and according to its result it behaves as expected.
Just know that the LocationManager is not aware of Maven metadata such as optional, it builds its graph based on module descriptors, manifest files and filenames only.

[sbordet]

@rfscholte you are right that the current behavior is correct - I have mistakenly assumed it was not because I had a failure, but further investigation shows that the problem is mine.

For sake of discussion, my case is the following:

moduleA {
  exports a;
  requires moduleB;
  requires static moduleD;
}

moduleB {
  exports b;
  requires static moduleC;
}

moduleC {
  exports c;
}

moduleD {
  exports d;
  requires transitive moduleE;
}

When running the tests with Surefire, I now have a correct module-path with moduleA:moduleB:moduleD and a classpath with moduleC (which is explicitly declared as a test dependency in the POM), but I get an error:
java.lang.IllegalAccessError: moduleA does not read moduleE

The solution is to configure Surefire with --add-modules moduleD, making it explicit now that the module graph must have moduleD and therefore transitively moduleE.
This is necessary because moduleA requires static moduleD, so moduleD is not part of the module graph due to the static modifier (despite being in the module-path).

I think your changes are good. Thanks!