diff mbox

[PATCHv2,4/4] toolchain: instrument external toolchain wrapper

Message ID 1374184490-13855-1-git-send-email-yann.morin.1998@free.fr
State Superseded
Headers show

Commit Message

Yann E. MORIN July 18, 2013, 9:54 p.m. UTC
From: "Yann E. MORIN" <yann.morin.1998@free.fr>

If BR_DEBUG_WRAPPER is set in the envirnment, dump the actual command
being exec()uted, to ease debugging issues with the wrapper.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

---
Changes v1 -> v2:
  - style fixes (Thomas)
---
 toolchain/toolchain-external/ext-toolchain-wrapper.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Peter Korsgaard July 19, 2013, 6:33 a.m. UTC | #1
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 Yann> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
 Yann> If BR_DEBUG_WRAPPER is set in the envirnment, dump the actual command
 Yann> being exec()uted, to ease debugging issues with the wrapper.

 Yann> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
 Yann> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

 Yann> ---
 Yann> Changes v1 -> v2:
 Yann>   - style fixes (Thomas)
 Yann> ---
 Yann>  toolchain/toolchain-external/ext-toolchain-wrapper.c | 8 ++++++++
 Yann>  1 file changed, 8 insertions(+)

 Yann> diff --git a/toolchain/toolchain-external/ext-toolchain-wrapper.c b/toolchain/toolchain-external/ext-toolchain-wrapper.c
 Yann> index 460f255..8e56414 100644
 Yann> --- a/toolchain/toolchain-external/ext-toolchain-wrapper.c
 Yann> +++ b/toolchain/toolchain-external/ext-toolchain-wrapper.c
 Yann> @@ -136,6 +136,14 @@ int main(int argc, char **argv)
 Yann>  	/* finish with NULL termination */
 Yann>  	*cur = NULL;
 
 Yann> +	if( getenv("BR_DEBUG_WRAPPER") ) {
 Yann> +		fprintf( stderr, "path='%s'\n", path );
 Yann> +		int i;
 Yann> +		for( i=0; args[i]; i++ ) {
 Yann> +			fprintf(stderr, "argv[%d]='%s'\n", i, args[i] );
 Yann> +		}
 Yann> +	}
 Yann> +

That's still not really the code style used elsewhere in this file.

You could argue that this feature isn't really needed as you can just
run 'strace -s 1000 -e execve <wrapper>' for basically the same
information, but OK - This is perhaps a bit more user friendly.

With that in mind, I think a more sensible output format is something
you can directly cut'n'paste and execute in the shell (after perhaps
tweaking something), so I've changed it to simply print the args space
seperated like this:

BR_DEBUG_WRAPPER=1 ./output/host/usr/bin/arm-none-linux-gnueabi-gcc --version
Executing /home/peko/source/buildroot/output/host/opt/ext-toolchain/bin/arm-none-linux-gnueabi-gcc --sysroot /home/peko/source/buildroot/output/host/usr/arm-buildroot-linux-gnueabi/sysroot -march=armv5te -mtune=arm926ej-s -mabi=aapcs-linux -mfloat-abi=soft -msoft-float -marm -pipe --version
arm-none-linux-gnueabi-gcc (Sourcery CodeBench Lite 2013.05-24) 4.7.3
Copyright (C) 2012 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Committed with these changes, thanks.
Yann E. MORIN July 19, 2013, 4:13 p.m. UTC | #2
Peter, All,

On 2013-07-19 08:33 +0200, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
> 
>  Yann> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>  Yann> If BR_DEBUG_WRAPPER is set in the envirnment, dump the actual command
>  Yann> being exec()uted, to ease debugging issues with the wrapper.
> 
>  Yann> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>  Yann> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
>  Yann> ---
>  Yann> Changes v1 -> v2:
>  Yann>   - style fixes (Thomas)
>  Yann> ---
>  Yann>  toolchain/toolchain-external/ext-toolchain-wrapper.c | 8 ++++++++
>  Yann>  1 file changed, 8 insertions(+)
> 
>  Yann> diff --git a/toolchain/toolchain-external/ext-toolchain-wrapper.c b/toolchain/toolchain-external/ext-toolchain-wrapper.c
>  Yann> index 460f255..8e56414 100644
>  Yann> --- a/toolchain/toolchain-external/ext-toolchain-wrapper.c
>  Yann> +++ b/toolchain/toolchain-external/ext-toolchain-wrapper.c
>  Yann> @@ -136,6 +136,14 @@ int main(int argc, char **argv)
>  Yann>  	/* finish with NULL termination */
>  Yann>  	*cur = NULL;
>  
>  Yann> +	if( getenv("BR_DEBUG_WRAPPER") ) {
>  Yann> +		fprintf( stderr, "path='%s'\n", path );
>  Yann> +		int i;
>  Yann> +		for( i=0; args[i]; i++ ) {
>  Yann> +			fprintf(stderr, "argv[%d]='%s'\n", i, args[i] );
>  Yann> +		}
>  Yann> +	}
>  Yann> +
> 
> That's still not really the code style used elsewhere in this file.

Doh. What happenned? I've just looked at my tree here, adn the style
fixes are applied.

OK. All sorted. I did 'git commit --amend'. I should forgot to pass '-a'
also (or run 'git add' first).

