Skip to content

Fall back from /dev/tty to /dev/stdout in buildall single-shim builds#14913

Open
revans2 wants to merge 2 commits into
NVIDIA:mainfrom
revans2:fix_buildall_single_shim_tty
Open

Fall back from /dev/tty to /dev/stdout in buildall single-shim builds#14913
revans2 wants to merge 2 commits into
NVIDIA:mainfrom
revans2:fix_buildall_single_shim_tty

Conversation

@revans2
Copy link
Copy Markdown
Collaborator

@revans2 revans2 commented May 29, 2026

Description

Small change that allows using buildall without a tty present. Better for automation or inside of a docker container.

Checklists

Documentation

  • Updated for new or modified user-facing features or behaviors
  • No user-facing change

Testing

  • Added or modified tests to cover new code paths
  • Covered by existing tests
    (Please provide the names of the existing tests in the PR description.)
  • Not required

Performance

  • Tests ran and results are added in the PR description
  • Issue filed with a link in the PR description
  • Not required

Single-shim/serial builds forced Maven output to /dev/tty, which has no
controlling terminal in non-interactive contexts (CI, containers, xargs
workers). Opening it fails and aborts the whole build. Fall back to
/dev/stdout when /dev/tty cannot be opened, and skip the on-failure log
tail for live streams since their output is already on screen.

Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
@revans2 revans2 self-assigned this May 29, 2026
@revans2 revans2 requested a review from a team as a code owner May 29, 2026 19:43
@revans2 revans2 added the build Related to CI / CD or cleanly building label May 29, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

Greptile Summary

This PR makes the build_single_shim function in build/buildall work in no-TTY environments (CI pipelines, Docker containers) by probing /dev/tty availability before deciding where Maven output is directed.

  • Replaces the unconditional LOG_FILE=\"/dev/tty\" assignment in serial/single-shim mode with a { : > /dev/tty; } 2>/dev/null probe: if /dev/tty is writable the behaviour is unchanged; otherwise LOG_FILE is set to empty string and output flows through the process's inherited stdout/stderr via exec 2>&1 in a subshell.
  • Wraps the Maven invocation in a subshell so exec redirects are scoped correctly, and updates the failure handler to use a case statement that skips the log-tail step for the two live-stream modes (\"\" and /dev/tty).

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to the serial/single-shim output-routing path and leaves parallel builds untouched.

The TTY probe is a well-established bash idiom, the subshell correctly scopes the exec redirects, and the three output modes (live TTY, inherited stdout, log file) are all handled consistently in both the normal and failure paths. No logic that affects the Maven invocation itself is changed.

No files require special attention.

Important Files Changed

Filename Overview
build/buildall Adds TTY probe to fall back to inherited stdout when /dev/tty is unavailable; subshell + exec pattern cleanly handles redirection for the three output modes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[build_single_shim called] --> B{BUILD_PARALLEL==1 OR NUM_SHIMS==1?}
    B -- Yes --> C{Try: > /dev/tty}
    C -- Success --> D[LOG_FILE=/dev/tty stream live to terminal]
    C -- Fail / no TTY --> E[LOG_FILE=empty stream to inherited stdout]
    B -- No / parallel --> F[LOG_FILE=mvn-build-VER.log write to file]
    D --> G[exec subshell with Maven]
    E --> G
    F --> G
    G -- Success --> H[Done]
    G -- Failure --> I{case LOG_FILE}
    I -- empty or /dev/tty --> J[output already visible, no tail needed]
    I -- file path --> K[echo filename + tail last 500 lines]
    J --> L[exit 255]
    K --> L
Loading

Reviews (2): Last reviewed commit: "Addressed review comments" | Re-trigger Greptile

Copy link
Copy Markdown
Collaborator

@amahussein amahussein left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the no-tty case.

Comment thread build/buildall Outdated
if { : > /dev/tty; } 2>/dev/null; then
LOG_FILE="/dev/tty"
else
LOG_FILE="/dev/stdout"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

falling back to /dev/stdout still reopens stdout by path, and that can fail in some non-interactive environments.
Would it be safer to make the no-tty path avoid a stdout filename entirely and just run Maven with the existing stdout, e.g. only redirect stderr with 2>&1? That should preserve the intended live output behavior without depending on /dev/stdout being reopenable.

Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
@revans2 revans2 requested a review from amahussein June 1, 2026 15:19
@revans2
Copy link
Copy Markdown
Collaborator Author

revans2 commented Jun 1, 2026

build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Related to CI / CD or cleanly building

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants