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

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jan 16, 2019, 10:34 UTC
Message-ID: <20190116103438.GG840@szeder.dev>
In-Reply-To: <5aa5d047c828a0db10544c706dd595f09db3215d.1547581039.git.jonathantanmy@google.com>

On Tue, Jan 15, 2019 at 11:40:28AM -0800, Jonathan Tan wrote:

Show 9 quoted lines

> 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