GNU bug report logs - #41914
[PATCH] Propagate return value of auto-loaded command

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 41914 in the body.
You can then email your comments to 41914 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tom de Vries <tdevries <at> suse.de>
To: bug-dejagnu <at> gnu.org
Subject: [PATCH] Propagate return value of auto-loaded command
Date: Wed, 17 Jun 2020 13:40:40 +0200

[Message part 1 (text/plain, inline)]

Hi,

I think I found a bug in proc unknown in lib/framework.exp.

Patch describing the problem and fixing it attached below.

Thanks,
- Tom

[0001-Propagate-return-value-of-auto-loaded-command.patch (text/x-patch, inline)]

Propagate return value of auto-loaded command

Consider a library file foo.tcl:
...
$ cat lib/foo.tcl
proc foo { } {
    return "foo-return"
}
...
and a test-case test.tcl:
...
$ cat test.tcl
\#!/usr/bin/tclsh

auto_mkindex lib *.tcl

lappend auto_path [pwd]/lib

set res [foo]
puts "RES: $res"
...
which gives us:
...
$ ./test.tcl
RES: foo-return
...

When overriding the ::unknown command using:
...
rename ::unknown ::tcl_unknown
proc unknown args {
    if {[catch {uplevel 1 ::tcl_unknown $args} msg]} {
        puts "ERROR: proc \"$args\" does not exist."
        exit
    }
}
...
we have instead:
...
$ ./test.tcl
RES:
...

What is missing, is the propagation of the return value of foo, like this:
...
         puts "ERROR: proc \"$args\" does not exist."
         exit
     }
-}
+} else {
+    return $msg
+}
...

Fix unknown in lib/framework.exp accordingly.

ChangeLog:

2020-06-17  Tom de Vries  <tdevries <at> suse.de>

	* lib/framework.exp (unknown): Propagate return value of auto-loaded
	command.

---
 lib/framework.exp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/framework.exp b/lib/framework.exp
index e6ce197..c9875d2 100644
--- a/lib/framework.exp
+++ b/lib/framework.exp
@@ -272,6 +272,9 @@ proc unknown args {
 	}
 	set exit_status 2
 	log_and_exit
+    } else {
+	# Propagate return value.
+	return $msg
     }
 }
 

Message #10 received at 41914-done <at> debbugs.gnu.org (full text, mbox):

From: Jacob Bachmeyer <jcb62281 <at> gmail.com>
To: Tom de Vries <tdevries <at> suse.de>
Cc: 41914-done <at> debbugs.gnu.org
Subject: Re: bug#41914: [PATCH] Propagate return value of auto-loaded command
Date: Wed, 17 Jun 2020 18:19:03 -0500

Tom de Vries wrote:
> I think I found a bug in proc unknown in lib/framework.exp.
>
> Patch describing the problem and fixing it attached below.
>   

I found and fixed the same bug while working on the patch for bug #41824.


-- Jacob

Message #13 received at 41914 <at> debbugs.gnu.org (full text, mbox):

From: Tom de Vries <tdevries <at> suse.de>
To: 41914 <at> debbugs.gnu.org
Subject: Re: bug#41914: closed (Re: bug#41914: [PATCH] Propagate return value
 of auto-loaded command)
Date: Thu, 18 Jun 2020 09:40:10 +0200

On 6/18/20 1:20 AM, GNU bug Tracking System wrote:
> Your bug report
> 
> #41914: [PATCH] Propagate return value of auto-loaded command
> 
> which was filed against the dejagnu package, has been closed.
> 
> The explanation is attached below, along with your original report.
> If you require more details, please reply to 41914 <at> debbugs.gnu.org.
> 

AFAICT, no fix was committed to master. So, I'm not sure I understand
why this bug was closed.

Thanks,
- Tom

Message #16 received at 41914-done <at> debbugs.gnu.org (full text, mbox):

From: Jacob Bachmeyer <jcb62281 <at> gmail.com>
To: Tom de Vries <tdevries <at> suse.de>
Cc: 41914-done <at> debbugs.gnu.org
Subject: Re: bug#41914: closed (Re: bug#41914: [PATCH] Propagate return value
 of auto-loaded command)
Date: Thu, 18 Jun 2020 16:50:30 -0500

Tom de Vries wrote:
> On 6/18/20 1:20 AM, GNU bug Tracking System wrote:
>   
>> Your bug report
>>
>> #41914: [PATCH] Propagate return value of auto-loaded command
>>
>> which was filed against the dejagnu package, has been closed.
>>
>> The explanation is attached below, along with your original report.
>> If you require more details, please reply to 41914 <at> debbugs.gnu.org.
>>
>>     
>
> AFAICT, no fix was committed to master. So, I'm not sure I understand
> why this bug was closed.

