Support Maven <exclusions> syntax within target <dependency> elements

[vogella]

Closes #286

The m2-pde target location now accepts the standard Maven <exclusions>/<exclusion> syntax inside each <dependency> element, in addition to the existing location-level <exclude> elements.

<dependency>
    <groupId>org.junit.jupiter</groupId>
    <artifactId>junit-jupiter-api</artifactId>
    <version>5.9.3</version>
    <exclusions>
        <exclusion>
            <groupId>org.apiguardian</groupId>
            <artifactId>apiguardian-api</artifactId>
        </exclusion>
    </exclusions>
</dependency>

Exclusions are per-dependency (not global) and are applied during transitive dependency collection via MavenDependencyCollector, consistent with standard Maven semantics. The * wildcard is supported for groupId and artifactId. The format round-trips correctly through serialize()/parse.

[github-actions]

Test Results

  336 files  ± 0    336 suites  ±0   1h 11m 59s ⏱️ + 1m 29s
  718 tests + 4    698 ✅ + 2  16 💤 ±0  2 ❌ +1  2 🔥 +1 
2 154 runs  +12  2 102 ✅ +10  48 💤 ±0  2 ❌ +1  2 🔥 +1 

For more details on these failures and errors, see this check.

Results for commit bd2e9b0. ± Comparison against base commit 98b66b5.

♻️ This comment has been updated with latest results.

[laeubi]

I'm missing UI integration for this feature through the target location editor and the dependency tree.

Additionally we already have a way to exclude dependencies, so one would expect this code path to be enhanced instead of adding a complete new codepath.

[vogella]

I'm missing UI integration for this feature through the target location editor and the dependency tree.

Once the base support is integrated I can also provide the UI integration.

Additionally we already have a way to exclude dependencies, so one would expect this code path to be enhanced instead of adding a complete new codepath.

I check

[vogella]

The bad thing with re-using the same logic: Exclusions behave globally (the same as ) once an artifact is excluded by any dependency's , it's excluded from the whole target location. Let me know if you really prefer this, I think we should better follow the standard Maven logic (and maybe retire the other global exclude logic at some point).

[laeubi]

Once the base support is integrated I can also provide the UI integration.

Please include it here in the first place, the UI is the most important part for people to discover such functionality.

The bad thing with re-using the same logic: Exclusions behave globally (the same as ) once an artifact is excluded by any dependency's , it's excluded from the whole target location.

Would it not be quite confusing if one exclude dependency X from artifact Y and then it is still pulled in by Z?

I think we should better follow the standard Maven logic

m2e only follows the xml structure to make it easy to copy things from pom to target (via the clipboard) but the logic already works quite differently compared to maven. So we should more think what makes most sense here.

and maybe retire the other global exclude logic at some point

The general way in the past was to read old formats and transform them to the new format after saving (what also proves that a new feature is actually a super-set of the older one). So at best this would be possible here as well, what makes me lean more towards a location specific <excludes> than to one scoped to the dependency (what we more allowed to group items that belong together).

[vogella]

Please include it here in the first place, the UI is the most important part for people to discover such functionality.

Can do, once you give your go for the core implementation, I can extend the UI functionality.

So we should more think what makes most sense here.

So the new global logic is ok? Anything else in the core structure which should be changed? If not, I can start working on the UI.

[laeubi]

Can do, once you give your go for the core implementation, I can extend the UI functionality.

There is an (old) open issue about that and I think the general direction is sound - but only UI integration will show how well it works out. We later will need to have similar change for Tycho but this is a second step then.

So the new global logic is ok?

Currently the exclusion is per <dependency> as mentioned above I'm not sure it is really useful and might better be per target location. Maven do it per dependency because maven does conflict resolution (what m2e does not) and because of different scope chains (what m2e doe differently).

As mentioned the ultimate proof would be if you can transform a target using the old exclusion definition into the new more flexible form so people do not need to manually adjust their targets.

[vogella]

I think a migration would not work correctly, we have global entries which now should move to one entry in the list. Which one? First? Last? Any?

[laeubi]

I think a migration would not work correctly, we have global entries which now should move to one entry in the list. Which one? First? Last? Any?

Please see what was written above: I don't think individual exclusions are useful - e.g. what would be the use-case for an individual exclusion? We can use the same maven syntax but should retain the exclusion to be still per target location. If there is an immediate benefit we might want to allow adding things at both places.

In any case even for your current proposal the equivalent would be to add them to each individual entry consequently.

[vogella]

I think a migration would not work correctly, we have global entries which now should move to one entry in the list. Which one? First? Last? Any?

Please see what was written above: I don't think individual exclusions are useful - e.g. what would be the use-case for an individual exclusion?

Yes, we already agreed on this.

We can use the same maven syntax but should retain the exclusion to be still per target location. If there is an immediate benefit we might want to allow adding things at both places.

So the migration would be to move it to one of the entries?

[laeubi]

So the migration would be to move it to one of the entries?

I don't understand what you mean by entries? Maybe give an example (or update the PR) on how do you envision elements are placed and where do you think there is a conflict.

[vogella]

So the migration would be to move it to one of the entries?

I don't understand what you mean by entries? Maybe give an example (or update the PR) on how do you envision elements are placed and where do you think there is a conflict.

Example for entries:

<dependency>
    <groupId>org.junit.jupiter</groupId>
    <artifactId>junit-jupiter-api</artifactId>
    <version>5.9.3</version>
    <exclusions>
        <exclusion>
            <groupId>org.apiguardian</groupId>
            <artifactId>apiguardian-api</artifactId>
        </exclusion>
    </exclusions>
</dependency>

<dependency>
    <groupId>org.junit.jupiter</groupId>
    <artifactId>junit-jupiter-somethingelse</artifactId>
    <version>5.9.3</version>
</dependency>

[laeubi]

Example for entries

So how does this match to what you have written above:

We can use the same maven syntax but should retain the exclusion to be still per target location. If there is an immediate benefit we might want to allow adding things at both places.

So taking your example and transform it into a while target block would give (ignoring the fact that it sounds odd to exclude a version just to include it then in the other item)

<location includeDependencyDepth="infinite" includeDependencyScopes="compile" includeSource="%s" missingManifest="error" type="Maven">
	<dependency>
	    <groupId>org.junit.jupiter</groupId>
	    <artifactId>junit-jupiter-api</artifactId>
	    <version>5.9.3</version>
	    <exclusions>
		<exclusion>
		    <groupId>org.apiguardian</groupId>
		    <artifactId>apiguardian-api</artifactId>
		</exclusion>
	    </exclusions>
	</dependency>

	<dependency>
	    <groupId>org.junit.jupiter</groupId>
	    <artifactId>junit-jupiter-somethingelse</artifactId>
	    <version>5.9.3</version>
	</dependency>
</location>

so what I proposed here (and what would be 1:1 mappable to the previous concept)

<location includeDependencyDepth="infinite" includeDependencyScopes="compile" includeSource="%s" missingManifest="error" type="Maven">
	<dependency>
	    <groupId>org.junit.jupiter</groupId>
	    <artifactId>junit-jupiter-api</artifactId>
	    <version>5.9.3</version>
	</dependency>

	<dependency>
	    <groupId>org.junit.jupiter</groupId>
	    <artifactId>junit-jupiter-somethingelse</artifactId>
	    <version>5.9.3</version>
	</dependency>
	
    	<exclusions>
		<exclusion>
		    <groupId>org.apiguardian</groupId>
		    <artifactId>apiguardian-api</artifactId>
		</exclusion>
	</exclusions>
</location>

For bnd instructions we even allow to specify global item and local item (that in this case then might be merged for the individual entries:

<location includeDependencyDepth="infinite" includeDependencyScopes="compile" includeSource="%s" missingManifest="error" type="Maven">
	<dependency>
	    <groupId>org.junit.jupiter</groupId>
	    <artifactId>junit-jupiter-api</artifactId>
	    <version>5.9.3</version>
	    <exclusions>
		<exclusion>
		    <groupId>something</groupId>
		    <artifactId>else</artifactId>
		</exclusion>
	</exclusions>
	</dependency>

	<dependency>
	    <groupId>org.junit.jupiter</groupId>
	    <artifactId>junit-jupiter-somethingelse</artifactId>
	    <version>5.9.3</version>
	</dependency>
	
    	<exclusions>
		<exclusion>
		    <groupId>org.apiguardian</groupId>
		    <artifactId>apiguardian-api</artifactId>
		</exclusion>
	</exclusions>
</location>

[HannesWell]

ignoring the fact that it sounds odd to exclude a version just to include it then in the other item

I believe there are cases where it makes sense to exclude versions and include other ones, e.g. when one simply wants to override the version of a transitive dependency. Since Maven targets don't override dependencies like default Maven that can be wanted.

Furthermore, does excluding one artifact also excludes its (transitive) dependencies, if nothing else requires them?
I haven’t checked it, but I think that would be good.

[laeubi]

I believe there are cases where it makes sense to exclude versions and include other ones, e.g. when one simply wants to override the version of a transitive dependency.

Sure but not the same version what is done in the example.