Re: Pass the -noecho option to spawn

Jonathan Wakely wrote:
> On Tue, 20 Dec 2022 at 02:51, Jacob Bachmeyer <[email protected]> wrote:
>   
>> Jonathan Wakely wrote:
>>     
>>> Is there a reason that dejagnu's local_exec proc doesn't use the
>>> -noecho option for spawning the command?
>>>
>>> Every log contains the spawned commands twice because dejagnu prints
>>> it then spawn repeats it (with an annoying \r appended, making copy &
>>> paste from the logs more awkward than it needs to be).
>>>
>>> [...]
>>>       
>> The main problem with this patch is that lib/remote.exp:local_exec is
>> documented, so it can also be called by testsuite code.  If local_exec
>> is called directly, only the message from spawn will appear in the log,
>> since the "Executing on ..." message is from remote_exec.  Using -noecho
>> would suppress that message, leaving no indication at all of what was
>> run in the log.  This is not a problem for your use case, but may cause
>> problems for other testsuites, so the patch as suggested cannot be taken
>> upstream.
>>     
>
> I see. How about added an additional {noecho 0} parameter to
> local_exec, causing it to add -noecho to the spawn command?
> The existing behaviour wouldn't change, but remote_exec could pass 1
> there to request the -noecho.
>   

The parameter lists for local_exec and remote_exec are currently very 
similar, so I would be reluctant to add a "silent" option to 
local_exec.  One possibility I am considering is to use [info level] to 
determine if local_exec was called by remote_exec and emit a message 
"Executing locally: ..." if not.  This would solve both problems (moving 
the message to local_exec entirely, that is, having remote_exec only 
emit its message if it is not going to call local_exec, was rejected on 
the grounds that the message from remote_exec contains a detail (the 
requested location) that local_exec does not know) and would also fix an 
apparent bug in local_exec, where the command to be run is echoed if 
spawn is used, but no similar output is produced if the code path using 
open is used.  There appear (at first glance) to be enough other bugs 
and convoluted code in local_exec that any changes there are likely to 
snowball into a near-rewrite, so I am waiting until I develop a better 
understanding of (and tests for) that part of DejaGnu before touching it.

As I mentioned before, this is probably not going to land for 1.6.4, 
since the emphasis on enhancements for 1.6.4 is mostly on lib/target.exp 
and the unit testing support, which still had some significant known 
bugs in 1.6.3, for example, dejagnu.h was very much not thread-safe.  
Major bugs are also priorities, but spurious messages in the log are, at 
most, minor bugs, and there are currently no tests written for 
lib/remote.exp, so lib/remote.exp will probably be a focus for a later 
release.  This issue has been added to my notes with the other 
lib/remote.exp issues and will be addressed in the future.


-- Jacob