Skip to content

Conversation

wowi42
Copy link
Contributor

@wowi42 wowi42 commented Sep 25, 2025

Fix #1458

According to POSIX standards, checks environment variables in this order:
1. TMPDIR (if set and accessible)
2. TMP (if set and accessible)
3. TEMP (if set and accessible)
4. Falls back to /tmp

  • Pull request is based on the default branch (3.x at this time)
  • Pull request includes tests for any new/updated operations/facts
  • Pull request includes documentation for any new/updated operations/facts
  • Tests pass (see scripts/dev-test.sh)
  • Type checking & code style passes (see scripts/dev-lint.sh)

Copy link
Member

@Fizzadar Fizzadar left a comment

Choose a reason for hiding this comment

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

Way nicer, slight change needed for backwards compatibiity (sorry) 🙏

elif [ -n "$TEMP" ] && [ -d "$TEMP" ] && [ -w "$TEMP" ]; then
echo "$TEMP"
else
echo "/tmp"
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't provide a default here, the fallback is specified at the config level currently (https://github.com/pyinfra-dev/pyinfra/blob/3.x/src/pyinfra/api/config.py#L29).

To be honest I think this way is nicer, but we can't break the config for backwards compatibility so if no matching env vars the fact still needs to return a blank string.

Choose a reason for hiding this comment

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

None of the TMPDIR|TMP|TEMP are set in my arch/zsh terminal or in another ubuntu/bash PC I checked.

I had the this issue and the initial problem was server.script not working. The underlying issue was that this fact returning empty. I remember seeing something about setting a default in the pyinfra code, but did not check in details. Still the script operation was failing (v3.5).

So if this returns a blank, my guess is it still would fail here in my machine, unless the script was fixed in the meantime to use the proper default (if that is how it should work).

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.

server.TmpDir fact is buggy and breaks things
3 participants