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.

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

Use mkstemp instead of tempnam #235

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ywwry66
Copy link

@ywwry66 ywwry66 commented Sep 11, 2023

Difference from 37bbe86: Set temporary directory path in runtime using environment variable: "TMP" for Windows and "TMPDIR" for Unix-like. This is because "P_tmpdir" does not expand correctly on Windows. If the environment variable is not set, falls back to "P_tmpdir".

Temp filename examples:

  • Windows 11: C:\Users<username>\AppData\Local\Temp\epdfinfo_xxxxxx
  • MSYS2 on Windows 11: <path_to_msys2>\tmp\epdfinfo_xxxxxx
  • macOS Ventura:/var/folders/xx/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/T/epdfinfo_xxxxxx

Closes #110

@ywwry66 ywwry66 force-pushed the remove-tempnam branch 2 times, most recently from 87f36cc to e7157a8 Compare September 11, 2023 03:01
Difference from 37bbe86: Set temporary directory path in runtime using
environment variable: "TMP" for Windows and "TMPDIR" for Unix-like.
This is because "P_tmpdir" does not expand correctly on Windows. If
the environment variable is not set, falls back to "P_tmpdir".

Temp filename examples:
- Windows 11: C:\Users\<username>\AppData\Local\Temp\epdfinfo_xxxxxx
- MSYS2 on Windows 11: <path_to_msys2>\tmp\epdfinfo_xxxxxx
- macOS Ventura:/var/folders/xx/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/T/epdfinfo_xxxxxx

Closes vedang#110
@ywwry66
Copy link
Author

ywwry66 commented Sep 11, 2023

CI failure is unrelated.

@vedang
Copy link
Owner

vedang commented Sep 19, 2023

Thank you, I have been waiting for someone to fix this on Windows correctly. I will review and merge it in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement mkstemp correctly for working against MSWindows
2 participants