kelseyhightower / envconfig

Golang library for managing configuration data from environment variables
MIT License
5.02k stars 377 forks source link

force use of `os.LookupEnv` when go1.5 (due to cacheing in go1.10) #108

Closed gedge closed 6 years ago

gedge commented 6 years ago

go1.10 caches env var accesses, but only when using os.LookupEnv (syscall.Getenv does not record env var accesses so cache seems reusable even if env vars have changed)

fixes #107

teepark commented 6 years ago

I think you misread the go test behavior change. In tests os.LookupEnv and os.Getenv now record the list of environment variables that were accessed in the test run. That list becomes part of the hashed cache key, along with files read and tests run.

It's not actually caching the results of the read (os.LookupEnv still calls through to syscall.Getenv regardless), except to use it as part of the hash key for the whole test.

But if only go 1.10 has this new behavior, it shouldn't be using any cache generated by the 1.5 test run (or 1.6, or 1.7, etc). Checking with the tip build in travis from before your commit, it looks like it wasn't using any cache. I believe travisci uses an independent environment for each version in the build matrix, so there shouldn't be any cache to collide with anyway.

gedge commented 6 years ago

I agree with your first paragraph (apart from the first sentence) and 2nd para. I also agree that Travis doesn't run go test twice to see this behaviour (cached or not).

Please re-read the issue I raised - apologies for editing, after the first draft.

The problem is that env_syscall.go is using syscall.Getenv (which skips using the cache), and this file was used by all non appengine system (which is 90+% of systems), so my testing was failing.

gedge commented 6 years ago

Worth mentioning that adding the go1.5 tag is also effective for later go versions.