Set `TERM` to prevent `No entry for terminal type "unknown"` messages during fetch
ClosedPublic
Actions

Authored by alexmv on Dec 21 2016, 10:58 PM.

Details

Reviewers

epriestley

Group Reviewers

Blessed Reviewers

Commits

rP972604e0e59c: Set `TERM` to prevent `No entry for terminal type "unknown"` messages during…

Summary

Fetches cause output in /var/tmp/phd/log/daemons.log as
follows:

PHLOG: 'Unexpected output while updating repository "rREPONAME": No entry for terminal type "unknown";
using dumb terminal settings.
' at [/path/to/phabricator/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php:455]

These warnings come from PHP itself. Silence these warnings by providing a
known value for TERM before shelling out to the PHP script.

See also D9744 (reverted in D11644) and T4990/T7119, which are a similar issue,
but in the pre-receive hooks, not the pull daemons.

Test Plan

Enabled in production, observed errors to be silenced and
no SSH hangs

Diff Detail

Repository

rP Phabricator

Branch

silence-fetch-warnings (branched from master)

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 15021
Build 19704: Run Core Tests
Build 19703: arc lint + arc unit

Event Timeline

alexmv created this revision.Dec 21 2016, 10:58 PM

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptDec 21 2016, 10:58 PM

Herald added a subscriber: epriestley. · View Herald Transcript

An alternative approach might be to add this by default to all subprocesses in PhutilExecutableFuture, but that feels a little muddy since this issue is unique to PHP subprocesses.

I think we only execute PHP subprocesses in a couple of cases -- the ones that spring to mind are here, and from the Daemon Overseer. If that one eventually turns out to be causing problems we could try lifting this out and doing it in PhutilExecutableFuture or similar but I think this is fine for now. Thanks!

This revision is now accepted and ready to land.Dec 21 2016, 11:07 PM

alexmv closed this revision.Dec 21 2016, 11:17 PM

epriestley mentioned this in T12070: phd no longer recognizes PHABRICATOR_ENV?.Jan 5 2017, 10:14 PM

epriestley mentioned this in D17148: Explicitly pass PHABRICATOR_ENV and PHABRICATOR_INSTANCE to subprocesses.Jan 5 2017, 10:33 PM

epriestley mentioned this in rPHUc5848b71c10f: Explicitly pass PHABRICATOR_ENV and PHABRICATOR_INSTANCE to subprocesses.Jan 5 2017, 10:48 PM