diff mbox

ct-ng build debug shell

Message ID 20121015153110.GA31836@sig21.net
State Accepted
Commit 3a7b2eee9dcd
Headers show

Commit Message

Johannes Stezenbach Oct. 15, 2012, 3:31 p.m. UTC
Add experimental debug config option to make crosstool-NG
drop into shell prompt on build errors instead of
just aborting the build.

Signed-off-by: Johannes Stezenbach <js@sig21.net>


--
For unsubscribe information see http://sourceware.org/lists.html#faq

Comments

Yann E. MORIN Oct. 15, 2012, 7:53 p.m. UTC | #1
Johannes, All,

Here is an alternate implementation of debug-shell, that I was working on
following your previous submission.

I prefer it over yours (even the revised one), becasue:
  - it does not duplicate CT_DoExecLog
  - it is integrated in the fault handler
  - it messes up with the trap, but in the fault handler, not
    in the normal code path
  
It's still not finished ( needs eg.: s/bash/${bash}/ ), and I did not yet
fully tested it, but it seems all is fitted in place.

I was working on it, since you said "I'm out of time atm and can't finish it."
Of course, your initial submission was of great help to come up with this
alternate solution! Thank you! :-)

Regards,
Yann E. MORIN.

--
For unsubscribe information see http://sourceware.org/lists.html#faq
Yann E. MORIN Oct. 15, 2012, 9:45 p.m. UTC | #2
Johannes, All,

Here is the final implementation.

As you did the initial work, I kept your SoB-line.
Tell me if you want me to remove it.

Regards,
Yann E. MORIN.

--
For unsubscribe information see http://sourceware.org/lists.html#faq
Johannes Stezenbach Oct. 16, 2012, 10:24 a.m. UTC | #3
Hi Yann,

On Mon, Oct 15, 2012 at 09:53:55PM +0200, Yann E. MORIN wrote:
> Here is an alternate implementation of debug-shell, that I was working on
> following your previous submission.
> 
> I prefer it over yours (even the revised one), becasue:
>   - it does not duplicate CT_DoExecLog
>   - it is integrated in the fault handler
>   - it messes up with the trap, but in the fault handler, not
>     in the normal code path
>   
> It's still not finished ( needs eg.: s/bash/${bash}/ ), and I did not yet
> fully tested it, but it seems all is fitted in place.
> 
> I was working on it, since you said "I'm out of time atm and can't finish it."
> Of course, your initial submission was of great help to come up with this
> alternate solution! Thank you! :-)

That's fine, I had a small amount of spare time yesterday and
brushed up my patches since it looked like noone else would do it.
But I like you version actually better than mine.
I'll test it soo but I can't make any promises as to when I get around...

Thanks
Johannes

--
For unsubscribe information see http://sourceware.org/lists.html#faq
Yann E. MORIN Oct. 16, 2012, 6:53 p.m. UTC | #4
Johannes, All,

On Tuesday 16 October 2012 Johannes Stezenbach wrote:
> On Mon, Oct 15, 2012 at 09:53:55PM +0200, Yann E. MORIN wrote:
> > Here is an alternate implementation of debug-shell, that I was working on
> > following your previous submission.
[--SNIP--]
> > I was working on it, since you said "I'm out of time atm and can't finish it."
> > Of course, your initial submission was of great help to come up with this
> > alternate solution! Thank you! :-)
> 
> That's fine, I had a small amount of spare time yesterday and
> brushed up my patches since it looked like noone else would do it.
> But I like you version actually better than mine.
> I'll test it soo but I can't make any promises as to when I get around...

OK, I'll wait a bit for your ACK, then. Thank you! :-)

Regards,
Yann E. MORIN.
Johannes Stezenbach Oct. 17, 2012, 10:15 a.m. UTC | #5
Hi Yann,

On Tue, Oct 16, 2012 at 08:53:29PM +0200, Yann E. MORIN wrote:
> On Tuesday 16 October 2012 Johannes Stezenbach wrote:
> > On Mon, Oct 15, 2012 at 09:53:55PM +0200, Yann E. MORIN wrote:
> > > Here is an alternate implementation of debug-shell, that I was working on
> > > following your previous submission.
> [--SNIP--]
> > > I was working on it, since you said "I'm out of time atm and can't finish it."
> > > Of course, your initial submission was of great help to come up with this
> > > alternate solution! Thank you! :-)
> > 
> > That's fine, I had a small amount of spare time yesterday and
> > brushed up my patches since it looked like noone else would do it.
> > But I like you version actually better than mine.
> > I'll test it soo but I can't make any promises as to when I get around...
> 
> OK, I'll wait a bit for your ACK, then. Thank you! :-)

My patch had this test:

+       if [ -t 0 -a -t 6 -a -t 2 ]; then
+              ...
+       else
+               CT_DoLog WARN "CT_DEBUG_CT_FIXUP_SHELL disabled due to I/O redirection"
+       fi

I haven't tested what happens if you run "c-ng build |& tee log" and
then try to run an interactive shell, but I guess it can't work?

Then, I'm running in this:

[INFO ]  Checking C library configuration
[ERROR]    You did not provide an eglibc config file!
[ERROR]
[ERROR]  >>
[ERROR]  >>  Build failed in step '(top-level)'
[ERROR]  >>
[ERROR]  >>  Error happened in: main[scripts/crosstool-NG.sh@585]


Current command (unknown), exited with error code: 1
Please fix it up and finish by exiting the shell with one of these values:
    1  fixed, continue with next build command
    3  abort build

ct-ng:~/tmp/tc/build>

Where scripts/crosstool-NG.sh@585 is "for step in ${CT_STEPS}; do".
I guess we can live with that but since there is no command
to fix or re-run it is a bit odd.


Best Regards
Johannes

--
For unsubscribe information see http://sourceware.org/lists.html#faq
Johannes Stezenbach Oct. 17, 2012, 11:13 a.m. UTC | #6
Hi,

and another mino issue:  when it runs into an error it prints:

Current command:
  'make' '-j5' 'all'
exited with error code: 2


It would be more useful to print a command that could be copy&pasted.


Best Regards
Johannes

--
For unsubscribe information see http://sourceware.org/lists.html#faq
Yann E. MORIN Oct. 17, 2012, 11:19 a.m. UTC | #7
Johannes, All,

On Wednesday 17 October 2012 13:13:06 Johannes Stezenbach wrote:
> and another mino issue:  when it runs into an error it prints:
> 
> Current command:
>   'make' '-j5' 'all'
> exited with error code: 2
> 
> 
> It would be more useful to print a command that could be copy&pasted.

Well, that *can* be copy-pasted. Especially if there are arguments with
spaces, then they are properly quoted.

For example, try this at your prompt:
    $ 'printf' '==%s==\n' 'arg1' 'arg 2'
    ==arg1==
    ==arg 2==

Regards,
Yann E. MORIN.
Yann E. MORIN Oct. 17, 2012, 11:38 a.m. UTC | #8
Johannes, All,

On Wednesday 17 October 2012 12:15:21 Johannes Stezenbach wrote:
> > > On Mon, Oct 15, 2012 at 09:53:55PM +0200, Yann E. MORIN wrote:
> > > > Here is an alternate implementation of debug-shell, that I was working on
> > > > following your previous submission.

> My patch had this test:
> 
> +       if [ -t 0 -a -t 6 -a -t 2 ]; then
> +              ...
> +       else
> +               CT_DoLog WARN "CT_DEBUG_CT_FIXUP_SHELL disabled due to I/O redirection"
> +       fi
> 
> I haven't tested what happens if you run "c-ng build |& tee log" and
> then try to run an interactive shell, but I guess it can't work?

Right, I'll see what I can do to add ths check.

> Then, I'm running in this:
> 
> [INFO ]  Checking C library configuration
> [ERROR]    You did not provide an eglibc config file!
> [ERROR]
> [ERROR]  >>
> [ERROR]  >>  Build failed in step '(top-level)'
> [ERROR]  >>
> [ERROR]  >>  Error happened in: main[scripts/crosstool-NG.sh@585]
> 
> 
> Current command (unknown), exited with error code: 1
> Please fix it up and finish by exiting the shell with one of these values:
>     1  fixed, continue with next build command
>     3  abort build
> 
> ct-ng:~/tmp/tc/build>
> 
> Where scripts/crosstool-NG.sh@585 is "for step in ${CT_STEPS}; do".
> I guess we can live with that but since there is no command
> to fix or re-run it is a bit odd.

That's the question: for commands that are not run via CT_DoExecLog, do we
want to spawn the debug-shell anyway, even if we can't display the command
that failed? My opinion is: yes, because we ant to debug the fscking b!tch,
even it it means a bit of rummaging...

At least, with the location in the error message and/or the last INFO line
of the log, it should be relatively  easy to find the real location of the
original error.

And remember that the debug-shell is just that: a debug-shell. Stumbling
across such a case can be fixed by adding a CT_DoExecLog in front of the
offending command.

In this specific case, I don't see what's wrong after just a quick glance,
but I think it's better to fix CT_TestOrAbort (and the likes) to properly
fail. I'll look at it tonight, when back home.

Thanks for the review!

Regards,
Yann E. MORIN.
Johannes Stezenbach Oct. 17, 2012, 1:54 p.m. UTC | #9
Hi Yann,

On Wed, Oct 17, 2012 at 01:38:38PM +0200, Yann E. MORIN wrote:
> On Wednesday 17 October 2012 12:15:21 Johannes Stezenbach wrote:
> > 
> > Current command (unknown), exited with error code: 1
> > Please fix it up and finish by exiting the shell with one of these values:
> >     1  fixed, continue with next build command
> >     3  abort build
> > 
> > ct-ng:~/tmp/tc/build>
> > 
> > Where scripts/crosstool-NG.sh@585 is "for step in ${CT_STEPS}; do".
> > I guess we can live with that but since there is no command
> > to fix or re-run it is a bit odd.
> 
> That's the question: for commands that are not run via CT_DoExecLog, do we
> want to spawn the debug-shell anyway, even if we can't display the command
> that failed? My opinion is: yes, because we ant to debug the fscking b!tch,
> even it it means a bit of rummaging...
> 
> At least, with the location in the error message and/or the last INFO line
> of the log, it should be relatively  easy to find the real location of the
> original error.
> 
> And remember that the debug-shell is just that: a debug-shell. Stumbling
> across such a case can be fixed by adding a CT_DoExecLog in front of the
> offending command.
> 
> In this specific case, I don't see what's wrong after just a quick glance,
> but I think it's better to fix CT_TestOrAbort (and the likes) to properly
> fail. I'll look at it tonight, when back home.

