Provisioner can provide dependencies without their transitives

[fvgh]

Extended Provisioner to provide dependencies either with or without their transitives.

Changed EclipseBasedStepBuilder to request dependencies only without transitives.
Removed work-around from Eclipse dependency lock-files.

The problem had been spotted when switching Groovy-Eclipse to new version supporting Eclipse-Base with the dependency lock.

Please consider this change as an implementation proposal. It is relatively verbose, since I tried to keep the public interfaces unchanged and backward compatible as discussed previously. Maybe this approach can be considered unnecessary for the Provisioner.

[fvgh]

@lutovich FYI: I made a small change to the CollectRequest call. In #296 I passed the dependencies as "managed dependencies". For Spotless it does not matter, but it was the wrong interface, so I corrected it.
I don't want to force you to do a review. If you have time, you are welcome...

[fvgh]

@nedtwigg I think the major issue is to find good method names and decide how far to go about preserving public interfaces. Feel free to make changes or let me know what to do. But I think you have a broader view on these things...

[nedtwigg]

How about this:

public interface Provisioner {
  /** Method interface has been extended to {@link Provisioner#provide}. */
  public Set<File> provisionWithDependencies(Collection<String> mavenCoordinates);
  /** Method interface has been extended to {@link Provisioner#provide}. */
  public default Set<File> provisionWithDependencies(String... mavenCoordinates) {
    return provisionWithDependencies(Arrays.asList(mavenCoordinates));
  }
  /** Method interface has been extended to {@link Provisioner#provide}. */
  public Set<File> provisionWithoutDependencies(Collection<String> mavenCoordinates);
  /** Method interface has been extended to {@link Provisioner#provide}. */
  public default Set<File> provisionWithoutDependencies(String... mavenCoordinates) {
    return provisionWithoutDependencies(Arrays.asList(mavenCoordinates));
  }
}

public class JarState {
    public static JarState from(..)
    public static JarState fromWithoutTransitives(..)

Advantages:

• we don't have to deprecate anything
• provide(true, 'some:artifact:1.0.0') and provide(false, 'some:artifact:1.0.0') is harder to read than provisionWithDependencies('some:artifact:1.0.0') and provisionWithoutDependencies('some:artifact:1.0.0')

You can still implement them with a private Set<File> provide(boolean transitives, Collection<String> artifacts).

I don't foresee a major version bump anywhere in the near or distant future - there's no big technical debt that I see, there's a lot of value in minimizing the amount of infrastructure that we deprecate.

[fvgh]

@nedtwigg How would you do the Provisioner implementation? Currently lambdas are used in many places and method references. I just did not want to alter that concept. Shall I just replace the lambdas by anonymous classes for now?

[nedtwigg]

I think you could use your same CompatibleProvisioner trick.

private static interface CompatibleProvisioner extends Provisioner {
  @Override
  @Deprecated
  default Set<File> provisionWithDependencies(Collection<String> mavenCoordinates) {
    return provide(true, mavenCoordinates);
  }

  /** Method interface has been extended to {@link Provisioner#provide}. */
  @Override
  @Deprecated
  default Set<File> provisionWithoutDependencies(Collection<String> mavenCoordinates) {
    return provide(false, mavenCoordinates);
  }
 
  Set<File> provisionWithTransitives(boolean withTransitives, Collection<String> mavenCoordinates);
}

But as usual, I missed a valuable property of your existing work, which is that you do throw new UnsupportedOperationException("Provisioner: provide without transitives"); for any client implementations that might be out in the wild. My suggestion above works transparently for callers of Provisioner, but it will create compile-errors for implementors of Provisioner, which your original implementation carefully turned into UnsupportedOperationException.

So I missed an important part of what you were adding. Adding the no-transitives capability:

• is required for your super-valuable eclipse-based suite of formatters
• is a backward-incompatible change for existing implementations of Provisioner, of which we are almost definitely the only implementors
  • we can paper-over with UnsupportedOperationException if we want to
• is backward-compatible for existing callers of Provisioner, but even if it wasn't it's mostly wrapped up by JarState anyway

For this case, I think it's okay to break only the implementations of Provisioner - according to mavencentral usages there aren't any (I control goomph and image-grinder). However, I'd prefer to not paper over it with UnsupportedOperationException. I also think that we should @Deprecate some of JarState too.

So I think your original approach is the best one - apologies for misunderstanding some of the reasoning :) I have a few suggestions that are easiest to describe by pushing code, I'll push in 5 mins.

[nedtwigg]

I'm done with my suggestions. Same idea as your original PR, except:

• legacy implementations of Provisioner fail at compile-time rather than runtime
  • allows the change to be quite a bit smaller
• no opaque boolean parameters - the method name always hints at its meaning

If this looks good to you, feel free to merge.

[nedtwigg]

When this gets merged, let's not squash it. I think the discussion here was helpful, I'd like to keep it.

[fvgh]

@nedtwigg Looks good. Fixed only some minor things in JavaDoc. Thanks for your help.