fun_with_flags icon indicating copy to clipboard operation
fun_with_flags copied to clipboard

Switch on ecto integration only if Ecto.Adapters.SQL exists

Open rewritten opened this issue 2 years ago • 3 comments

Hi.

I'm using this amazing library, with a custom backend, and I'm also using Ecto for entities (without a SQL backend either). The library, when compiling, complains that

warning: Ecto.Adapters.SQL.query!/3 is undefined (module Ecto.Adapters.SQL is not available or is yet to be defined)

(rightfully so) but the fact is that I don't have ecto_sql dependency so there is no reason FWF should expect to be able to use it for persistence.

I would be able to attempt a patch for this in a few days if you folks cannot work on that.

rewritten avatar Jul 20 '22 15:07 rewritten

Thank you for reporting this.

That's odd. I have not seen it before, but it seems you have an interesting setup.

Just to be sure, can you share your FWF configuration?

Still, chances are that it's because of this: https://github.com/tompave/fun_with_flags/blob/c97d99bf93410fb5dfb4645d22a0a24a9b00657e/lib/fun_with_flags/store/persistent/ecto/record.ex#L1

Basically the library is not trying to use Ecto, but on compilations it's trying to compile files that are meant to be ignored if ecto is not present. In your case, Ecto is present, but not Ecto.SQL, so the compiler trips over those references.

Maybe that should be Code.ensure_loaded?(Ecto.Adapters.SQL) instead.

tompave avatar Jul 30 '22 22:07 tompave

Also looking at https://github.com/tompave/fun_with_flags/issues/138 (closing that because I think it's actually the same issue). I think that nothing is really trying to use Ecto, but the conditional compilation guard is failing because your app actually includes Ecto.

I'm not sure if this can be solved, but clearly FWF cannot depend on Ecto, but at the same time the app depends on ecto itself. Maybe adding those ref excludes in FWF def project() would do?

For what is worth, FWF depends on ecto_sql, but as an optional dependency. That should be fine: https://github.com/tompave/fun_with_flags/blob/c97d99bf93410fb5dfb4645d22a0a24a9b00657e/mix.exs#L62

tompave avatar Jul 30 '22 22:07 tompave

@rewritten, can you please give this branch a try and report back? https://github.com/tompave/fun_with_flags/pull/140

tompave avatar Jul 30 '22 23:07 tompave

It works flawlessly, no more warnings displayed 🎉 💜 💛

rewritten avatar Aug 09 '22 15:08 rewritten

The fix for this has been merged into master.

tompave avatar Aug 12 '22 09:08 tompave