Fix runtime persistence config for releases, resolves #109

[connorlay]

defdelegate/2 calls persistence_adapter/0 at compile-time, causing system env configuration to be read prematurely and stored into the function definition itself. This commit replaces calls to defdelegate/2 with functions that call persistence_adapter/0 at runtime, fixing the issue.

I believe this resolves #109.

[durub]

Thanks for this PR, @connorlay. I was affected by this and was very lost on how to fix it.

It would be great to have this fixed. I took a look and it looks good to me!

[dvic]

hi @connorlay have you seen my comment in #109 (comment)? Have you considered keeping it at compile-time but using Application.compile_env?

[connorlay]

Have you considered keeping it at compile-time but using Application.compile_env?

@dvic I have not, but I like that idea! I'm curious if @tompave has any thoughts regarding this? I might have some cycles to spend on this issue next week, we'll see.

[dvic]

Have you considered keeping it at compile-time but using Application.compile_env?

@dvic I have not, but I like that idea! I'm curious if @tompave has any thoughts regarding this? I might have some cycles to spend on this issue next week, we'll see.

I can probably also hack on it this weekend, this issue has bitten me already twice, I want it solved! hah 😄

[marcandre]

This change is required for compatibility with upcoming Elixir 1.13.
A fix and a release would be much appreciated 🙏

[k-cross]

@tompave elixir 1.13 is coming soon and it'd be awesome to get this in. Really love the library, thanks for all your work on it and thank you @connorlay for resolving this issue!

[tompave]

Hello, thank you for working on this, and apologies for the long wait.
I've been away from this project for some time, but I'll have a look soon.

In the meantime, the code changes in the PR look good. @connorlay I've just released v.17.0 and pushed some changes to the Elixir and OTP versions in CI, plus bumped some dependencies in Mix.lock. Can you please rebase this on master and push, so that you can run the latest CI?

[connorlay]

Can you please rebase this on master and push, so that you can run the latest CI?

@tompave Done! It looks like I need maintainer approval to run the new CI jobs.

[tompave]

Ok, I've caught up with the issue and the discussion in this PR.

I think that this PR solves part of the problem, and I'm going to merge it. Thank you for working on this ❤️ and apologies again for the long wait. :-(

When I say "part of the problem" I mean that the store module is still compiled into a module attribute in lib/fun_with_flags.ex#L30. Admittedly that is just the module name, which won't change unless the cache is disabled. By default, without custom config, the cache is enabled so most people will not encounter the issue.

There are also another couple of defdelegate that need to be updated:
https://github.com/tompave/fun_with_flags/blob/v1.7.0/lib/fun_with_flags.ex#L439-L459
I'll do it after merging this.

One final comment. @dvic and @connorlay, on using Application.compile_env/3, I've replied in #109 (comment). The relevant part:

The suggestion to use Application.compile_env/3 is something I considered in the past to address the problem. Unfortunately, since that function first appeared in Elixir 1.10, I can't adopt it and still keep backward compatibility with older Elixirs. The good news is that FWF v1.7.0 (released yesterday) bumps the supported range to Elixir 1.9-1.12 (and drops 1.8), and FWF v1.8.0 will bump the supported range to Elixir 1.10-1.13, which means that I'll be able to update the config to use that new helper.

[tompave]

@marcandre and @k-cross: why do you think that this is a blocker for Elixir 1.13? It doesn't seem to be required in my tests with v1.13-rc0.

[marcandre]

The defdelegate no longer compiled in 1.13:

== Compilation error in file lib/fun_with_flags/protocols/actor.ex ==
** (CompileError) lib/fun_with_flags/protocols/actor.ex:117: undefined function defdelegate/2 (there is no such import)

could not compile dependency :fun_with_flags, "mix compile" failed. Errors may have been logged above. You can recompile this dependency with "mix deps.compile fun_with_flags", update it with "mix deps.update fun_with_flags" or clean it with "mix deps.clean fun_with_flags"

I presume this is because of "Add defdelegate to the list of unallowed macros inside protocols as protocols do not allow function definitions"

Latest release works.

[tompave]

@marcandre Ah, I see.

That was actually already fixed on master (but yet unreleased). It was fixed with #105.

On the other hand, the current PR (111) removes other defdelegate calls that were causing config+compile issues, but were not incompatible with Elixir 1.13 per se.

[marcandre]

Ah, then my mistake, sorry 😅