Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

adds support for ${TMP} ${TEMP} and ${TMPDIR} in configs #2655

Merged
merged 2 commits into from
Jan 16, 2025
Merged

adds support for ${TMP} ${TEMP} and ${TMPDIR} in configs #2655

merged 2 commits into from
Jan 16, 2025

Conversation

andrewpmartinez
Copy link
Member

  • TMP/TEMP/TMPDIR are standards from windows, macOs, bsd, and linux
  • TMP/TEMP/TMPDIR are set for all oses to allow cross platform configs
  • Priotity order for selection is TMP/TEMP/TMPDIR
  • For windows if TMP/TEMP are not set C:\temp is the default

@andrewpmartinez
adds support for ${TMP} ${TEMP} and ${TMPDIR} in configs
2261a55 
- TMP/TEMP/TMPDIR are standards from windows, macOs, bsd, and linux
- TMP/TEMP/TMPDIR are set for all oses to allow cross platform configs
- Priotity order for selection is TMP/TEMP/TMPDIR
- For windows if TMP/TEMP are not set C:\temp is the default
@andrewpmartinez andrewpmartinez requested review from a team as code owners January 16, 2025 16:29
dovholuknf
dovholuknf reviewed Jan 16, 2025
View reviewed changes
common/config/env.go Outdated Show resolved Hide resolved
dovholuknf
dovholuknf approved these changes Jan 16, 2025
View reviewed changes
Copy link
Member

@dovholuknf dovholuknf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't "get it" but it looks good to me :)

@andrewpmartinez andrewpmartinez merged commit 870de5d into main Jan 16, 2025
40 checks passed
@andrewpmartinez andrewpmartinez deleted the support.tmp.dir.in.config branch January 16, 2025 18:56
@qrkourier
Copy link
Member

This causes a surprising result: a specified TMPDIR is silently ignored. Each temp dir var should be respected if already defined.

This assumes the OS default according to stdlib is the desired temporary dir. Still, it's a common practice to assign a var like TMPDIR in the parent environment by way of specifying a temporary directory for a particular purpose, which will be silently ignored by ziti and the OS default will be used instead of the specified directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dovholuknf dovholuknf dovholuknf approved these changes

@plorenz plorenz plorenz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@andrewpmartinez @qrkourier @plorenz @dovholuknf