Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set stdout to line buffering #2580

Closed
SirLynix opened this issue Jul 18, 2022 · 14 comments
Closed

Set stdout to line buffering #2580

SirLynix opened this issue Jul 18, 2022 · 14 comments

Comments

@SirLynix
Copy link
Member

Is your feature request related to a problem? Please describe.

When running xmake on GitHub actions on Linux and macOS, you have to wait a few minutes to see the log.
image

This is because on Linux/macOS stdout is fully buffered on Github by default.

Describe the solution you'd like

XMake could enable line buffering using setvbuf(stdout, NULL, _IONBF, 0); to flush on line feed.

Describe alternatives you've considered

I'm not sure if it can be done outside of xmake.

Additional context

No response

@waruqi
Copy link
Member

waruqi commented Jul 18, 2022

I think it should be possible to control the stdout buffer size by creating a child process, like python -u.

@waruqi
Copy link
Member

waruqi commented Jul 26, 2022

You can try this patch, but I'm not sure if it works. #2605

xmake update -s github:xmake-io/xmake#stdio

@SirLynix
Copy link
Member Author

Unfortunately it doesn't work, same behavior as before (tested on Linux CI).

Isn't there some code missing there, since it tests for XMAKE_STDOUT_NBUF env? Or should I set it myself from CI?

@waruqi
Copy link
Member

waruqi commented Jul 26, 2022

I improved it, try it again.

export XMAKE_STDOUT_NBUF=0

@SirLynix
Copy link
Member Author

It seems to work, every line gets printed on Github CI directly.

However, this seems to disable completely stdout buffer, which could be problematic with verbose output. Couldn't we use _IOLBF (line buffering) by default? (it's a standard value).

setvbuf(stdout, tb_null, _IOLBF, BUFSIZ);

This is standard and can be used on Windows as well.

On Posix we can get a better value than BUFSIZE using

    struct stat stats;
    int fileno(FILE*);
    if(fstat(fileno(fp), &stats) == -1) { // POSIX only
        perror("fstat"); return EXIT_FAILURE;
    }
 
    printf("BUFSIZ is %d, but optimal block size is %ld\n", BUFSIZ, stats.st_blksize);
    if(setvbuf(fp, NULL, _IOFBF, stats.st_blksize) != 0) {
       perror("setvbuf failed"); // POSIX version sets errno
       return EXIT_FAILURE;
    }

@waruqi
Copy link
Member

waruqi commented Jul 27, 2022

if(setvbuf(fp, NULL, _IOFBF, stats.st_blksize) != 0) {

? _IOFBF?

@SirLynix
Copy link
Member Author

My mistake, I copy-pasted the code from the documentation. Should be _IOLBF for line-feed buffering

@waruqi
Copy link
Member

waruqi commented Jul 27, 2022

try it again. export XMAKE_STDOUT_LINEFLUSH=y

@waruqi
Copy link
Member

waruqi commented Jul 27, 2022

This is standard and can be used on Windows as well.

xmake do not use fwrite(stdout) on windows. It can be a little complicated to process. There is also an encoding conversion before writing to stdout, and a separate stream buffer

@SirLynix
Copy link
Member Author

It works great. What would be the issue of enabling this by default?

This is standard and can be used on Windows as well.

xmake do not use fwrite(stdout) on windows. It can be a little complicated to process. There is also an encoding conversion before writing to stdout, and a separate stream buffer

Okay, it's not an issue on Windows CI anyway.

@waruqi
Copy link
Member

waruqi commented Jul 27, 2022

I'm not sure if it has an impact on build performance or if there are any other side effects. For the time being it is better to control it through environment variables.

@SirLynix
Copy link
Member Author

SirLynix commented Jul 27, 2022

This is already the default behavior on most OS, and for reference this is done by Ninja as well:
https://github.com/ninja-build/ninja/blob/master/src/ninja.cc#L1518
https://github.com/ninja-build/ninja/blob/master/src/build_log.cc#L193

@waruqi
Copy link
Member

waruqi commented Jul 27, 2022

ok, I have enabled it by default.

@SirLynix
Copy link
Member Author

Thanks!

@waruqi waruqi added this to the v2.7.1 milestone Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants