Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions lib/ecto/migration.ex
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,19 @@ defmodule Ecto.Migration do
such as migrations. For a repository named `MyApp.FooRepo`, `:priv` defaults to
"priv/foo_repo" and migrations should be placed at "priv/foo_repo/migrations"

* `:migrations_paths` - a list of paths where migrations are located. This option
allows you to specify multiple migration directories that will all be used when
running migrations. Relative paths are considered relative to the application root
(the directory containing `mix.exs`). If this option is not set, the default path
is derived from the `:priv` configuration. For example:

config :app, App.Repo,
migrations_paths: ["priv/repo/migrations", "priv/repo/tenant_migrations"]

When using this option, all specified paths will be checked for migrations, and
migrations will be sorted by version across all directories as if they were in
a single directory.

* `:start_apps_before_migration` - A list of applications to be started before
running migrations. Used by `Ecto.Migrator.with_repo/3` and the migration tasks:

Expand Down
51 changes: 47 additions & 4 deletions lib/ecto/migrator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,49 @@ defmodule Ecto.Migrator do
Application.app_dir(app, Path.join(priv, directory))
end

@doc """
Gets the migrations paths from a repository configuration.

This function checks the repository configuration for the `:migrations_paths`
option. If found, it returns a list of absolute paths by resolving any relative
paths against the application directory. If not found, it returns a single-element
list containing the default migrations path.

Relative paths in the `:migrations_paths` configuration are considered relative
to the root of the application (the directory containing `mix.exs`).

## Examples

# In config/config.exs
config :my_app, MyApp.Repo,
migrations_paths: ["priv/repo/migrations", "priv/repo/tenant_migrations"]

"""
@spec migrations_paths(Ecto.Repo.t()) :: [String.t()]
def migrations_paths(repo) do
config = repo.config()

