Problems with reading other applications config at compile time

maciej-szlosarczyk commented 1 year ago

ex_cldr reads other applications' configuration values at compile time, namely phoenix's and ecto json_library.

There are two separate issues here:

Ecto does not defined json_library

According to this issue, ecto has moved the json_library config value down to adapter level (i.e postgrex). ex_cldr reads non-existent config value.

Problems while building Docker images

Reading other applications configuration at compile time can cause problems when building docker images - one of the ways docker images are build is that you build dependencies without config, in a separate step. Asciinema server does that:

https://github.com/asciinema/asciinema-server/blob/662b2098660dfcf5edc82a8cc652740f415bcfab/Dockerfile#L24-L25

This can cause pretty opaque failures:

------
 > [5/5] RUN mix phx.digest:
#0 0.636 ERROR! the application :phoenix has a different value set for key :json_library during runtime compared to compile time. Since this application environment entry was marked as compile time, this difference can lead to different behaviour than expected:
#0 0.636
#0 0.636   * Compile time value was not set
#0 0.636   * Runtime value was set to: Jason
#0 0.636
#0 0.636 To fix this error, you might:
#0 0.636
#0 0.636   * Make the runtime value match the compile time one
#0 0.636
#0 0.636   * Recompile your project. If the misconfigured application is a dependency, you may need to run "mix deps.compile phoenix --force"
#0 0.636
#0 0.636   * Alternatively, you can disable this check. If you are using releases, you can set :validate_compile_env to false in your release configuration. If you are using Mix to start your system, you can pass the --no-validate-compile-env flag
#0 0.636
#0 0.636
#0 0.652
#0 0.652 07:49:00.701 [error] Task #PID<0.221.0> started from #PID<0.101.0> terminating
#0 0.652 ** (stop) "aborting boot"
#0 0.652     (elixir 1.14.5) Config.Provider.boot/2
#0 0.652 Function: &:erlang.apply/2
#0 0.652     Args: [#Function<1.81321692/1 in Mix.Tasks.Compile.All.load_apps/3>, [ex_cldr: "/_build/dev/lib"]]
#0 0.652 ** (EXIT from #PID<0.101.0>) an exception was raised:
#0 0.652     ** (ErlangError) Erlang error: "aborting boot"
#0 0.652         (elixir 1.14.5) Config.Provider.boot/2
#0 0.652
------

I have create a reproduction case here: https://github.com/maciej-szlosarczyk/carbon, it reliably fails on every run of docker build .

Proposed solutions

Remove Application.compile_env(:ecto, :json_library), there is no such configuration option present in Ecto.
Consider changing Application.compile_env(:ex_cldr, :json_library) to raise if the value is not set (Application.compile_env!). It's a breaking change, but it produces better error messages.

kipcole9 commented 1 year ago

Thanks for the report @maciej-szlosarczyk and sorry for the very cryptic messaging.

Remove Application.compile_env(:ecto, :json_library)

I didn't know this - will definitely remove

Application.compile_env(:ex_cldr, :json_library)

I'm still a bit confused on the behaviour since in the example app (thanks very much, greatly appreciated) the config :phoenix, json_library: Jason is still compile time so I'm surprised its being reported as Compile time value was not set. I'll try some further experiments today. However given your report, removing the ability to use whatever Phoenix uses for a json_library makes sense if there's no other resolution

What are your thoughts on auto configuring Jason or Poison if they are available at compile time? (that's the current behaviour).

Future releases won't be json-based

For the next release cycle of ex_cldr aligned to CLDR 44 the local data will be in erlang term format, not json. This should result in faster compile times and remove the requirement for a json library at all.

maciej-szlosarczyk commented 1 year ago

Hey @kipcole9, thank you for a very quick change!

I think the change introduced in https://github.com/elixir-cldr/cldr/commit/fef2f97aa5de0350a4ec6688bb97875fc239edd4 is the best choice. Judging by number of downloads on hex.pm Jason and Poison are by the most popular JSON encoders for Elixir, but folks still have an option to adjust if they want to use something less common.

kipcole9 commented 1 year ago

I've published ex_cldr version 2.37.2 with the following changelog entry:

Bug Fixes

Remove checking Phoenix and Ecto json_library configurations. Ecto no longer configures its :json_library in config.exs and checking for Phoenix configuration can cause config failures when building in Docker. Thanks to @maciej-szlosarczyk for the report. Closes #208.

Enhancements

Improves parsing of language tag extensions. Primarily intended to improve support for tempo.

Thanks for the report and collaboration.

elixir-cldr / cldr