Stop disabling Windows LF -> CRLF translation

[johnstep]

Signed-off-by: John Stephens johnstep@docker.com

runc was changed to remove the unix.ONLCR terminal flag in opencontainers/runc#1146, which stops the translation of LF to CRLF. The Windows console should keep its default behavior of converting LF to CRLF.

@jhowardmsft

[codecov-io]

Codecov Report

Merging #151 into master will increase coverage by 0.02%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #151      +/-   ##
==========================================
+ Coverage   44.96%   44.98%   +0.02%     
==========================================
  Files         169      169              
  Lines       11381    11374       -7     
==========================================
  Hits         5117     5117              
+ Misses       5971     5964       -7     
  Partials      293      293

[vdemeester]

LGTM 🐯

[johnstep]

I need to update this to keep the former behavior for older versions of runc.

[thaJeztah]

@johnstep do you want to do that as part of this PR, or a follow up?

[thaJeztah]

@johnstep looks like part of this was added/changed in moby/moby#23878 if that helps

ping @jstarks @jhowardmsft PTAL

[jstarks]

Hmm. I think Windows containers will still do the LF->CRLF conversion on the server side, so this would probably break Windows containers terminal output as-is.

[johnstep]

@jstarks I could not find any issues when I tested, legacy and new console, Windows and Linux containers, interactive and not, TTY and not.

[jstarks]

Hmm... I can see how in cases where LF -> CRLF is supposed to happen, this change is harmless because a second extra CR is not going to cause any problems.

But how does this set of changes work when a process inside the container wants to avoid LF -> CRLF conversion entirely? For example, on Linux emacs will use LF to advance the row and expect that the column will stay the same.

If the CLI is now going to always convert LF to CRLF, then it seems that this will break.

But maybe I don't quite understand the full set of changes in play here.

[jstarks]

Basically it seems that a container process used to be able to turn off ONLCR and it would be respected by the user's terminal emulator, but now there is no way for a container process to control the effective state of ONLCR.

I'm not so sure turning off ONLCR by default was the right choice in runc. And now we're trying to paper over that in the CLI.

[thaJeztah]

Yes, there was some prior discussion about changing the behaviour (given that having ONCLR enabled is - AFAICT - the default behaviour). Looking at the changes needed in Docker, at least it looks to be a breaking change in RunC

[crosbymichael]

We are going to revert the change in runc.