palantir / sls-packaging

A set of Gradle plugins for creating SLS-compatible packages
Apache License 2.0
39 stars 74 forks source link

Provide an escape hatch for not including automatically discovered product dependency ranges #523

Open mglazer opened 5 years ago

mglazer commented 5 years ago

What happened?

I'd like for there to be some sort of escape hatch so that I can explicitly ignore automatic product dependency version determination.

I have a product dependency which is version 3.53.0, however, I know that my application is fully compatible with the version 2.70.0 version of that product's wire interface. Therefore, I want to be able to have a dependency declaration automatically fill in the range of:

[2.70.0, 4.x.x)

However, at present, I cannot do that.

What did you want to happen?

I want some form of escape hatch so that I can say that my actual service dependencies are a larger range than the declared library dependency.

iamdanfox commented 5 years ago

The core value proposition of sls-packaging 3.X is that it's impossible for devs to shoot themselves in the foot by specifying overly permissive ranges of product dependencies like this.

I can see how in your case, if you depend on a new MP jar but only use a couple of endpoints, then technically lowering this product dep would be safe (right now), but we wouldn't be able to guarantee this for all future cases.

In this situation, I normally recommend people to downgrade the jar that is providing these ranges (i.e. 3.53.0 -> 2.70.0), forcing if necessary. If I remember correctly though, that didn't work in your case because it had a too low upper range (2.X.X instead of 3.X.X??)

markelliot commented 5 years ago

There is a release of that (internal) dependency that has a correct range [2.70.x, 3.x.x); ping me over Slack and I think I can point you at it.

mglazer commented 5 years ago

Hey all:

I just want to provide a bit of extra flavor as to why these other proposed options do not work.

The special internal snapshot version that includes the full range

This works ok for some products. However, it starts to break down as soon as you start running into conflicting dependencies between the other libraries which this dependency brings in, and modern libraries that you intend to use. The problem is that I want to depend on MP 3.x, the API, but by doing so, I also have to depend on MP 3.x the Java library, which has a number of conflicting Java dependency conflicts.

These manifest as either runtime or compile time breaks (more often than not, compile time breaks).

Depending on MP 2.x

This doesn't work for mostly the same reason as the special snapshot version. The library dependencies conflict.

I have tried a few different options in Gradle all of which haven't quite worked. Specially, I tried specifying a special configuration just for the sls-packaging dependency resolution so that I could specify the MP API version for that part of the build process, but not for the actual distribution bundling portion of the build process, but that failed. I believe that it also caused significant problems when used with GCV since that format doesn't allow for different versions being used in different configurations.

The major problem here is that my product is taking an API dependency on the MP 2.x product, the APIs which my product (this is actually a number of products this affects so I wouldn't be filing this for a single product), use are only within same set that MP 2.x supported. I have enough unit and integration tests to validate that everything works against an older MP 2.x (when running in integration tests). So let me product declare that it depends on an older wire API, even if the library dependencies it takes might be higher.

markelliot commented 5 years ago

I think the fix here is to update the API version that's bad to have improved dependencies. If you pick a version greater than the one that bridges the major versions, we cannot protect you from actually depending on the lower of the major versions, because you may unwittingly call endpoints that are not in fact supported on the version you purport to access.

mglazer commented 5 years ago

So make a fix to MP 3.0.0-rc26402 such that I bump all of the dependencies to something more reasonable and then I ensure all of my products depend on MP 3.0.0-rc26403?

markelliot commented 5 years ago

Something like that? Our general practice is that API jars have ~0 dependencies, so the main thing I’d like to understand is why that’s not true in this case.

mglazer commented 5 years ago

The API JAR itself doesn't have dependencies, but if I want to declare in my versions.props:

com.palantir.mp:* = 3.50

I'd actually have to break it out to have:

com.palantir.mp:mp-api = 2.70.0
com.palantir.mp:mp-implementation = 3.50

But that just feels like a worse solution here.

markelliot commented 5 years ago

actually it's pretty standard for us to break that kind of thing out so that the API dependency is clear.

mglazer commented 5 years ago

OK, I just tried doing that by adding the following to my versions.props:

com.palantir.mp:mp-api = 3.0.0-rc26402

(along with some other ones, which were bringing in the MANIFEST.MF specifying this version range)

I then restricted my dependency block so I wasn't bringing in MP 2.60:

    productDependency {
        productGroup = "com.palantir.mp"
        productName = "mp"
        minimumVersion = '2.70.24'
        recommendedVersion = '2.70.24'
        maximumVersion = '3.x.x'
    }

and it worked, even though it feels messy. I'll live with it though.