Re: [ptxdist] [PATCH v2] ptxd_make_world_compile.sh: conditionally redirect stderr>stdout

[Michael Olbrich <m.olbrich@pengutronix.de>]

On Fri, May 19, 2017 at 10:40:48PM -0400, jon@ringle.org wrote:
> From: Jon Ringle <jringle@gridpoint.com>
> 
> I discovered after updating to a new version of ptxdist that all of my
> compile output including errors/warnings are now all going to stdout
> instead of having the errors/warning going to stderr.
> 
> This is causing some problem when I use qtcreator as my IDE because
> qtcreator will only filter stderr for errors/warnings to put into the
> Issues tab, where one can simply click on the error/warning and be taken
> right to the source line where the error/warning is found.
> 
> I found that this problem was introduced by commit
> 30b9267e35eea1c2edb4da0231a428bfa25b6766
> 
> This patch will conditionally redirect stderr>stdout on compile output
> if ptxdist has output synchronization enabled.

We do this to make sure the stdout and stderr are properly interleaved when
output sync is used. However, your use-case is certainly valid.

> This small program demonstrates the behavior with this patch applied:
> 
> $ cat local_src/testprog/testprog.c
> #include <stdio.h>
> #include <stdlib.h>
> 
> #error An error here
> int main(int argc, char *argv[])
> {
>         printf("Hello World, I'm testprog!\n");
> 
>         exit(EXIT_SUCCESS);
> }
> 
> $ ptxdist compile testprog >/dev/null
> testprog.c:4:2: error: #error An error here
>  #error An error here
>   ^
> make[1]: *** [testprog] Error 1
> make: *** [/home/jringle-admin/code/gpec/ec1k-rootfs/platform-EC1K/state/testprog.compile] Error 2
> 
> $ ptxdist compile --output-sync testprog >/dev/null
> make: *** [/home/jringle-admin/code/gpec/ec1k-rootfs/platform-EC1K/state/testprog.compile] Error 2
> 
> $ ptxdist compile --no-output-sync testprog >/dev/null
> testprog.c:4:2: error: #error An error here
>  #error An error here
>   ^
> make[1]: *** [testprog] Error 1
> make: *** [/home/jringle-admin/code/gpec/ec1k-rootfs/platform-EC1K/state/testprog.compile] Error 2
> 
> Signed-off-by: Jon Ringle <jringle@gridpoint.com>
> ---
> 
> v2 just removes an extra line that was put in the previous version of the patch
> 
>  bin/ptxdist                            |  6 +++++-
>  scripts/lib/ptxd_make_world_compile.sh | 10 ++++++----
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/bin/ptxdist b/bin/ptxdist
> index 1e3c53a..ed113c6 100755
> --- a/bin/ptxdist
> +++ b/bin/ptxdist
> @@ -2383,6 +2383,7 @@ setup_config() {
>  #      PTXDIST_PARALLELMFLAGS_INTERN
>  #      PTXDIST_PARALLELMFLAGS_EXTERN
>  #      PTXDIST_OUTPUT_SYNC
> +#      PTXDIST_STDERR_REDIRECT
>  #
>  setup_parallel() {
>  	# default no parallel for now
> @@ -2415,10 +2416,12 @@ setup_parallel() {
>  	fi
>  
>  	PTXDIST_OUTPUT_SYNC="${PTXDIST_OUTPUT_SYNC:-${PTXDIST_QUIET}}"
> +	PTXDIST_STDERR_REDIRECT=2
>  	if [ "${PTXDIST_OUTPUT_SYNC}" == "1" ]; then
>  		if "${PTXCONF_SETUP_HOST_MAKE}" -h | grep -q -- --output-sync; then
>  			PTXDIST_OUTPUT_SYNC="--output-sync="
>  		fi
> +		PTXDIST_STDERR_REDIRECT=1
>  	fi
>  	if [[ "${PTXDIST_OUTPUT_SYNC}" =~ [01] ]]; then
>  		unset PTXDIST_OUTPUT_SYNC

Call it PTXDIST_FD_LOGERR and export it in this function with the other
variables below.


> @@ -2720,7 +2723,8 @@ setup_export() {
>  		PTXDIST_OUTPUT_SYNC \
>  		PTXDIST_PEDANTIC \
>  		PTXDIST_QUIET \
> -		PTXDIST_VERBOSE
> +		PTXDIST_VERBOSE \
> +		PTXDIST_STDERR_REDIRECT
>  }
>  
>  
> diff --git a/scripts/lib/ptxd_make_world_compile.sh b/scripts/lib/ptxd_make_world_compile.sh
> index 9cc739e..cbaf355 100644
> --- a/scripts/lib/ptxd_make_world_compile.sh
> +++ b/scripts/lib/ptxd_make_world_compile.sh
> @@ -11,6 +11,8 @@
>  # call the compiler
>  #
>  ptxd_make_world_compile() {
> +
> +    ( exec 2>&${PTXDIST_STDERR_REDIRECT}
>      ptxd_make_world_init &&
>  
>      if [ -z "${pkg_build_dir}" ]; then
> @@ -28,7 +30,7 @@ ptxd_make_world_compile() {
>  	    "${ptx_build_python}" \
>  	    setup.py \
>  	    "${pkg_make_opt}"
> -	) 2>&1
> +	)
>  	;;
>  	meson)
>  	ptxd_eval \
> @@ -37,7 +39,7 @@ ptxd_make_world_compile() {
>  	    "${pkg_make_env}" \
>  	    ninja -C "${pkg_build_dir}" \
>  	    "${pkg_make_opt}" \
> -	    "${pkg_make_par}" 2>&1
> +	    "${pkg_make_par}"
>  	;;
>  	*)
>  	ptxd_eval \
> @@ -46,8 +48,8 @@ ptxd_make_world_compile() {
>  	    "${pkg_make_env}" \
>  	    "${MAKE}" -C "${pkg_build_dir}" \
>  	    "${pkg_make_opt}" \
> -	    "${pkg_make_par}" 2>&1
> +	    "${pkg_make_par}"
>  	;;
> -    esac
> +    esac )
>  }
>  export -f ptxd_make_world_compile

This is getting quite complex. And I think it should be consistent for all
build stages. How about this:

At the end of ptxd_make_world_init() call "exec 2>&${PTXDIST_FD_LOGERR}"
and remove all the other redirections in the build stages. Basically
reverting the commit you mentioned above.
And in ptxd_bailout() redirect to PTXDIST_FD_STDERR. This keeps the
redirection in one place and we still see the ptxdist errors in quiet mode.

Michael

> -- 
> 1.9.1
> 
> 
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de