mybatis / migrations

A command line Database migrations tool
http://mybatis.github.io/migrations
Apache License 2.0
214 stars 77 forks source link

Please make smaller; minimize dependencies #259

Closed jwaterloo2020 closed 1 year ago

jwaterloo2020 commented 2 years ago

mybatis-migrations is just 119KB but it has a dependency upon mybatis which adds 1.7MB and that doesn't even include its transitive dependencies. The only classes directly used from mybatis are Resources and ResolverUtil. Those classes are only dependent upon classes within the same org.apache.ibatis.io package. This means a self-contained or with minimized dependencies should just be under 150KB. Please either copy the package or create a new artifact that both projects could depend upon. There are only 9 classes in the org.apache.ibatis.io package and they don't depend upon anything else in mybatis other than logging. The logging dependency could just be replaced with JUL, SLF4J or be made into its own artifact.

migrations\src\main\java\org\apache\ibatis\migration\commands\BaseCommand.java import org.apache.ibatis.io.Resources; migrations\src\main\java\org\apache\ibatis\migration\JavaMigrationLoader.java import org.apache.ibatis.io.ResolverUtil; import org.apache.ibatis.io.ResolverUtil.Test;

package org.apache.ibatis.io;

import org.apache.ibatis.logging.Log; import org.apache.ibatis.logging.LogFactory;

hazendaz commented 2 years ago

Mybatis does include far more than it should such as logging wrapper, shade of ognl and javassist, so it has no external libs. That number is the entire thing. Taking out the shaded items, its 643kb. Additionally, I've always been against wrapping logger frameworks and with slf4j major rewrite handing more doesn't seem right and detrimental in many regards. Not to mention now it has a lot of legacy no one should use. I think those type of situations are simply vestiges of years gone by where releases were very infrequent. Releases are still infrequent for mybatis and that should change. And progress towards just that is slowly happening.

Doing those things would have an immediate impact on its need for sure but reduction would only be in getting rid of ognl / javassist and unnecessary logging support. Such an issue really belongs on the core project and I do know its been discussed not including such things in the past but never got traction. Its more of a discussion through discussion tab though as not really an issue. Getting to that point, sure you end up defining your own logging then, that's not saving much really other than bindings here. And sure you select a proxy if needed, so only ognl or javassist. Ultimately there is still more to consider. Does it go modular and break mybatis into many more pieces depending on usage, maybe.

As to how its used throughout, mybatis core is the baseline for all the other projects. Maybe it is true some are not so much addons and maybe only use very small portions such as this, but I have to ask, what does 2mb size really save you? Its not really huge, comparing it to other frameworks used (thinking of guava), many are much larger and already shown by its bloated here. These days I'm not seeing what that benefit might be of being smaller. I agree a lot needs to change but really this needs discussed on the core as your ask is really to change that. On here, its more of want a smaller size but what is the concern there? Is it worth the development effort to save sizing here, does it really make sense? What is your use-case in how this works? Do you use maven plugin for it by chance? If so, is there no concern for fact large number of libraries for maven for example get downloaded that far outweigh this?

jwaterloo2020 commented 2 years ago

For the most part mybatis migrations does not need mybatis it works with DataSource really well. Mybatis does have a security issue which mibatis migrations does not need. Mybatis migrations, due to its potential small size, is a much better option than liquibase that does not support rollback in unpaid version and liquibase which is architecturally inconsistent. I have used all three. ... With a few minor but major release changes it could easily rival Roby on Rails for being the best migration at least in the Java world. Some of the core interfaces need to return List of String scripts instead of a single String script in the core interfaces and let migration totally take over delimiters. Also should be more composable with automatic rollback generated from the do actions. The latter anyone can build themselves but size (modularity) and delimiter issues really need to be fixed on the mybatis side of things. ... If you can show how to shade mybatis migrations into ~150KB jar would be great. I don't shade that often cause it is dangerous. Also wouldn't hurt if a shaded jar was put into maven repository as a common deployment option.

jwaterloo2020 commented 2 years ago

I haven't tested it but shaded it is 173KB. Thank you for the suggestion. Hope it still works!

<?xml version="1.0" encoding="UTF-8"?>

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">

4.0.0
<groupId>org.mybatis</groupId>
<artifactId>mybatis-migrations-shaded</artifactId>
<version>3.3.11</version>

<name>mybatis-migrations-shaded</name>
<url>http://www.example.com</url>

<properties>
    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
    <maven.compiler.source>11</maven.compiler.source>
    <maven.compiler.target>11</maven.compiler.target>
</properties>

<dependencies>
    <dependency>
        <groupId>junit</groupId>
        <artifactId>junit</artifactId>
        <version>4.11</version>
        <scope>test</scope>
    </dependency>
    <dependency>
        <groupId>org.mybatis</groupId>
        <artifactId>mybatis-migrations</artifactId>
        <version>3.3.11</version>
    </dependency>
</dependencies>

<build>
    <pluginManagement><!-- lock down plugins versions to avoid using Maven 
            defaults (may be moved to parent pom) -->
        <plugins>
            <plugin>
                <artifactId>maven-clean-plugin</artifactId>
                <version>3.1.0</version>
            </plugin>
            <plugin>
                <artifactId>maven-resources-plugin</artifactId>
                <version>3.0.2</version>
            </plugin>
            <plugin>
                <artifactId>maven-compiler-plugin</artifactId>
                <version>3.8.0</version>
            </plugin>
            <plugin>
                <artifactId>maven-surefire-plugin</artifactId>
                <version>2.22.1</version>
            </plugin>
            <plugin>
                <artifactId>maven-jar-plugin</artifactId>
                <version>3.0.2</version>
            </plugin>
            <plugin>
                <artifactId>maven-install-plugin</artifactId>
                <version>2.5.2</version>
            </plugin>
            <plugin>
                <artifactId>maven-deploy-plugin</artifactId>
                <version>2.8.2</version>
            </plugin>
            <plugin>
                <artifactId>maven-site-plugin</artifactId>
                <version>3.7.1</version>
            </plugin>
            <plugin>
                <artifactId>maven-project-info-reports-plugin</artifactId>
                <version>3.0.0</version>
            </plugin>
        </plugins>
    </pluginManagement>
    <plugins>
        <plugin>
            <groupId>org.apache.maven.plugins</groupId>
            <artifactId>maven-shade-plugin</artifactId>
            <version>3.3.0</version>
            <executions>
                <execution>
                    <phase>package</phase>
                    <goals>
                        <goal>shade</goal>
                    </goals>
                    <configuration>
                        <minimizeJar>true</minimizeJar>
                        <filters>
                            <filter>
                                <artifact>org.mybatis:mybatis-migrations</artifact>
                                <includes>
                                    <include>org/apache/ibatis/migration/**</include>
                                    <include>META-INF/LICENSE</include>
                                    <include>META-INF/NOTICE</include>
                                    <include>mybatis-migrations.properties</include>
                                </includes>
                            </filter>
                        </filters>
                    </configuration>
                </execution>
            </executions>
        </plugin>
    </plugins>
</build>

hazendaz commented 1 year ago

@jwaterloo2020 mybatis 3 core is now out of this code base on master. Feel free to give the new build a test run.