niloc132
commented
2 months ago In general in Maven it's allowed
You're conflating Maven and java - in Maven with the j2cl-maven-plugin this is allowed too, and while Java handles duplicates on its classpath very gracefully, ClosureCompiler does not.
You're also mixing "dependency" and "class" - listing a single dependency, even with different versions, multiple times is managed the way you would expect with Maven for a Java project too - Maven will decide which version of that groupId+artifactId to use (default strategy is to pick the definition "closest" to your project, which is sort of defined as "sort the various dependency decls by distance from the project being built, and use the first"), and will not include the others. You can see this when reviewing mvn dependency:tree -Dverbose and looking for duplicates and conflicts). But the issue you're encountering here isn't around the dependency being listed twice, but two distinct dependencies (groupId+artifactId+classifier) containing the same resource. Maven doesn't care about this, only thinks about dependencies, and Java just picks the "first" one it encounters on the classpath. J2CL and Javac/turbine also don't care, they don't mind duplicates, they just pick the first. But it is explicitly an error in closure to have duplicate declarations of closure modules or provides.
With all of that said, if this fix resolves your bug, it sounds like you have two classifiers but the same groupid+artifactid for some dependency, and, most importantly, that these artifacts contain the same .java (or potentially .js) files.
If that is the case, it probably is correct for your project to just use <excludes> to keep one off the final classpath, so that closure only sees one set of them (and so you don't transpile them twice, and muddy up the classpath for anything else that depends on them).
And if so, this could break cases where "classifier" is used to mean "this artifact is different from that one for some important reason" - imagine a library that ships with an optional shim, and you can add the classifier to opt-in to using the shim. Not saying someone is necessarily using it this way, but it is possible at least, and it would break that case (without a clear rule about "the first one wins" or how to define which classifier beats other classifiers).
But this fix will not solve the "bug" where two distinctly named dependencies contain the same class or js file, which seemed closer to what the original fix was intended to do (and would have solved). Can we take a few steps back and look at the problem rather than jumping to solutions? If an excludes (or possibly even "just don't add that extra classifier=sources dep") resolves this, it would seem simpler...
salmonb
The issue I'm facing is that after the dependency replacements have been applied, the effective project has some final dependencies listed multiple times.
The Vertispan plugin doesn't manage this case properly. In general in Maven it's allowed to have the same dependency listed multiple times in a pom.xml, even if it's not recommended, it's not a fault and this shouldn't break the build. And in particular if that pom is an effective pom generated by a tool (even if that effective pom is just virtual in memory) it should be even more relevant to not break the build.
I moved the
distinct()operator to an upper level, in theTaskFactory.scope()method, and this solves this issue in a cleaner way than my initial proposition. Indeed, in which case would it make sense to have duplicates in the list of Project returned by this method?Can you please reconsider my PR?