Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

liquibase-gradle-plugin should declare picocli as a transitive dependency #129

Open
mr-serjey opened this issue Aug 11, 2023 · 9 comments
Open

Comments

@mr-serjey
Copy link

This issue is my reaction to the following note from README.md...

IMPORTANT: Liquibase now uses the picocli library to parse options, but for some reason that library isn't a transitive dependency of Liquibase itself, so if you want to use this plugin with Liquibase 4.4.0+, you'll have to add the liquibaseRuntime 'info.picocli:picocli:4.6.1' dependency to your build.gradle file.

Liquibase supports multiple integration interfaces defined in liquibase-core.jar in liquibase.integration package:

  • ant
  • commandline
  • servlet
  • spring

I believe, developers of liquidate-core expected the consuming projects to define necessary dependencies for the integration type of consumer's choice, rather than providing them transitively. (e.g. I would be surprised to receive picocli, spring and servlet api from liquibase-core).

So I believe since liquibase-gradle-plugin has integration via commandline interface, it is on 'liquibase-gradle-plugin' to provide necessary dependencies (e.g. picocli) as transitive dependencies.

I would recommend to define picocli dependency in liquibaseRuntime on the plugin level, or adopt ant integration interface.

@stevesaliman
Copy link
Collaborator

Your explanation of why Liquibase doesn't have picocli as a transitive dependency makes sense. Why bring it if you don't need it?

The gradle plugin didn't bring picocli in as a transitive dependency for a similar reason. The plugin isn't tied to any particular version of Liquibase, and there is no need to bring in a dependency if you're not using a version of Liquibase that requires it.

At this point, I think enough people have moved to a new enough version of Liquibase that it might make sense to just have it, but which version should it include? the plugin still doesn't know what version of LB you want, so it also doesn't know what version of picocli it depends on.

@mr-serjey
Copy link
Author

mr-serjey commented Sep 11, 2023

I believe ideally the first move should be done on liquibase-core side:

  1. Following best practices the liquibase-core should expose optional dependency to an appropriate version of picocli (they even considered that here)
  2. Alternatively the should start publishing liquibase-cli into a public maven repository (together with the rest publications)
  3. Or create and publish liquibase BOM

Here is the issues that people already filed in support of these ideas:

If/when these changes done, the changes in liquibase-gradle-plugin are straight forward.

I can think of few alternatives, but they require the maintenance of the picocli to liquiba-core version mapping on liquibase-gradle-plugin side (which is far from ideal, but probably can be considered to support already published versions of liquibase-core)

@landsman
Copy link

The mention of another dependency in the documentation also surprised me. I would like to have it straight as a liquibase-gradle-plugin dependency and not really worry about it.

@stevesaliman
Copy link
Collaborator

The challenge is that the liquibase-gradle-plugin doesn't know what version of Liquibase you want to use, so it also doesn't know what version of picocli it needs, or even if it is needed.

Maintining mappings of Liquibase releases to the version of picocli needed means that I'd potentially need a new release of the plugin each time Liquibase had a new release - or at least whenever a new version of picocli was used. Less than ideal.

@landsman
Copy link

But you are talking about versions where there is some BC break, right? It's still a much better solution than letting this effort to users, to be honest.

@filipelautert
Copy link

@stevesaliman from the discussion here, I believe PR liquibase/liquibase#5042 that will be included in the next liquibase release and adds picocli as an optional dependency could help on that.

@filipelautert filipelautert self-assigned this Nov 6, 2023
@filipelautert
Copy link

@stevesaliman as per Florent closing comment on his own request (liquibase/liquibase#2751 (comment) ) , seems that replacing liquibase-core by liquibase-cli as a dependency resolves this. What do you think?

@stevesaliman
Copy link
Collaborator

The thought that comes to my mind is this: Does liquibase-cli include the classes from liquibase-core? The plugin doesn't necessarily need them, but the Groovy DSL will in order to parse groovy changes.

But if the cli includes the core, that would be a good solution for the plugin.

@filipelautert
Copy link

@stevesaliman liquibase-cli depends on liquibase-standard (that is core minus snowflake) . Pom is here: https://github.com/liquibase/liquibase/blob/master/liquibase-cli/pom.xml

But I just noticed that we don't publish liquibase-cli to maven repo... so until that is changed it won't help :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Development PR Issues
Development

No branches or pull requests

5 participants