finch
finch copied to clipboard
runfinch
fix: add stricter check for DOCKER_CONFIG var
Signed-off-by: Justin Alvarez [email protected]
Issue #, if available: Closes #126
Description of changes:
- Previously, this code was just checking for any substring containing
export DOCKER_CONFIG, now we check for a line starting with the fully configured DOCKER_CONFIG variable (likeexport DOCKER_CONFIG="/Users/justin/.finch")
Testing done:
-
added unit test for this scenario
-
I want to test this practically, but its difficult to change the user on my mac and keep the same VM. Thinking of ways to test before merging
-
[x] I've reviewed the guidance in CONTRIBUTING.md
License Acceptance
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
@pendo324 I was wondering if it would simplify things if the DOCKER_CONFIG variable was passed in as a Env variable in the lima config? Similar to https://github.com/runfinch/finch/pull/129
@pendo324 I was wondering if it would simplify things if the DOCKER_CONFIG variable was passed in as a Env variable in the lima config? Similar to #129
Sorry for the late response @ollypom, I was out for the holidays and I'm still catching up on things. I believe I tried this before but wasn't able to get it working because of the fact that the DOCKER_CONFIG is dependent on the $USER (since it's set to /Users/$USER/.finch).
Some of this complication is due to the fact that the Lima env map adds to /etc/environment, which can't evaluate variables, by definition.
It's definitely possible that I overlooked something though, open to changing this behavior as its kinda hacky the way it is currently. The other advantage of the way it is right now is that its dynamically evaluated on vm init, which means it will get updated in the rare case that the $USER changes (which is pretty hard to test actually, which is the reason I haven't tried to merge this yet).