It was closed because a fix had been rolled into the fix for #41824.  
Linus' Law applied here:  we both found the same bug and actually fixed 
it the same way.  I merged the comment and ChangeLog entry from your 
patch into the temporary "PR41824" branch.

Is there a better way to record this circumstance in debbugs than 
closing the second bug?


-- Jacob

Message #19 received at 41914-done <at> debbugs.gnu.org (full text, mbox):

From: Tom de Vries <tdevries <at> suse.de>
To: jcb62281 <at> gmail.com
Cc: 41914-done <at> debbugs.gnu.org
Subject: Re: bug#41914: closed (Re: bug#41914: [PATCH] Propagate return value
 of auto-loaded command)
Date: Fri, 19 Jun 2020 14:56:41 +0200

On 6/18/20 11:50 PM, Jacob Bachmeyer wrote:
> Tom de Vries wrote:
>> On 6/18/20 1:20 AM, GNU bug Tracking System wrote:
>>  
>>> Your bug report
>>>
>>> #41914: [PATCH] Propagate return value of auto-loaded command
>>>
>>> which was filed against the dejagnu package, has been closed.
>>>
>>> The explanation is attached below, along with your original report.
>>> If you require more details, please reply to 41914 <at> debbugs.gnu.org.
>>>
>>>     
>>
>> AFAICT, no fix was committed to master. So, I'm not sure I understand
>> why this bug was closed.
> 
> It was closed because a fix had been rolled into the fix for #41824. 
> Linus' Law applied here:  we both found the same bug and actually fixed
> it the same way.  I merged the comment and ChangeLog entry from your
> patch into the temporary "PR41824" branch.
> 
> Is there a better way to record this circumstance in debbugs than
> closing the second bug?

Well, in my understanding, yes, by:
- mentioning the circumstance without closing the bug, and
- closing the bug once the fix is present on master.

It's just that in all projects I worked in, this practice is adhered to,
with the specific purpose of being able to track problems to completion.

Of course, if the dejagnu project doesn't adhere to this, I have no
issues with that.  I just asked the question because I was surprised.

Thanks,
- Tom

Message #22 received at 41914-done <at> debbugs.gnu.org (full text, mbox):

From: Jacob Bachmeyer <jcb62281 <at> gmail.com>
To: Tom de Vries <tdevries <at> suse.de>
Cc: 41914-done <at> debbugs.gnu.org
Subject: Re: bug#41914: closed (Re: bug#41914: [PATCH] Propagate return value
 of auto-loaded command)
Date: Fri, 19 Jun 2020 19:07:02 -0500

Tom de Vries wrote:
> On 6/18/20 11:50 PM, Jacob Bachmeyer wrote:
>   
>> Tom de Vries wrote:
>>     
>>> On 6/18/20 1:20 AM, GNU bug Tracking System wrote:
>>>  
>>>       
>>>> Your bug report
>>>>
>>>> #41914: [PATCH] Propagate return value of auto-loaded command
>>>>
>>>> which was filed against the dejagnu package, has been closed.
>>>>
>>>> The explanation is attached below, along with your original report.
>>>> If you require more details, please reply to 41914 <at> debbugs.gnu.org.
>>>>
>>>>     
>>>>         
>>> AFAICT, no fix was committed to master. So, I'm not sure I understand
>>> why this bug was closed.
>>>       
>> It was closed because a fix had been rolled into the fix for #41824. 
>> Linus' Law applied here:  we both found the same bug and actually fixed
>> it the same way.  I merged the comment and ChangeLog entry from your
>> patch into the temporary "PR41824" branch.
>>
>> Is there a better way to record this circumstance in debbugs than
>> closing the second bug?
>>     
>
> Well, in my understanding, yes, by:
> - mentioning the circumstance without closing the bug, and
> - closing the bug once the fix is present on master.
>
> It's just that in all projects I worked in, this practice is adhered to,
> with the specific purpose of being able to track problems to completion.
>
> Of course, if the dejagnu project doesn't adhere to this, I have no
> issues with that.  I just asked the question because I was surprised.
>   

Fair enough; I am new to package maintenance and probably closed the bug 
earlier than needed.  The fix is definitely going into 1.6.3, but I was 
not aware of a convention of holding bugs open until the patches have 
landed on master.  I will try to do better in the future.


-- Jacob

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.