OK, guess it was just a bit unexpected for me, but it's OK.
You've got my ACK on the patch.

Thank you,
Johannes

--
For unsubscribe information see http://sourceware.org/lists.html#faq
diff mbox

Patch

diff -r c94bf1e11db2 config/global/ct-behave.in
--- a/config/global/ct-behave.in	Mon Oct 15 11:48:02 2012 +0200
+++ b/config/global/ct-behave.in	Mon Oct 15 17:24:08 2012 +0200
@@ -71,6 +71,25 @@ 
       further doesn't gain much, and takes far more time (believe me, I've
       got figures here! :-) ).
 
+config DEBUG_CT_FIXUP_SHELL
+    bool
+    prompt "Drop into shell prompt on build errors"
+    depends on EXPERIMENTAL
+    help
+      By default. crosstool-NG terminates the build when a build
+      command fails.  When this option is selected, crosstool-NG
+      instead drops into a shell prompt, with the environment set
+      up appropriately to re-run build commands manually to debug
+      the failure or even hot-fix it.  You then have three choices,
+      which you select by the shell exit code:
+        exit 1: you hot-fixed the issue, continue with the next build command
+        exit 2: ask crosstool-NG to re-run the failed build command
+        exit 3: ask crosstool-NG to abort the build
+      Other exit codes and ^D just cause crosstool-NG to restart the
+      shell and print a helpful message.
+      Note that this feature is disabled if stdin, stdout or stderr
+      are not to a terminal.
+
 config NO_OVERIDE_LC_MESSAGES
     bool
     prompt "Do *not* overide LC_MESSAGES (EXPERIMENTAL)"
diff -r c94bf1e11db2 scripts/crosstool-NG.sh.in
--- a/scripts/crosstool-NG.sh.in	Mon Oct 15 11:48:02 2012 +0200
+++ b/scripts/crosstool-NG.sh.in	Mon Oct 15 17:24:08 2012 +0200
@@ -25,6 +25,15 @@ 
 . .config.2
 # Yes! We can do full logging from now on!
 
+if [ "${CT_DEBUG_CT_FIXUP_SHELL}" = "y" ]; then
+	# Note: stdout is saved in fd 6
+	if [ -t 0 -a -t 6 -a -t 2 ]; then
+		CT_DoExecLog() { CT_DoExecLog_WithFixupShell "$@"; }
+	else
+		CT_DoLog WARN "CT_DEBUG_CT_FIXUP_SHELL disabled due to I/O redirection"
+	fi
+fi
+
 # Override the locale early, in case we ever translate crosstool-NG messages
 if [ -z "${CT_NO_OVERIDE_LC_MESSAGES}" ]; then
     export LC_ALL=C
diff -r c94bf1e11db2 scripts/functions
--- a/scripts/functions	Mon Oct 15 11:48:02 2012 +0200
+++ b/scripts/functions	Mon Oct 15 17:24:08 2012 +0200
@@ -175,6 +175,61 @@ 
     [ $? -eq 0 ]
 }
 
+# Variant of CT_DoExecLog for CT_DEBUG_CT_FIXUP_SHELL=y
+CT_DoExecLog_WithFixupShell() {
+    local level="$1"
+    local result
+    shift
+    (
+    for i in "$@"; do
+        tmp_log+="'${i}' "
+    done
+    while true; do
+        case "${1}" in
+            *=*)    eval export "'${1}'"; shift;;
+            *)      break;;
+        esac
+    done
+    trap ERR
+    while true; do
+        CT_DoLog DEBUG "==> Executing: ${tmp_log}"
+        "${@}" 2>&1 |CT_DoLog "${level}"
+        result=$?
+        if [ $result -eq 0 ]; then
+            break
+        fi
+        (
+            exec >&6
+            echo "command error $result:"
+            echo "$@"
+            echo "please fix it up and finish by exiting the shell"
+            while true; do
+                bash --rcfile <(echo "PS1='ct-ng:\w> '") -i
+                result=$?
+                case $result in
+                    1)  result=0; break;;
+                    2)  break;;
+                    3)  break;;
+                    *)  echo "please exit with one of these values:"
+                        echo "1  fixed, continue with next build command"
+                        echo "2  repeat this build command"
+                        echo "3  abort build"
+                        ;;
+                esac
+            done
+            exit $result
+        )
+        result=$?
+        if [ $result -ne 2 ]; then
+            break
+        fi
+    done
+    exit $result
+    )
+    # Catch failure of the sub-shell
+    [ $? -eq 0 ]
+}
+
 # Tail message to be logged whatever happens
 # Usage: CT_DoEnd <level>
 CT_DoEnd()