Support crystal 0.36.1+

[kimburgess]

Narrows special case behaviour introduced in #4 to crystal 0.36.0 only.

Support for operator based macro's re-introduced to compiler.

[kimburgess]

Looks to be a fun new compatibility issue in nightly (what will become 0.36.1) due to https://github.com/crystal-lang/crystal/blob/63a3c51ebf97a7c461fae6c122fa53619ce96465/src/kernel.cr#L548

[kimburgess]

Well this is supremely inconvenient. As macro's must be defined prior to usage, and in this case usage is part of the prelude libs, I cannot see a workable resolution.

Considered options (none are good):

1. remove support for static env usage tracking (which drops core lib functionality)
2. only supporting partial tracking e.g. ignoring 'soft' lookups via []? (still leaves things fragile if [] is ever used, and presents incorrect overall usage info)
3. patching the compiler to use the .fetch method (no argument exists for this outside of providing compatibility for this extremely insignificant shard; also reduces readability there, and remains fragile as above).

Very open to other ideas or approaches.

[stakach]

they've already re-introduced []? macros in 0.36.1 / nightlies - or am I missing something else?
crystal-lang/crystal#10338

[kimburgess]

Support for defining operator macros is back, however there is also now a call to ENV[]? in kernel.cr (linked above).

Macro’s must be defined before they are called as that stage of compilation is single pass. As this call happens before anything here is parsed, this breaks the compiler.

[stakach]

can't hurt making a pull request, fewer macros used before user code the better I would think

[kimburgess]

It only becomes a macro when introduced via this library. The default implementation for ENV.[]? is just a method.

[stakach]

maybe we use a different namespace?
like SENV? or SEC_ENV

Then we use that for all our projects

[kimburgess]

I don't think that's a good approach. The original purpose of this lib was to transparently insert the ability to pull from either the environment or a secret store, with no API changes. Tracking of what is accessed was introduced as a secondary feature purely to support auto-generation of CLI output.

The runtime tracking is solid (and arguably provides value for auditing purposes of accessed secrets), it's the hackery for the compile time resolution that introduces the fragility.

For that use case, a secondary module namespace may be used (ENV::Tracked or similar). This can house the accessor macros ([], []?) which then fetch from ENV. Alternatively for the exact case linked above, it may also just be simpler to just explicitly note the user-configurable env vars as a help string.

[stakach]

@xxkb would overriding the default crystal lang prelude to inject the old behaviour before kernel is loaded work?
I think we could probably configure an applications shard.yml to allow us to inject it early.

Tiny bit more work on our end, but would then provide the original behaviour - which was super handy

[kimburgess]

I don't think you can do it via a shard.yml, but you can inject a custom one as part of build command.

IMHO it seems to be an excessively convoluted workaround for just explicitly specifying what env vars are tweakable. Unless there is another use case that I'm missing?

[stakach]

it's just super useful as a bunch of ENV vars are hidden in different shards, so for any project it's reasonably difficult to work out what's available / being able to put anything into secrets

I think it's worth doing via the build command for our projects / adding support for a custom prelude in this project

[kimburgess]

Yep cool, the cross shard tracking is a good one.

From what I can see usage is limited to SG services only. What about rolling this functionality into the base app template. This way it can host the pre-prelude macro injection and ensure it's used in the compile step in the Dockerfile.

[stakach]

that's exactly what I was thinking!

[stakach]

to clarify, thinking this app hosts the prelude file and the base app template can ensure it's included
so that we only have to update the prelude file in one spot when something changes