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

Provide Liquibase jar without packaged dependencies and pro packages #2629

Closed
filiphr opened this issue Mar 10, 2022 · 8 comments · Fixed by #3050
Closed

Provide Liquibase jar without packaged dependencies and pro packages #2629

filiphr opened this issue Mar 10, 2022 · 8 comments · Fixed by #3050

Comments

@filiphr
Copy link

filiphr commented Mar 10, 2022

We at the open-source project flowable-engine are heavy users of Liquibase. We use it as a library. Lately we have noticed that the liquibase-core JAR is getting bigger and bigger.

  • 4.5.0 - 3.8 MB
  • 4.6.0 - 6.2 MB
  • 4.6.1 - 6.2 MB
  • 4.6.2 - 6.3 MB
  • 4.7.0 - 6.3 MB
  • 4.7.1 - 6.3 MB
  • 4.8.0 - 6.4 MB

We did a comparison between 4.5.0 and 4.8.0 and I saw the following dependencies packaged within:

  • JSQLParser
  • Apache Commons Collection4
  • Apache Commons Text
  • Apache Commons Lang3
  • OpenCSV Parser (Seems like this was already there)

We could only find imports from OpenCSV Parser in liquibase-core. The rest of the dependencies are not used, they are not even defined as dependencies in the pom.xml.

We understand that you are doing extra steps to include the classes needed for Liquibase Pro in liquibase-core. This is entirely OK and we understand what it means to have a business around an Open Source project.

We are big fans of Liquibase and we would like to ask you to consider releasing something that can be used as a library without the need for the pro classes and other dependencies. For example the current liquibase-core as it is in GitHub to be released as a new artifact. Perhaps liquibase-base and then the liquibase-core jar will have a dependency on liquibase-base and it will also contain the pro classes together with the other dependencies that you are using.

@kataggart
Copy link
Contributor

@filiphr Thanks for this feedback and suggestion! We are in the process of evaluating various aspects of our build and release process, so your input is well timed. I can't promise a timeline, but will definitely commit to reviewing and discussing your feedback here. Stay tuned.

@nvoxland
Copy link
Contributor

Yes, those additional packages we shaded in are the main thing driving the increase in size.

We had previously been including OpenCSV but it was a copied over subset of classes and was getting very out of date. So rather than dealing with that copy/paste that's hard to manage we brought it in as a complete dependency. The collection4, text, and lang3 depenedencies are sub-dependencies of opencsv. So they come from that, not from anything in the pro code.

We could look at using proguard to include just the OpenCSV classes we actually use rather than a simple "shade", I hadn't bothered with it thinking that is just adding a couple MBs which would be fine.

But, I'd be interested to hear if/when you're hitting issues with the extra size being a problem. That would help us better understand how to work through possible packaging options.

@filiphr
Copy link
Author

filiphr commented Mar 11, 2022

OK I didn't know that the other dependencies are OpenCSV dependencies. This means that OpenCSV is really heavy. My next question is then, is OpenCSV a mandatory dependency? I see that it is used in the OfflineConnection, which sounds something like opt in, and also in the DiffOutputControl, which is used in a lot of places, and I am not sure how that fits into things in Liquibase.

But, I'd be interested to hear if/when you're hitting issues with the extra size being a problem. That would help us better understand how to work through possible packaging options.

We use Liquibase as a library, and we build a library as well. This means that our users pull our dependencies. We try to not bring large dependencies with a lot of unnecessary 3rd party dependencies. On top of that, the fact that you are shading OpenCSV and its dependencies, which are really common dependencies in 3rd party application means that users when trying to use CollectionUtils, StringUtils will get suggestions from both your repackaged packages and also the actual dependencies. This is not the most user friendly developer experience.

What I would suggest is perhaps to roll your own CSV writer, in the end what you need is a tiny fraction of what OpenCSV offers. From what I can see OpenCSV has a lot of complex things for writing objects to to CSV.

In any case having a pro free dependency is another good thing.

One other thing that helps around the size is also the speed of Liquibase. You are heavy users of the Service Locator pattern, which is a really nice thing. However, this together with the pro code means that a lot of times there are things loaded which are not going to be used, because no pro features are being used.

In any case, my suggestion is not to change liquibase-core, because that would make it more complicated for your current user. I am suggesting adding a new module, that liquibase-core will then transitively pull and would enable users like us to use it.

@kdebski85
Copy link

kdebski85 commented Mar 18, 2022

Why cannot liqubase-core just have Maven dependency on OpenCSV?
I already have Common Collection4, Text and Lang3 in my application.
After Liquibase upgrade, I have two copies of them. If they were just normal dependencies, I would have one copy.
Also, creating separate liquibase-pro is a good idea.

I currently use 4.3.5. It had 3.3 MB, which was already quite big.
6.5 MB for 4.9.0 is too much.

@kataggart kataggart added this to To Do in Conditioning++ via automation Jun 2, 2022
@nvoxland
Copy link
Contributor

nvoxland commented Jun 2, 2022

We are currently working on that. I actually opened #2903 last night that switches opencsv to be a maven dependency vs. shaded in. There is a separate project going that will make a new liquibase-commercial.jar which has the paid add-ons vs. including that in liquibase-core. The dependencies that come from that will also be pulled out to separately managed jars vs. being shaded in as well.

The split of the pro code takes a bit more build-time coordination work so is more complex, but we're hoping to wrap that up in the near future.

@filiphr
Copy link
Author

filiphr commented Jun 2, 2022

This are some really great news @nvoxland. I am looking forward in consuming this new jars from Liquibase.

Regarding opencsv dependency, is this something that is mandatory or is it an optional thing that is only needed in the CLI? It would be good if we can reduce the liquibase-core.jar to only contain what is needed to consume Liquibase as a dependency, and have a liquibase.jar or liquibase-cli.jar that would be consumable as a CLI.

@filiphr
Copy link
Author

filiphr commented Jul 25, 2022

Thanks a lot for doing this changes to your releases. It helps a lot in using Liquibase as a library in an another open source project.

@kataggart
Copy link
Contributor

@filiphr you got to it first!!! I was so excited to come tell you about it! https://www.liquibase.com/blog/two-jars-beat-as-one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants