Initial working pass of MariaDB 10.6.12... #710

TheKnowles · 2023-03-28T16:08:57Z

Initial working pass of MariaDB 10.6.12 for linux with a simulated start for winx64. OSX support is currently not available.

Opening this PR knowing it cannot currently be merged to main. I am looking for additional eyes on this and suggestions.

Originally, I branched off of the last commit prior to the upgrade to Java 17.

After I attempted to use it in my software (Java 11, Spring 5 / Boot 2.7.x), I ran into an issue where the annotations had been switched from javax to jakarta as part of the move to Spring 6. I then moved my branch to right before this change.

I would like to suggest creating a 2.x release branch that maintains a Java 11 / Spring 5 release and allow 3.x + to be Java 17 onwards. But this becomes a nightmare quickly because more recent MariaDB instances are not necessarily tied to the user's Java version, nor what version of Spring they are using. Fixes to MariaDB4j itself would then need to get backpatched to prior release branches as applicable.

The compatibility matrix effectively becomes MariaDB / Spring Framework / this product.

I don't mind doing the work for this, but at what point do we want cutoff retrofitting here versus maintaining a hard fork? Open to suggestions.

Having that said, this update was quite bespoke. In no particular order, several challenges that had to be solved to get 10.6.12 working in Linux:

MariaDB changed their download urls since the 10.3 commit to this repository.
MariaDB 10.4 removed no password root user. Additional commentary below
MariaDB 10.5.2 onward swapped how they did sym links between mysql* and maria* equivalents. Additional commentary below
OSX support was incomprehensible.
Consolidated slf4j to version 2.0.6. 1.7.x and 2.x results in NOOP binding

DL URL

https://dlm.mariadb.com/browse/mariadb_server/ ... versus the previous url in 10.3

Looks like Win32 support was dropped at some point.

Root user:

https://mariadb.com/kb/en/authentication-from-mariadb-104/

You can use the user that owns the data directory (generally the user that started MariaDB4j) to give the root user a password to continue the functionality of the unit tests.

Sym links:

I updated the entire codebase to reference maria* instead of mysql for binaries.

OSX:

Bintray was decommisioned in May 2021. Homebrew OCI got moved to Github Content Repository which is a Docker repo. After a lot of hunting and trying to figure out how to at least simulate a download, I got stuck at pulling the OCI from GHCR here:

https://github.com/Homebrew/homebrew-core/pkgs/container/core%2Fmariadb%2F10.6/69479903?tag=10.6.12

Because I was not on the right OS, docker would not pull.

I believe people have ran into this before and effectively have to build from source themselves or be on osx.

for winx64. Osx support is currently not available. Branched right before annotation change to allow Spring 5.x usage.

TheKnowles · 2023-03-28T16:56:51Z

Related: https://github.com/vorburger/MariaDB4j/issues/344 https://github.com/vorburger/MariaDB4j/issues/699

vorburger · 2023-04-17T17:26:39Z

DBs/pom.xml

@@ -20,16 +20,19 @@
    <packaging>pom</packaging>

    <modules>
+<!--        <module>mariaDB4j-db-win32-10.6.12</module>-->


Why is this commented? You can (should) uncomment this, so that it can be tested by others - and eventually deployed.

vorburger · 2023-04-17T17:27:06Z

DBs/pom.xml

-	<module>mariaDB4j-db-mac64-10.2.11</module>
+<!--        <module>mariaDB4j-db-mac64-10.6.12</module>-->
+	    <module>mariaDB4j-db-mac64-10.2.11</module>


Clean this up and revert this effectively formatting only change here, to avoid confusion? It's OK, for me, to ignore the Mac side of things in this version upgrade PR, just because it's already not working on current Macs as-is; that should be handled (more generally) as part of #497.

vorburger · 2023-04-17T17:27:30Z

DBs/pom.xml

        <module>mariaDB4j-db-mac64-10.1.23</module>
        <module>mariaDB4j-db-mac64-10.1.9</module>
        <module>mariaDB4j-db-mac64-5.5.34</module>

+<!--        <module>mariaDB4j-db-linux64-10.6.12</module>-->


Uncomment this (as above for the Windows version).

vorburger · 2023-04-17T17:28:54Z

README.md