case config[:migrations_paths] do
nil ->
[migrations_path(repo)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should actually move the logic of migrations_path here and reimplement migrations_path as hd(migrations_paths(...)), adding a note it is now discouraged. Otherwise you can have code using migrations_path and migrations_paths and working with two completely set of directories.

Copy link
Author

@pnezis pnezis Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept migrations_path unchanged to ensure backwards compatibility. If we call hd(migrations_paths(...)) and the :migrations_paths is set with more than one paths then it will only return the first one.

Should we raise in such cases? e.g.

def migrations_path(repo, directory) do
   if is_list(config[:migrations_paths]) && length(config[:migrations_paths]) > 1 do
       raise "multiple migrations paths defined, use migrations_paths/1 instead"
   end

   hd(...)
end 

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raising is good! But please use pattern matching! :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, in order to handle the optional directory argument expected by migrations_path/1 I used an opts in migrations_paths/1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see! I would prefer to not expose the directory option, since it doesn't work with the configuration, so perhaps we have to refactor this into a private function where we keep the directory hidden.

I believe the directory though was added for people who want to have additional migrations directories, but not plug them all by default. So this pull request has the unfortunate side-effect of making those cases harder. :( And now I am not sure how to move forward.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#245 introduced this.

If you ask me, since the mix tasks support passing multiple migrations paths, we should only have migrations_paths. We could keep migrations_path/1 as it is for backwards compatibility or soft deprecate is.

Currently if you call migrations_path with a different directory then you will need to pass it explicitly in the ecto.migrate --migrations-path in order to be applied.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that's how some projects tackle it indeed. In production you run them at distinct moments, so you don't want the additional path as default. And then you have mix tasks/aliases so you don't specify all of them:

[
  "ecto.migrate.data": "migrate --migration-path foo/bar",
  "ecto.migrate.all": "migrate --migration-path foo/bar --migration-path foo/baz"
]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or I could bring its implementation back and have something like:

def migrations_path(repo, directory \\ "migrations") do
    config = repo.config()

    if is_list(config[:migrations_paths]) do
         IO.warn("some warning....")
    end 

    priv = config[:priv] || "priv/#{repo |> Module.split() |> List.last() |> Macro.underscore()}"
    app = Keyword.fetch!(config, :otp_app)
    [Application.app_dir(app, Path.join(priv, directory))]
end

def migrations_paths(repo) do
   case config[:migrations_paths] do
      nil -> [migrations_path(repo)]
      ...
   end
end

wdyt?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the main problem is when you have multiple repos:

with the suggested change

config :my_app, ecto_repos: [Repo1, Repo2, Repo3]

config :my_app, Repo1, migrations_paths: ["foo", "bar", "../baz"]
config :my_app, Repo2, migrations_paths: ["foo2", "bar2"]
config :my_app, Repo3 # no migrations path the default priv/migrations is used

mix ecto.migrate # works automatically for all repos

# similarly the
mix ecto.migrate -r Repo1

currently

you need to create an for each repo alias and re-alias ecto.migrate or create another alias in order to work

[
    "ecto.migrate.repo1": "migrate -r Repo1 --migration-path foo --migration-path bar ...",
    "ecto.migrate.repo2": "migrate -r Repo2 --migration-path foo2 ...",
     "ecto.migrate.repo3": "migrate -r Repo3"
     "ecto.migrate.all": [
          "ecto.migrate.repo1", "ecto.migrate.repo2", "ecto.migrate.repo3" 
      ]  
]

which works but is not convenient and prone to errors, e.g. when you want to add a new repo, or use the same repo in multiple apps


paths when is_list(paths) ->
app = Keyword.fetch!(config, :otp_app)

Enum.map(paths, fn path ->
if Path.type(path) == :absolute do
path
else
Application.app_dir(app, path)
end
end)

other ->
raise ArgumentError,
":migrations_paths must be a list of paths, got: #{inspect(other)}"
end
end

@doc """
Gets all migrated versions.

Expand Down Expand Up @@ -372,13 +415,13 @@ defmodule Ecto.Migrator do

Equivalent to:

Ecto.Migrator.run(repo, [Ecto.Migrator.migrations_path(repo)], direction, opts)
Ecto.Migrator.run(repo, Ecto.Migrator.migrations_paths(repo), direction, opts)

See `run/4` for more information.
"""
@spec run(Ecto.Repo.t(), atom, Keyword.t()) :: [integer]
def run(repo, direction, opts) do
run(repo, [migrations_path(repo)], direction, opts)
run(repo, migrations_paths(repo), direction, opts)
end

@doc ~S"""
Expand Down Expand Up @@ -464,12 +507,12 @@ defmodule Ecto.Migrator do

Equivalent to:

Ecto.Migrator.migrations(repo, [Ecto.Migrator.migrations_path(repo)])
Ecto.Migrator.migrations(repo, Ecto.Migrator.migrations_paths(repo))

"""
@spec migrations(Ecto.Repo.t()) :: [{:up | :down, id :: integer(), name :: String.t()}]
def migrations(repo) do
migrations(repo, [migrations_path(repo)])
migrations(repo, migrations_paths(repo))
end

@doc """
Expand Down
36 changes: 35 additions & 1 deletion lib/mix/ecto_sql.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,45 @@ defmodule Mix.EctoSQL do

@doc """
Ensures the given repository's migrations paths exists on the file system.

This function checks for migrations paths in the following order:
1. Command-line options (`--migrations_path`)
2. Repository configuration (`:migrations_paths`)
3. Default path based on `:priv` configuration or "priv/repo/migrations"
"""
@spec ensure_migrations_paths(Ecto.Repo.t(), Keyword.t()) :: [String.t()]
def ensure_migrations_paths(repo, opts) do
paths = Keyword.get_values(opts, :migrations_path)
paths = if paths == [], do: [Path.join(source_repo_priv(repo), "migrations")], else: paths

paths =
if paths == [] do
# Use repo config if available, otherwise fall back to default
config = repo.config()

case config[:migrations_paths] do
nil ->
[Path.join(source_repo_priv(repo), "migrations")]

config_paths when is_list(config_paths) ->
app = Keyword.fetch!(config, :otp_app)
# In Mix context, we use deps_paths or cwd for path resolution
base_dir = Mix.Project.deps_paths()[app] || File.cwd!()

Enum.map(config_paths, fn path ->
if Path.type(path) == :absolute do
path
else
Path.join(base_dir, path)
end
end)

other ->
raise ArgumentError,
":migrations_paths must be a list of paths, got: #{inspect(other)}"
end
else
paths
end

if not Mix.Project.umbrella?() do
for path <- paths, not File.dir?(path) do
Expand Down
199 changes: 199 additions & 0 deletions test/ecto/migrator_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,205 @@ defmodule Ecto.MigratorTest do
end
end

describe "migrations_paths" do
defmodule RepoWithMigrationsPaths do
def config do
[
otp_app: :ecto_sql,
migrations_paths: ["priv/repo/migrations", "priv/repo/tenant_migrations"]
]
end
end

defmodule RepoWithAbsoluteMigrationsPaths do
def config do
[
otp_app: :ecto_sql,
migrations_paths: ["/absolute/path/migrations", "relative/path/migrations"]
]
end
end

defmodule RepoWithInvalidMigrationsPaths do
def config do
[
otp_app: :ecto_sql,
migrations_paths: "not_a_list"
]
end
end

test "returns default path when migrations_paths is not configured" do
paths = migrations_paths(TestRepo)
expected = [migrations_path(TestRepo)]
assert paths == expected
end

test "returns configured paths with relative paths resolved" do
paths = migrations_paths(RepoWithMigrationsPaths)
app_dir = Application.app_dir(:ecto_sql)

assert paths == [
Path.join(app_dir, "priv/repo/migrations"),
Path.join(app_dir, "priv/repo/tenant_migrations")
]
end

test "handles absolute paths correctly" do
paths = migrations_paths(RepoWithAbsoluteMigrationsPaths)
app_dir = Application.app_dir(:ecto_sql)

assert paths == [
"/absolute/path/migrations",
Path.join(app_dir, "relative/path/migrations")
]
end

test "raises error when migrations_paths is not a list" do
assert_raise ArgumentError,
":migrations_paths must be a list of paths, got: \"not_a_list\"",
fn ->
migrations_paths(RepoWithInvalidMigrationsPaths)
end
end
end

describe "migrations with migrations_paths set" do
@describetag migrated_versions: []

test "runs migrations from multiple configured paths in order" do
in_tmp(fn path ->
# Create two migration directories
File.mkdir_p!("migrations_a")
File.mkdir_p!("migrations_b")

# Create migrations in different directories
create_migration("migrations_a/15_migration_a.exs")
create_migration("migrations_b/16_migration_b.exs")
create_migration("migrations_a/17_migration_c.exs")

# Use explicit paths since we're in a tmp directory
paths = [
Path.join(path, "migrations_a"),
Path.join(path, "migrations_b")
]

# Run all migrations
assert run(TestRepo, paths, :up, all: true, log: false) == [15, 16, 17]
end)
end

test "migrations/1 reports status from multiple configured paths" do
in_tmp(fn path ->
File.mkdir_p!("migrations_a")
File.mkdir_p!("migrations_b")

create_migration("migrations_a/18_migration_a.exs")
create_migration("migrations_b/19_migration_b.exs")
create_migration("migrations_a/20_migration_c.exs")

paths = [
Path.join(path, "migrations_a"),
Path.join(path, "migrations_b")
]

# All should be pending (down)
expected = [
{:down, 18, "migration_a"},
{:down, 19, "migration_b"},
{:down, 20, "migration_c"}
]

assert migrations(TestRepo, paths) == expected
end)
end

test "can rollback migrations from multiple paths" do
in_tmp(fn path ->
File.mkdir_p!("migrations_a")
File.mkdir_p!("migrations_b")

create_migration("migrations_a/21_migration_a.exs")
create_migration("migrations_b/22_migration_b.exs")
create_migration("migrations_a/23_migration_c.exs")

paths = [
Path.join(path, "migrations_a"),
Path.join(path, "migrations_b")
]

# Run all migrations
assert run(TestRepo, paths, :up, all: true, log: false) == [21, 22, 23]

# Rollback step by step
capture_io(:stderr, fn ->
assert run(TestRepo, paths, :down, step: 1, log: false) == [23]
assert run(TestRepo, paths, :down, step: 2, log: false) == [22, 21]
end)
end)
end

test "migrations from multiple paths are sorted by version" do
in_tmp(fn path ->
File.mkdir_p!("migrations_a")
File.mkdir_p!("migrations_b")

# Create migrations with versions that would be out of order if grouped by directory
create_migration("migrations_b/24_first.exs")
create_migration("migrations_a/25_second.exs")
create_migration("migrations_b/26_third.exs")
create_migration("migrations_a/27_fourth.exs")

paths = [
Path.join(path, "migrations_a"),
Path.join(path, "migrations_b")
]

# They should run in version order, not directory order
assert run(TestRepo, paths, :up, all: true, log: false) == [24, 25, 26, 27]

# Verify migrations status is also in correct order
expected = [
{:up, 24, "first"},
{:up, 25, "second"},
{:up, 26, "third"},
{:up, 27, "fourth"}
]

assert migrations(TestRepo, paths) == expected
end)
end

test "handles missing migration files from one path" do
in_tmp(fn path ->
File.mkdir_p!("migrations_a")
File.mkdir_p!("migrations_b")

create_migration("migrations_a/28_migration_a.exs")
create_migration("migrations_b/29_migration_b.exs")

paths = [
Path.join(path, "migrations_a"),
Path.join(path, "migrations_b")
]

# Run migrations
assert run(TestRepo, paths, :up, all: true, log: false) == [28, 29]

# Delete a migration file from one directory
File.rm("migrations_b/29_migration_b.exs")

# Should show file not found for the deleted migration
expected = [
{:up, 28, "migration_a"},
{:up, 29, "** FILE NOT FOUND **"}
]

assert migrations(TestRepo, paths) == expected
end)
end
end

describe "with_repo" do
defmodule Repo do
def start_link(opts) do
Expand Down
Loading