Revert "Make 'echo' raise IOErrors when appropriate (#16367)"

[narimiran]

This is a too big breaking change to be part of Nim 1.x

[timotheecour]

This is not a good revert, and merging this PR within 2 hours of opening it leaves very little room for feedback (time zones differ).

Also, when you revert a bugfix, please re-open the corresponding bug (#16366); I just did; and also add a notification on the PR that was reverted so this doesn't go un-noticed.

We already provided a -d:nimLegacyEchoNoRaise flag that a user can put in their user or project config as mitigation during a transition period, it's a good enough workaround (and you had agreed with it in #16367 (comment)); you're very unlikely to depend on both wanting to raise vs not wanting to raise in a program.

At the very least, a pushable --experimental:echoRaises flag should be introduced to replace it.

After the revert, you're silently ignoring bugs again, which affects programs and makes error diagnostic harder.

D: raises "Attempting to write to closed File"

import std.stdio;
void main(string[]args){
  writeln("ok1");
  stdout.close();
  writeln("ok2");
}

python: raises ValueError: I/O operation on closed file.

import sys
print("ok1")
sys.stdout.close()
print("ok2")

ruby: raises `write': closed stream (IOError)

puts "ok1"
STDOUT.close
puts "ok2"

[Varriount]

I agree, I honestly don't see why this was required. Perhaps allow the old behavior under a define, sure, but raising an exception is expected.

[Araq]

It doesn't matter, it was a breaking change, after more than 10 years. If you need the exception raising use system.write. Nim being "consistent" with itself has its own value, it doesn't have to be consistent with languages it never was consistent with.

[stefantalpalaru]

Is this going to be backported to 1.6? I get this confusing error that doesn't seem to break compilation:

[amd] 11 Tue Feb 08 23:43:11 |/mnt/sde1/storage/nimbus-eth2|
stefan$ make NIM_COMMIT="version-1-6" NIMFLAGS="--warnings:off" -j8
Building: build/nimbus_beacon_node
Building: build/deposit_contract
Building: build/resttest
Building: build/logtrace
Building: build/ncli
Building: build/ncli_db
Building: build/wss_sim
[using Nim version version-1-6]
Building: build/stack_sizes
[using Nim version version-1-6]
[using Nim version version-1-6]
[using Nim version version-1-6]
[using Nim version version-1-6]
[using Nim version version-1-6]
[using Nim version version-1-6]
[using Nim version version-1-6]
io.nim(139)              raiseEIO
Error: unhandled exception: errno: 32 `Broken pipe` [IOError]

[normal output follows, with no non-zero exit codes]

[narimiran]

Is this going to be backported to 1.6?

This revert is already part of 1.6.0, there is nothing to backport here.

[stefantalpalaru]

Is it, though? I can find the first few lines still there, in "version-1-6", slightly modified:

Nim/lib/system/io.nim

Lines 233 to 235 in 7994556

	if w != 0:
	if doRaise: raiseEIO("cannot write string to file")
	break

[narimiran]

That was not the part of the commit (#16367) this PR reverts.

It was introduced in https://github.com/nim-lang/Nim/pull/12385/files, back in 2019.

[stefantalpalaru]

I got a full backtrace from the latest "version-1-6":

/mnt/sde1/storage/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/compiler/nim.nim(138) nim
/mnt/sde1/storage/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/compiler/nim.nim(90) handleCmdLine
/mnt/sde1/storage/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/compiler/cmdlinehelper.nim(41) processCmdLineAndProjectPath
/mnt/sde1/storage/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/compiler/nim.nim(63) processCmdLine
/mnt/sde1/storage/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/compiler/commands.nim(1074) processSwitch
/mnt/sde1/storage/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/compiler/commands.nim(844) processSwitch
/mnt/sde1/storage/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/compiler/commands.nim(99) writeVersionInfo
/mnt/sde1/storage/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/compiler/msgs.nim(313) msgWriteln
/mnt/sde1/storage/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/lib/system/io.nim(156) checkErr
/mnt/sde1/storage/nimbus-eth2/vendor/nimbus-build-system/vendor/Nim/lib/system/io.nim(139) raiseEIO
Error: unhandled exception: errno: 32 `Broken pipe` [IOError]