call out ecto optional dependency

[aeruder]

Under elixir 1.15 code paths are pruned much more precisely when compiling deps. When a top-level project calls out ecto and geo_postgis, elixir will see the lack of an ecto dependency here and possibly compile geo_postgis before ecto is built.

if Code.ensure_loaded?(Ecto.Type) do
  defmodule Geo.PostGIS.Geometry do

would sometimes leave out Geo.PostGis.Geometry in elixir 1.15 and you'd get compile errors like the following in your application compile:

module Geo.PostGIS.Geometry is not available or is yet to be defined

This change calls out this optional ecto dependency. This can be reproduced very reliably by running:

• mix compile
• mix deps.compile --force geo_postgis
• mix compile --force

[aeruder]

Before:

% elixir --version
Erlang/OTP 26 [erts-14.0.2] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1] [jit]

Elixir 1.15.4 (compiled with Erlang/OTP 26)

% mix compile
[...]
Generated geo_postgis_dep_test app

% mix deps.compile --force geo_postgis
==> geo_postgis
Compiling 4 files (.ex)
Generated geo_postgis app

% mix compile --force
Compiling 1 file (.ex)
warning: Geo.PostGIS.Geometry.blank?/1 is undefined (module Geo.PostGIS.Geometry is not available or is yet to be defined)
  lib/geo_postgis_dep_test.ex:16: GeoPostgisDepTest.hello/0

Generated geo_postgis_dep_test app

After:

% mix compile
[...]
Generated geo_postgis_dep_test app

% mix deps.compile --force geo_postgis
==> geo_postgis
Compiling 4 files (.ex)
Generated geo_postgis app

% mix compile --force
Compiling 1 file (.ex)
Generated geo_postgis_dep_test app

[mgibowski-andjaro]

With the Postgrex upgrade to 0.17.3 I'm consistently getting the following error:

** (ArgumentError) unknown type Geo.PostGIS.Geometry for field :location
    (ecto 3.10.3) lib/ecto/schema.ex:2318: Ecto.Schema.check_field_type!/4
    (ecto 3.10.3) lib/ecto/schema.ex:1931: Ecto.Schema.__field__/4

And no error with code from this PR.

[tomtaylor]

@bryanjos it would be great to get this merged and released. Is there anything we can do to help?

[s3cur3]

@aeruder Thanks so much for this. I'm trying to figure out how to communicate the change to users... in my testing, I think what folks will need after taking the patch is:

mix deps.clean geo_postgis ecto && mix deps.get

The annoying thing is that in my testing, if I go back to the old version (for instance, because I was switching branches to a commit before the update), I have to run that again or else get the same error:

== Compilation error in file lib/my_app/whatever.ex ==
** (ArgumentError) unknown type Geo.PostGIS.Geometry for field :bounding_box
    (ecto 3.10.3) lib/ecto/schema.ex:2318: Ecto.Schema.check_field_type!/4
    (ecto 3.10.3) lib/ecto/schema.ex:1931: Ecto.Schema.__field__/4
    lib/my_app/whatever.ex:23: (module)

I think the best thing we can do is communicate that this is a one-time pain for users on Elixir v1.15, and you should just update all your branches at once. 😅

Does that sound right to you?

[s3cur3]

Works in my testing (with the above caveat that users will need to clean their deps when they update). I'm gonna merge as-is, update the documentation, and cut a release soon to try to get ahead of the Elixir 1.15 upgrade pain for folks as much as possible.

[josevalim]

Nice job @aeruder! In this case the Elixir v1.15 actually helped you find a bug. Without listing Ecto as an optional dependency, there was no guarantee Ecto would be compiled before geo_postgis. So this bug always existed depending on the order of your deps. Elixir v1.15 made the bug consistently reproducible. :)

[aeruder]

Nice job @aeruder! In this case the Elixir v1.15 actually helped you find a bug. Without listing Ecto as an optional dependency, there was no guarantee Ecto would be compiled before geo_postgis. So this bug always existed depending on the order of your deps. Elixir v1.15 made the bug consistently reproducible. :)

Yea, 1.15 has a method to making it reproducible (recompiling geo_postgis separately), but if you run deps.compile it still finds ecto if ecto was compiled first since it was already loaded and the load_path not having ecto no longer really matters. TBH, it'd be amazing if there was some way to actually hide Ecto (or remove those modules from beam) in that situation so it is always broken and 100% reproducible.

[josevalim]

Ah, excellent. I will investigate this!