Base directory is hardcoded for init script

[colinphill-mdsol]

I'm writing an adapter around Migratus for an internal task runner. I'd like to write tests for it, and I'd like those tests to use a suite of migrations in my project's test_resources directory. I see there is a :parent-migration-dir option which controls where Migratus looks for the migrations directory, and this project's own tests use that option to run test migrations. The init function, however, does not respect this setting; it always looks in resources.

[yogthos]

Ah yeah, that looks like an omission. Any chance you could do a PR for this?

[colinphill-mdsol]

Unfortunately my company's OSS policies are...unsettled, and I can't at present make contributions.

[yogthos]

Ah, fair enough. I'll take a look.

[yogthos]

Hmm, it looks like the init-script should already be resolved relative to the migration directory

migratus/src/migratus/database.clj

Line 123 in f0972c6

(defn find-init-script [dir init-script-name]

[colinphill-mdsol]

When that function is called, the directory provided does not account for this option. It resolves the :migration-dir option against a base path, but the base path is static.

Tracing the data flow:

We dispatch init on store type, which is generally going to be :database.

migratus/src/migratus/core.clj

Lines 191 to 194 in f0972c6

	(defn init
	"Initialize the data store"
	[config & [name]]
	(proto/init (proto/make-store config)))

The database implementation of init passes its entire config to get-migration-dir and get-init-script.

migratus/src/migratus/database.clj

Lines 292 to 300 in f0972c6

	(init [this]
	(let [conn (connect* (assoc (:db config) :transaction? (:init-in-transaction? config)))]
	(try
	(init-db! conn
	(utils/get-migration-dir config)
	(utils/get-init-script config)
	(sql-mig/wrap-modify-sql-fn (:modify-sql-fn config))
	(get config :init-in-transaction? true)
	(props/load-properties config))

Both of those functions look only at their respective properties, falling back to the defaults of "migrations" and "init.sql".

migratus/src/migratus/utils.clj

Lines 20 to 28 in f0972c6

	(defn get-migration-dir
	"Gets the :migration-dir from config, or default if missing."
	[config]
	(get config :migration-dir default-migration-dir))
	(defn get-init-script
	"Gets the :init-script from config, or default if missing."
	[config]
	(get config :init-script default-init-script-name))

The init-db! function passes those two values directly to find-init-script and slurps the result.

migratus/src/migratus/database.clj

Lines 279 to 282 in f0972c6

	(defn init-db! [db migration-dir init-script-name modify-sql-fn transaction? properties]
	(if-let [init-script (some-> (find-init-script migration-dir init-script-name)
	slurp
	(inject-properties properties))]

Nothing along the way has accounted for :parent-migration-dir, and at this point in the call stack we no longer have access to that value because we're no longer holding the entire config.

[colinphill-mdsol]

Here is where the actual hardcoding happens:

migratus/src/migratus/utils.clj

Lines 80 to 89 in f0972c6

	(defn find-migration-dir
	"Finds the given directory on the classpath. For backward
	compatibility, tries the System ClassLoader first, but falls back to
	using the Context ClassLoader like Clojure's compiler.
	If classloaders return nothing try to find it on a filesystem."
	([^String dir]
	(or (find-migration-dir (ClassLoader/getSystemClassLoader) default-migration-parent dir)
	(-> (Thread/currentThread)
	(.getContextClassLoader)
	(find-migration-dir default-migration-parent dir))))

And the function we've used to find the script does indeed call that unary overload of find-migration-dir:

migratus/src/migratus/database.clj

Lines 123 to 128 in f0972c6

	(defn find-init-script [dir init-script-name]
	(let [dir (utils/ensure-trailing-slash dir)]
	(when-let [migration-dir (utils/find-migration-dir dir)]
	(if (instance? File migration-dir)
	(find-init-script-file migration-dir init-script-name)
	(find-init-script-resource dir migration-dir init-script-name)))))

[yogthos]

Ah ok, thanks for digging in. It's been a while since I've looked at it. I'll try get a fix out in the coming days.

[colinphill-mdsol]

Also worth mentioning: This option isn't actually documented. One alternative is to just say it's an implementation detail and not make this change. I think my use case is a compelling one for promoting it to a full-fledged feature, though.

[yogthos]

Yeah, I think this would be worth doing and doesn't look like it should be too much work.

[colinphill-mdsol]

Ah, I just discovered that this also happens with the migrate function. Same basic flow – it lands in that unary overload of find-migration-dir. Full support for this option might wind up touching a fair amount of the public API.

[yogthos]

Ah yeah, looks like it might be a bit more fiddly than it initially appeared. Perhaps for your use case it might be easier to just use with-redefs for find-init-script.

[colinphill-mdsol]

I wound up settling on a workaround of just using a couple .. segments in the configuration. Not ideal, but adequate!

[yogthos]

I'll keep the issue open. I think it'd be good to fix if I get time. Glad the workaround is working in the mean time.