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
FS#2086 - IS_TTY in the makefile is broken #6943
Comments
rdiez: I recently learnt that GNU Make has variables for this purpose: New variables: I have attached a patch to fix this issue in OpenWrt's build system. Please treat the patch as a suggestion. If something is not right with it, please fix this issue in whichever way you find suitable. |
rdiez: I have jumped through all the hoops and posted the patch with Git, which was automatically picked up by Patchwork. It is here: build: IS_TTY is now set according to GNU Make's MAKE_TERMOUT The reason why the patch was rejected is: Currently OpenWrt requires "GNU make v3.82", see include/prereq-build.mk http://lists.infradead.org/pipermail/openwrt-devel/2020-June/029772.html However, there is no other practical way to fix this issue, as far as I know. GNU Make v3.82 is 10 years old, and the one we would need, version 4.1, is 6 years old. I could try to change the Makefile logic to identify the GNU Make version, but the way OpenWrt makes me jump through hoops to post patches has discouraged me. Given that the IS_TTY logic is currently broken, I would still apply the patch. If an older version of GNU Make without MAKE_TERMOUT runs this line: export IS_TTY=$(if $(MAKE_TERMOUT),1,0) Such an old versoin will then always decide that the output is not a TTY, and you will lose message colours on your terminal. That is not a big deal. The flip side is that your build log files will no longer have terminal rubbish inside. |
aparcar0: This patch is required for this to work: |
The fix was applied in a015d91. |
rdiez:
File include/toplevel.mk tries to find out whether the build is running in an interactive console like this:
export IS_TTY=$(shell tty -s && echo 1 || echo 0)
That is the wrong way to do it. The code above is testing stdin, and not stdout. As a result, piping to a log file with a tool like 'tee' yields a log file that still contains the same \r codes etc. as when running on an interactive console.
Other tools like systemd's systemctl do it right by checking stdout. On an interactive console, run these 2 commands to see the difference:
systemctl --help
systemctl --help | tee test.txt
It turns out that checking stdout is not so easy to do inside a makefile. In the shell, you would normally check with "-t 1", but a$(shell ...) construct inside a makefile won't work, because $ (shell) always redirects stdout in order to capture the command's output.
See here for the gory details:
https://stackoverflow.com/questions/44097324/timing-of-makefile-include-statements-for-auto-generated-files/44133082
While there are workarounds for makefiles, I wouldn't go that far. I would just stop trying. and let the user decide with some variable. Maybe some day there will be a wrapper shell script, instead of using the makefile directly. In such a wrapper script it would be trivial to check for things like interactive terminal and/or terminal color support.
The text was updated successfully, but these errors were encountered: