Skip to content

Make output window configurable: BUILDKIT_TERMHEIGHT#4284

Merged
jedevc merged 1 commit intomoby:masterfrom
holzman:configurable-output-height
Oct 3, 2023
Merged

Make output window configurable: BUILDKIT_TERMHEIGHT#4284
jedevc merged 1 commit intomoby:masterfrom
holzman:configurable-output-height

Conversation

@holzman
Copy link
Copy Markdown
Contributor

@holzman holzman commented Sep 27, 2023

The output window was previously hard-coded to a height of 6; this patch makes it configurable at run-time by setting the BUILDKIT_TERMHEIGHT environment variable.

@holzman
Copy link
Copy Markdown
Contributor Author

holzman commented Sep 27, 2023

I like the tty output, but I normally have tall windows and found myself annoyed by only having 6 lines of
output.

@holzman holzman force-pushed the configurable-output-height branch from f772bfe to 98bc4eb Compare September 27, 2023 20:17
Copy link
Copy Markdown
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

Iiuc this is not the window or viewport height (that is detected automatically from tty), but how many log lines are visible for active steps in tty mode (if there is enough room). In that case, the more suitable name would be something like BUILDKIT_TTY_LOG_LINES or smth like that?

@tonistiigi
Copy link
Copy Markdown
Member

I like the tty output, but I normally have tall windows and found myself annoyed by only having 6 lines of
output.

Theoretically, this could be somewhat automatically detected by counting the active lines and comparing them with the screen height. Eg. if the screen height is 40 lines, the whole tty output is 10 lines, and there is only one active step then indeed the logs for the active step should not be limited by 6 lines but could take 2-3 times more room. If more lines go active or the screen is resized then it should reduce the limit though. Atm. this logic is more simplified, every active step gets a maximum of 6 lines and if it doesn't fit anymore then the logs for some steps are not shown anymore.

@holzman
Copy link
Copy Markdown
Contributor Author

holzman commented Sep 28, 2023

Iiuc this is not the window or viewport height (that is detected automatically from tty), but how many log lines are visible for active steps in tty mode (if there is enough room). In that case, the more suitable name would be something like BUILDKIT_TTY_LOG_LINES or smth like that?

Sure, I'll change it. I based the env var on the variable already in the code, which is confusingly named.

@holzman holzman force-pushed the configurable-output-height branch from 669a0e3 to e7aa5c2 Compare September 28, 2023 18:41
The output window was previously hard-coded to a height of 6;
this patch makes it configurable at run-time by setting the
BUILDKIT_TTY_LOG_LINES environment variable.

Signed-off-by: Burt Holzman <burt@fnal.gov>
@holzman holzman force-pushed the configurable-output-height branch from e7aa5c2 to 2677a22 Compare September 28, 2023 18:44
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.

3 participants