Aiven-Open / tiered-storage-for-apache-kafka

RemoteStorageManager for Apache Kafka® Tiered Storage
Apache License 2.0
91 stars 19 forks source link

chore: re-enable deps validation #442

Open jeqo opened 10 months ago

jeqo commented 10 months ago

Re-enable custom task to validate storage dependencies.

As a reminder this task collects dep versions used by each storage module, and compares it against versions used by core. It it contains the same dep but versions do not match, then fail build.

e.g. running the first commit on storage:gcs and azure reports conflicting libraries that are fixed in the following commits:

> Task :storage:gcs:validateDeps FAILED
Conflict found for checker-qual: 3.42.0 vs 3.37.0
Conflict found for error_prone_annotations: 2.24.1 vs 2.21.1

> Task :storage:azure:validateDeps FAILED
Conflict found for jackson-annotations: 2.13.5 vs 2.16.1
Conflict found for jackson-core: 2.13.5 vs 2.16.1
Conflict found for jackson-databind: 2.13.5 vs 2.16.1
AnatolyPopov commented 6 months ago

What about just using

configurations.configureEach {
    resolutionStrategy {
        failOnVersionConflict()
    }
}

?

jeqo commented 6 months ago

@AnatolyPopov if you reproduce the first commit in this PR, the following conflicts are found:

> Task :storage:azure:validateDeps FAILED
Conflict found for jackson-annotations: 2.13.5 vs 2.16.1
Conflict found for jackson-core: 2.13.5 vs 2.16.1
Conflict found for jackson-databind: 2.13.5 vs 2.16.1

FAILURE: Build failed with an exception.

* Where:
Build file '/mnt/aiven-home-dir/aiven/aiven-open/tiered-storage-for-apache-kafka/storage/build.gradle' line: 62

* What went wrong:
Execution failed for task ':storage:azure:validateDeps'.
> Dependency conflicts found!. Expression: conflictsFound. Values: conflictsFound = true

This is unfortunately not found by your suggested configuration. IIUC, as the storage modules are not dependent on core then the collision of libraries between these are not found by default tooling--hence the custom script.

AnatolyPopov commented 6 months ago

@jeqo What's the scope of the dependencies you listed? And because of reversed dependecies those conflicts should be visible when you run :core:dependencies then

jeqo commented 6 months ago

What's the scope of the dependencies you listed?

It's comparing dependency versions from runtimeClasspath.

And because of reversed dependecies those conflicts should be visible when you run :core:dependencies then

I'm running core:dependencies with the configuration provided before and still I can't observe the same validation. Could you provide a complete example with an out-of-the-box command that reports the same issues on this project? As an reference I added the output of running: ./gradlew storage:gcs:validateDeps storage:azure:validateDeps --continue on the first commit in the PR description.

AnatolyPopov commented 6 months ago

Well, after looking into this more deeply I can say that default tooling does not find the conflict because there is no dependencies between core module and submodules of storage. However I believe we should have such a dependency for this use case specifically because the storage can not be used with core. The ideal scope for such a dependency would be maven's provided I guess but there is no such thing in Gradle since we are not using war plugin. So IMO the solution should be:

  1. Use the default tooling as I proposed before
  2. Create a providedRuntime scope similar to Grade war plugin e.g.
    configurations {
    providedRuntime
    implementation.extendsFrom(providedRuntime)
    }
  3. Add a dependency in :storage:core like this providedRuntime(project(":core"))

This kind of scope definition will not change the resulting distribution, e.g. :core dependencies will not be included into artifacts of submodules of :storage but it will make the tooling work properly. And I guess for connectors or any other similar plugins people are doing similar things. And same for web apps, since the container provides some libraries there.

AnatolyPopov commented 6 months ago

@jeqo Please take a look https://github.com/Aiven-Open/tiered-storage-for-apache-kafka/pull/518