Subject: Re: [PATCH v2 2/4] sideband: reverse its dependency on pkt-line

From: SZEDER Gábor <[email-masked]>
Date: Wed, 16 Jan 2019 11:34:38 +0100

On Tue, Jan 15, 2019 at 11:40:28AM -0800, Jonathan Tan wrote:
> A subsequent patch will teach struct packet_reader a new field that, if
> set, instructs it to interpret read data as multiplexed. This will
> create a dependency from pkt-line to sideband.
> 
> To avoid a circular dependency, split recv_sideband() into 2 parts: the
> reading loop (left in recv_sideband()) and the processing of the
> contents (in demultiplex_sideband()), and move the former into pkt-line.
> This reverses the direction of dependency: sideband no longer depends on
> pkt-line, and pkt-line now depends on sideband.

In the last couple of days I noticed occasional but frequent failures
in the test 'leading space' in 't5409-colorize-remote-messas.sh' on
'pu' and on this topic.  Bisect suggests that this patch is the
culprit, but of course bisecting an occasional test failure can't be
completely trusted.

The trace output of the failing test looks like this:

  + git --git-dir child/.git -c color.remote=always push -f origin HEAD:refs/heads/leading-space
  + cat output
  remote: error: error        
  remote: ERROR: also highlighted        
  remote: hint: hint        
  remote: hinting: not highlighted        
  remote: success: success        
  remote: warning: warning        
  remote: prefixerror: error        
  remote:   
  remote: error: leading space        
  remote:             
  remote: Err        
  To /<...>/trash directory.t5409-colorize-remote-messages.stress-4/.
   * [new branch]      HEAD -> leading-space
  + test_decode_color
  + awk 
  < ... snip enormous awk script ... >
  + grep   <BOLD;RED>error<RESET>: leading space decoded
  error: last command exited with $?=1
  not ok 6 - leading space

Notice how that "error: leading space" lines up with the other
messages.  On 'master' that line looks like this:

  remote:   error: leading space