Add missing environment variables #129
Conversation
These environment variables are needed for subprocesses to do things like create lockfiles and all that!
bradhe
requested review from
Copilot,
jo-sm,
konstantinoscs,
sammuti and
socksy
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for temporary directory environment variables (TEMP and TMP) to the environment normalization function, ensuring that Python subprocesses can properly create temporary files and lock files.
- Adds
TEMPenvironment variable passthrough for Python subprocess compatibility - Adds
TMPenvironment variable passthrough to handle Windows environment variable variations
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
konstantinoscs
left a comment
konstantinoscs
left a comment
There was a problem hiding this comment.
LGTM. A useful comment from copilot?
|
|
||
| // Apparently, according to some random person on Stack Overflow, sometimes the var can be | ||
| // TEMP and sometimes it can be TMP. So uh...let's just grab both just in case. | ||
| let tmp = std::env::var("TMP").unwrap_or_default(); |
There was a problem hiding this comment.
I appreciate the honesty. We should probably try to verify this with some official documentation....somewhere?
There was a problem hiding this comment.
I spent 5 minutes trying to find something that wasn't Stack Overflow or Reddit posts complaining about it. Generally, seems like it dates back to DOS. There's some basic docs here that reference both lol https://learn.microsoft.com/en-us/windows/deployment/usmt/usmt-recognized-environment-variables
These environment variables are needed for subprocesses to do things like create lock files and all that!