-    <version>3.0.0</version>
+    <version>2.7.0</version>


Nope, thanks but no thanks, see #717 - do you want to revert this for this PR? 😄

vorburger · 2023-04-17T17:29:04Z

README.md

@@ -146,7 +146,7 @@ source; just git clone this and then mvn install or deploy. -- MariaDB4j's Maven
 <dependency>
    <groupId>ch.vorburger.mariaDB4j</groupId>
    <artifactId>mariaDB4j</artifactId>
-    <version>3.0.0-SNAPSHOT</version>
+    <version>2.7.0-SNAPSHOT</version>


revert please, as above

vorburger · 2023-04-17T17:29:16Z

mariaDB4j-app/pom.xml

@@ -6,7 +6,7 @@
 	<parent>
 		<groupId>ch.vorburger.mariaDB4j</groupId>
 		<artifactId>mariaDB4j-pom</artifactId>
-		<version>3.0.0-SNAPSHOT</version>
+		<version>2.7.0-SNAPSHOT</version>


nope, please revert

vorburger · 2023-04-17T17:29:22Z

mariaDB4j-core/pom.xml

@@ -6,7 +6,7 @@
    <parent>
        <groupId>ch.vorburger.mariaDB4j</groupId>
        <artifactId>mariaDB4j-pom</artifactId>
-        <version>3.0.0-SNAPSHOT</version>
+        <version>2.7.0-SNAPSHOT</version>


nope, please revert

vorburger · 2023-04-17T17:40:59Z

mariaDB4j-core/src/main/java/ch/vorburger/mariadb4j/DB.java

@@ -109,7 +109,7 @@ protected ManagedProcess createDBInstallProcess() throws ManagedProcessException
        logger.info("Installing a new embedded database to: " + baseDir);
        File installDbCmdFile = configuration.getExecutable(Executable.InstallDB);
        ManagedProcessBuilder builder = new ManagedProcessBuilder(installDbCmdFile);
-        builder.setOutputStreamLogDispatcher(getOutputStreamLogDispatcher("mysql_install_db"));
+        builder.setOutputStreamLogDispatcher(getOutputStreamLogDispatcher("mariadb-install-db"));


May I ask why?

Starting with mariadb 10.5.2 the bootstrap script is mariadb-install-db.
Sym linking the older script mysql_install_db caused the unpacked jar to not have the correct platform sub directory.

vorburger · 2023-04-17T17:41:22Z

mariaDB4j-core/src/main/java/ch/vorburger/mariadb4j/DB.java