Doh, too bad. Sorry for the inconvenience... :-(

> You could argue that this feature isn't really needed as you can just
> run 'strace -s 1000 -e execve <wrapper>' for basically the same
> information, but OK - This is perhaps a bit more user friendly.

My use-case was to see how the kernel build-system was calling the
wrapper. So I just ran:
    BR_DEBUG_WRAPPER=1 make
in my Buildroot build dir.

> With that in mind, I think a more sensible output format is something
> you can directly cut'n'paste and execute in the shell (after perhaps
> tweaking something), so I've changed it to simply print the args space
> seperated like this:

Yet, one of the motivation behind the \n-separated args was to easily
see the args, without having to 'parse' the command line with the eyes.

The Linux kernel is passing something like 40+ args to the wrapper, so
the line is getting rather long, and difficult to grok visually, while
the one-arg per line output made it very easy.

But OK, that's sane, too.

> Committed with these changes, thanks.

Again, sorry for the inconvenience. :-(

I already owed you one ${BEVERAGE} in Edimburgh; you can make that two,
now! ;-)

Regards,
Yann E. MORIN.
Peter Korsgaard July 19, 2013, 9:30 p.m. UTC | #3
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

Hi,

 >> That's still not really the code style used elsewhere in this file.

 Yann> Doh. What happenned? I've just looked at my tree here, adn the style
 Yann> fixes are applied.

 Yann> OK. All sorted. I did 'git commit --amend'. I should forgot to pass '-a'
 Yann> also (or run 'git add' first).

 Yann> Doh, too bad. Sorry for the inconvenience... :-(

No problem ;)

 >> You could argue that this feature isn't really needed as you can just
 >> run 'strace -s 1000 -e execve <wrapper>' for basically the same
 >> information, but OK - This is perhaps a bit more user friendly.

 Yann> My use-case was to see how the kernel build-system was calling the
 Yann> wrapper. So I just ran:
 Yann>     BR_DEBUG_WRAPPER=1 make
 Yann> in my Buildroot build dir.

So you are only interested in the arguments to the wrapper, or do you
want to see the final arguments to the real compiler?


 >> With that in mind, I think a more sensible output format is something
 >> you can directly cut'n'paste and execute in the shell (after perhaps
 >> tweaking something), so I've changed it to simply print the args space
 >> seperated like this:

 Yann> Yet, one of the motivation behind the \n-separated args was to easily
 Yann> see the args, without having to 'parse' the command line with the eyes.

 Yann> The Linux kernel is passing something like 40+ args to the wrapper, so
 Yann> the line is getting rather long, and difficult to grok visually, while
 Yann> the one-arg per line output made it very easy.

Ok, but a kernel build with 40+ lines per gcc invocation is presumably
also quite overwhelming?


 Yann> Again, sorry for the inconvenience. :-(

 Yann> I already owed you one ${BEVERAGE} in Edimburgh; you can make
 Yann> that two, now! ;-)

heh, sounds good ;)
Yann E. MORIN July 19, 2013, 9:39 p.m. UTC | #4
On 2013-07-19 23:30 +0200, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
>  >> You could argue that this feature isn't really needed as you can just
>  >> run 'strace -s 1000 -e execve <wrapper>' for basically the same
>  >> information, but OK - This is perhaps a bit more user friendly.
> 
>  Yann> My use-case was to see how the kernel build-system was calling the
>  Yann> wrapper. So I just ran:
>  Yann>     BR_DEBUG_WRAPPER=1 make
>  Yann> in my Buildroot build dir.
> 
> So you are only interested in the arguments to the wrapper, or do you
> want to see the final arguments to the real compiler?

No, I'm interested in the final, complete argv[*].

>  >> With that in mind, I think a more sensible output format is something
>  >> you can directly cut'n'paste and execute in the shell (after perhaps
>  >> tweaking something), so I've changed it to simply print the args space
>  >> seperated like this:
> 
>  Yann> Yet, one of the motivation behind the \n-separated args was to easily
>  Yann> see the args, without having to 'parse' the command line with the eyes.
> 
>  Yann> The Linux kernel is passing something like 40+ args to the wrapper, so
>  Yann> the line is getting rather long, and difficult to grok visually, while
>  Yann> the one-arg per line output made it very easy.
> 
> Ok, but a kernel build with 40+ lines per gcc invocation is presumably
> also quite overwhelming?

The trick is to wait it break, and re-run with BR_DEBUG_WRAPPER=1 set and
then inspect the reason for the breakage.

>  Yann> I already owed you one ${BEVERAGE} in Edimburgh; you can make
>  Yann> that two, now! ;-)
> 
> heh, sounds good ;)

Remember: for each ${BEVERAGE} I owe you, I get one too! :-)
/me is looking forward to it!

Regards,
Yann E. MORIN.
diff mbox

Patch

diff --git a/toolchain/toolchain-external/ext-toolchain-wrapper.c b/toolchain/toolchain-external/ext-toolchain-wrapper.c
index 460f255..8e56414 100644
--- a/toolchain/toolchain-external/ext-toolchain-wrapper.c
+++ b/toolchain/toolchain-external/ext-toolchain-wrapper.c
@@ -136,6 +136,14 @@  int main(int argc, char **argv)
 	/* finish with NULL termination */
 	*cur = NULL;
 
+	if( getenv("BR_DEBUG_WRAPPER") ) {
+		fprintf( stderr, "path='%s'\n", path );
+		int i;
+		for( i=0; args[i]; i++ ) {
+			fprintf(stderr, "argv[%d]='%s'\n", i, args[i] );
+		}
+	}
+
 	if (execv(path, args))
 		perror(path);