Resolve the socket path in Server's repr instead of hard-coding /tmp - #727
Open
tony wants to merge 1 commit into
Open
Resolve the socket path in Server's repr instead of hard-coding /tmp#727tony wants to merge 1 commit into
tony wants to merge 1 commit into
Conversation
why: The repr fell through to a hard-coded /tmp/tmux-<euid>/default when neither socket_name nor socket_path was given, which is every bare Server(). tmux resolves its socket directory from $TMUX_TMPDIR, so under a test harness, sandbox, or container the repr named a socket the object was not talking to -- in tracebacks, --showlocals banners, and logs, which is where someone is trying to tell servers apart. Closes #723. what: - Add libtmux._internal.env.resolve_socket_path(), which resolves tmux-<euid>/<socket_name or "default"> under $TMUX_TMPDIR, else /tmp ($TMPDIR is not consulted, matching tmux) - Use it for the Server.__repr__ fall-through, and for the socket unlink in pytest_plugin._reap_test_server, which had to compute it inline - Drop the getattr(self, 'socket_name', 'default') in the socket_name branch, which had already established socket_name is not None - Add regression tests setting $TMUX_TMPDIR via monkeypatch
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #727 +/- ##
==========================================
- Coverage 52.45% 52.28% -0.17%
==========================================
Files 26 26
Lines 3729 3737 +8
Branches 747 747
==========================================
- Hits 1956 1954 -2
- Misses 1469 1479 +10
Partials 304 304 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
repr(Server())naming a socket the object is not talking to. The fall-through branch hard-coded/tmp/tmux-<euid>/default, but tmux resolves its socket directory from$TMUX_TMPDIR, so under a test harness, a sandbox, a container, or anyone withTMUX_TMPDIRexported the repr was simply wrong.libtmux._internal.env.resolve_socket_path(), which computestmux-<euid>/<socket_name or "default">under$TMUX_TMPDIR, falling back to/tmp.pytest_plugin._reap_test_server, which carried a comment explaining it had to recompute whatServerwould not tell it. Both callers now share one resolver.getattr(self, 'socket_name', 'default')in thesocket_namebranch that ran afterself.socket_name is not Nonehad already been established.Display-only: which socket the object talks to is unchanged.
Why the repr matters
A repr is what appears in tracebacks,
--showlocalsbanners, logs, andprint(server)— precisely where someone is trying to work out which server they are looking at. Naming the wrong socket there is worse than naming none.Before / After
Under a
TMUX_TMPDIR:With
TMUX_TMPDIRunset the output is byte-identical to before, which is why the three shipped doctests that assert this repr needed no edit.Design decisions
$TMUX_TMPDIRor/tmp, and nothing else.TMPDIRis deliberately not consulted, because tmux does not consult it either —man tmux: "the sockets are all created in a directory tmux-UID under the directory given by TMUX_TMPDIR or in /tmp". Confirmed empirically on tmux 3.7b:TMPDIR=<dir> tmux -L probestill lands in/tmp/tmux-1000/probe. ConsultingTMPDIRwould have broken the existing doctests on any machine that exports it.Computed, not observed. The helper says where tmux would put the socket, not that a daemon is listening. Its docstring says so and points live callers at the
#{socket_path}format instead.Server.__repr__is the only non-test consumer, and it is display-only.Lives in
_internal.libtmux._internal.envalready owns the tmux-environment readers (resolve_env,socket_path_from_env,pane_id_from_env), so this adds no public API surface.Empty
TMUX_TMPDIRis treated as unset. Verified against tmux 3.7b, whereTMUX_TMPDIR= tmux -L probealso lands in/tmp.Scope
Repairing the unreachable
socket_pathauto-derivation inServer.__init__is deliberately not in this PR — that is a behaviour change with wider blast radius, tracked separately. This resolver would make it a small change, though.Test plan
test_repr_socket_path_honors_tmux_tmpdir— setsTMUX_TMPDIRviamonkeypatchand asserts the repr names a path inside it; fails onmastertest_repr_socket_name— covers thegetattrremoval (behaviour-preserving, so it passes either way)Server.__repr__andresolve_socket_path, which are collected and runREADME.md,docs/quickstart.md,docs/api/libtmux.neo.mduv run ruff check ./uv run ruff format ./uv run mypycleanuv run py.test --reruns 0cleanjust build-docscleanNote for the merger
The
CHANGESentry cites the issue number; swap it for this PR's number on merge.CHANGESwill conflict with the sibling PRs for #724, #725 and #726, which all add to the same block — keep both sides.