@@ -174,7 +174,7 @@ protected String getReadyForConnectionsTag() {

    synchronized ManagedProcess startPreparation() throws ManagedProcessException, IOException {
        ManagedProcessBuilder builder = new ManagedProcessBuilder(configuration.getExecutable(Server));
-        builder.setOutputStreamLogDispatcher(getOutputStreamLogDispatcher("mysqld"));
+        builder.setOutputStreamLogDispatcher(getOutputStreamLogDispatcher("mariadbd"));


Starting with mariadb 10.5.2 the bootstrap script is mariadb-install-db.
Sym linking the older script mysql_install_db caused the unpacked jar to not have the correct platform sub directory.

vorburger · 2023-04-17T17:42:48Z

mariaDB4j-core/src/main/java/ch/vorburger/mariadb4j/DBConfigurationBuilder.java

@@ -43,7 +43,7 @@
 */
 public class DBConfigurationBuilder {

-    protected static final String WIN32 = "win32";
+    protected static final String WINX64 = "winx64";


Why? Is this "required" (really needed) or just "nicer"? What changes that requires this to change?

I want to make sure that we don't break backwards compability for end-users of this library unncessarily.

vorburger · 2023-04-17T17:45:08Z

mariaDB4j-core/src/main/java/ch/vorburger/mariadb4j/DBConfigurationBuilder.java

-        executables.putIfAbsent(Server, () -> new File(baseDir, "bin/mysqld" + getExtension()));
-        executables.putIfAbsent(Client, () -> new File(baseDir, "bin/mysql" + getExtension()));
-        executables.putIfAbsent(Dump, () -> new File(baseDir, "bin/mysqldump" + getExtension()));
+        executables.putIfAbsent(Server, () -> new File(baseDir, "bin/mariadbd" + getExtension()));
+        executables.putIfAbsent(Client, () -> new File(baseDir, "bin/mariadb" + getExtension()));
+        executables.putIfAbsent(Dump, () -> new File(baseDir, "bin/mariadb-dump" + getExtension()));


Oh, so the binaries all have new names in recent MariaDB versions? Interesting. But that then means that after merging this it will ONLY work with those new binaries. I'm torn about this, because the idea was that one could just put another binaries JAR on the classpath and it would still work. Hm... perhaps should be made configurable, then? Are you willing to work on this PR to introduce that? Or perhaps first raise a simpler separate PR proposing that?

vorburger · 2023-04-17T17:45:34Z

mariaDB4j-maven-plugin/pom.xml

@@ -6,7 +6,7 @@
    <parent>
        <groupId>ch.vorburger.mariaDB4j</groupId>
        <artifactId>mariaDB4j-pom</artifactId>
-        <version>3.0.0-SNAPSHOT</version>
+        <version>2.7.0-SNAPSHOT</version>


revert please, as above

vorburger · 2023-04-17T17:45:38Z

mariaDB4j-springboot/pom.xml

@@ -6,7 +6,7 @@
    <parent>
        <groupId>ch.vorburger.mariaDB4j</groupId>
        <artifactId>mariaDB4j-pom</artifactId>
-        <version>3.0.0-SNAPSHOT</version>
+        <version>2.7.0-SNAPSHOT</version>


revert please, as above

vorburger · 2023-04-17T17:45:46Z

mariaDB4j/pom.xml

@@ -6,7 +6,7 @@
 	<parent>
 		<groupId>ch.vorburger.mariaDB4j</groupId>
 		<artifactId>mariaDB4j-pom</artifactId>
-		<version>3.0.0-SNAPSHOT</version>
+		<version>2.7.0-SNAPSHOT</version>


revert please, as above

vorburger · 2023-04-17T17:46:50Z

mariaDB4j/src/test/java/ch/vorburger/mariadb4j/StartSimulatedForAllPlatformsTest.java

-    @Test public void simulatedStartOSX() throws Exception {
-        checkPlatformStart(DBConfigurationBuilder.OSX);
-    }
+    // Commented out for 10.6.12 binary addition
+    // With the decommission of bintray in 2021 and the move of OCI images to GitHub Content Repo
+    // as docker images, it was impossible to unpack a version for DBs/.
+    // See additional commentary in the PR.
+//    @Test public void simulatedStartOSX() throws Exception {
+//        checkPlatformStart(DBConfigurationBuilder.OSX);
+//    }


Agreed, but isn't this actually unrelated to the purpose of this PR, to bump for Linux & Windows? Or do you "need" this, here?

vorburger · 2023-04-17T17:49:22Z

mariaDB4j/src/test/java/ch/vorburger/mariadb4j/tests/MariaDB4jSampleTutorialTest.java

@@ -86,15 +86,13 @@ protected void check(DBConfigurationBuilder config) throws SQLException, Managed
        db.start();

        String dbName = "mariaDB4jTest"; // or just "test"
-        if (!dbName.equals("test")) {


Why this change? Do newer versions of MariaDB not "already have a DB named "test"? Then the inline comment below shouldn't be kept... This is actually kind of related to the other discussion, above; it will break anyone wanting to use older JARs. How about making this conditional of the MariaDB version?

vorburger · 2023-04-17T17:50:04Z

mariaDB4j/src/test/java/ch/vorburger/mariadb4j/tests/MariaDB4jSampleTutorialTest.java


        Connection conn = null;
        try {
-            conn = DriverManager.getConnection(db.getConfiguration().getURL(dbName), "root", "");
+            conn = DriverManager.getConnection(db.getConfiguration().getURL(dbName), System.getProperty("user.name"), "");


This is interesting. I guess it's... cleaner? Would you be willing to submit a small PR just for this?

vorburger · 2023-04-17T17:53:14Z

mariaDB4j/src/test/java/ch/vorburger/mariadb4j/tests/MariaDB4jSampleTutorialTest.java

+        // we will modify the root user password back to an empty string
+        // Using the user that owns the data directory / uid we can execute this initial bootstrapping command
+        // to give us pre 10.4 behavior for the purposes of this test
+        db.run("SET PASSWORD FOR 'root'@'localhost' = PASSWORD('root');", System.getProperty("user.name"), "");


Oh, interesting. Because this test is also the "Tutorial", in a sense this promotes doing something "insecure". I'm actually a bit confused why the root user is still required if System.getProperty("user.name") is being use above. I wonder if there is a better solution for this. Could this entire TutorialTest just not use the (DB, not OS) root user at all? Perhaps improving this is worth a separate smaller PR, as a pre-requisite for this?

vorburger · 2023-04-17T17:53:31Z

pom.xml

@@ -11,7 +11,7 @@
    </parent>

 	<artifactId>mariaDB4j-pom</artifactId>
-	<version>3.0.0-SNAPSHOT</version>
+	<version>2.7.0-SNAPSHOT</version>


revert please, as above

vorburger · 2023-04-17T17:55:30Z

pom.xml

@@ -155,10 +156,15 @@
 	            <type>pom</type>
 	            <scope>import</scope>
 	        </dependency>
+			 <dependency>
+				 <groupId>org.slf4j</groupId>
+				 <artifactId>slf4j-api</artifactId>


Erm, why is this required? And what does it have to do with the rest of the PR? I think some time (long?) ago I had removed this, and seems to work as-is (on the current main branch). Would you be willing to revert this change here and take it to a separate PR, for separate discussion, if needed?

vorburger

@TheKnowles Thank You for this!!! My apologies for the 3 week delay in reviewing. Unfortunately I can only work on this project occassionally. I'm hoping that my review feedback here is "motivating" to sort this out together to get it to a state where it can be merged, and not ... "harsh" in any way? I really do appreciate the contribution, but this PR will require some work. Also it needs to be rebased on the current main branch, as per related inline comment, and #717.

Initial working pass of MariaDB 10.6.12 for linux with a simulated start

7c8c144

for winx64. Osx support is currently not available. Branched right before annotation change to allow Spring 5.x usage.

vorburger self-requested a review April 17, 2023 17:25

vorburger reviewed Apr 17, 2023

View reviewed changes

vorburger mentioned this pull request Apr 17, 2023

Clarify 2.x Maintenance #717

Open

vorburger reviewed Apr 17, 2023

View reviewed changes

vorburger requested changes Apr 17, 2023

View reviewed changes

vorburger mentioned this pull request Apr 17, 2023

mysql: error while loading shared libraries: libnsl.so.1: cannot open shared object file: No such file or directory #156

Closed

TheKnowles closed this Apr 20, 2023

vorburger mentioned this pull request May 1, 2023

Support for sequences #727

Closed

TheKnowles mentioned this pull request Sep 15, 2023

Initial addition of 10.11.5 with working unit tests. #771

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial working pass of MariaDB 10.6.12... #710

Initial working pass of MariaDB 10.6.12... #710

TheKnowles commented Mar 28, 2023

TheKnowles commented Mar 28, 2023

vorburger Apr 17, 2023

vorburger Apr 17, 2023 •

edited

Loading

vorburger Apr 17, 2023

vorburger Apr 17, 2023 •

edited

Loading

vorburger Apr 17, 2023

vorburger Apr 17, 2023

vorburger Apr 17, 2023

vorburger Apr 17, 2023

TheKnowles Apr 17, 2023

vorburger Apr 17, 2023

TheKnowles Apr 17, 2023

vorburger Apr 17, 2023

vorburger Apr 17, 2023

vorburger Apr 17, 2023

vorburger Apr 17, 2023

vorburger Apr 17, 2023

vorburger Apr 17, 2023

vorburger Apr 17, 2023

vorburger Apr 17, 2023

vorburger Apr 17, 2023

vorburger Apr 17, 2023

vorburger Apr 17, 2023

vorburger left a comment

Initial working pass of MariaDB 10.6.12... #710

Initial working pass of MariaDB 10.6.12... #710

Conversation

TheKnowles commented Mar 28, 2023

TheKnowles commented Mar 28, 2023

Choose a reason for hiding this comment

vorburger Apr 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vorburger Apr 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vorburger left a comment

Choose a reason for hiding this comment

vorburger Apr 17, 2023 •

edited

Loading

vorburger Apr 17, 2023 •

edited

Loading