Skip to content

backend,termstatus: Unify foreground/background detection#5493

Merged
MichaelEischer merged 1 commit into
restic:masterfrom
greatroar:ioctl
Sep 8, 2025
Merged

backend,termstatus: Unify foreground/background detection#5493
MichaelEischer merged 1 commit into
restic:masterfrom
greatroar:ioctl

Conversation

@greatroar

@greatroar greatroar commented Sep 7, 2025

Copy link
Copy Markdown
Contributor

What does this PR change? What problem does it solve?

PR #5358 reintroduced a version of the TIOCGPGRP ioctl call that works on all Unix platforms except Linux, due to a bug/inconsistency in x/sys/unix. This commit fixes that by introducing termstatus.Tcgetpgrp.

It also introduces termstatus.Getpgrp to deal with the different signature of unix.Getpgrp in Solaris vs. all other Unix platforms, so that some AIX/Solaris-specific code can be removed elsewhere and foreground/background detection is done the same everywhere except on Windows.

I tested this on Linux and checked that AIX, Darwin, FreeBSD and Solaris continue to compile.

Was the change previously discussed in an issue or on the forum?

The bug in x/sys/unix was first noticed in #4223, which was fixed by #4339.

Checklist

  • I have added tests for all code changes.
  • I have added documentation for relevant changes (in the manual).
  • There's a new file in changelog/unreleased/ that describes the changes for our users (see template).
  • I'm done! This pull request is ready for review.

@MichaelEischer

Copy link
Copy Markdown
Member

I see you've also noticed the bug in #5358 . While starting to fix it, I got a bit carried away and refactored most terminal-related code into a separate package: #5494 .

My suggestion would be to drop the tcgetpgrp() hack in my PR and use the approach from here. It's probably better to sort out the ioctl mess first and then I can rebase my PR afterwards.

Comment thread internal/ui/termstatus/getpgrp_sysv.go Outdated
@greatroar greatroar force-pushed the ioctl branch 4 times, most recently from 561c699 to 884ae8c Compare September 8, 2025 06:30
PR restic#5358 reintroduced a version of the TIOCGPGRP ioctl call that works
on all Unix platforms except Linux, due to a bug/inconsistency in
x/sys/unix. This commit fixes that by introducing termstatus.Tcgetpgrp.

It also introduces termstatus.Getpgrp and termstatus.Tcsetpgrp to deal
with the different signature of unix.Getpgrp in Solaris vs. all other
Unix platforms and an int-overflowing constant on AIX, so that some
AIX/Solaris-specific code can be removed elsewhere and
foreground/background detection is done the same everywhere except on
Windows.

@MichaelEischer MichaelEischer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks for fixing this once again!

@MichaelEischer MichaelEischer merged commit cc1fe6c into restic:master Sep 8, 2025
11 checks passed
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.

2 participants