RFC: os2 = os - cruft

[timotheecour]

goal

• new os2 module intended as a replacement/evolution of std/os, fixing some design flaws in std/os that are hard to fix because of backward compatibility concerns
• API's may be incompatible with std/os but it won't cause breaking changes for clients of std/os since it's a new module
• an optional os2conv module (containing converters to/from Path<=>string) can optionally be imported as a temporary measure to ease migration for modules wishing to migrate from os to os2
• more intuitive, strongly typed, efficient API
• should be ok to use inside compiler (like the rest of fusion)

implementation

• share code as much as possible with os; if needed, refactor os by introducing a common base osimpl that both os and os2 will import; avoid code duplication unless absolutely needed

• we start with fusion/experimental/os2; breaking changes are allowed and expected but a guideline is that for each breaking API change we provide (in changelog) how to migrate to newer API

• when we're confident no major breaking change will be needed, we move the stable code to fusion/os2 and fusion/experimental/os2 calls import fusion/os2; export os2 so that clients of fusion/experimental/os2 will keep working; unstable API's can

strongly typed

still open debate on what's the right type representation for paths.

• option 1: use string
  that's what std/os (or io.nim etc) uses; it makes type-unsafe API's, eg: writeFile("foo", "bar") is easy to misuse
  it causes issues where string semantics are used instead of path semantics, eg for equality:

"./foo//bar/" != "foo/bar/" # even though paths are same

• option 2: same as compiler/pathutils.nim

type
  AbsoluteFile* = distinct string
  AbsoluteDir* = distinct string
  RelativeFile* = distinct string
  RelativeDir* = distinct string

IMO this is not the right approach, as explained here nim-lang/RFCs#71, and experience shows this doesn't work well and is proably not fixable as is

• option 3:

type Path* = distinct string

this is a lighter weight approach to pathutils.nim and keeps most of its benefits (eg writeFile can't be misused, equality is guaranteed to use path semantics) without its flaws

• option 4:

type Path* = object
  value: string
proc p(a: string): Path =
  # normalize `a`

eg:
let file = p"./foo//bar/"
this option eagerly normalizes paths upon construction

API's benefiting from strong typing

let a = path"./foo//bar/baz"
doAssert a[0] == path"foo"
doAssert a[^1] == path"baz"

empty path means uninitialized, not "."

a / b

• raise if a is empty (source of many bugs in the past where
• raise if b is absolute, or return b

prefer identifiers to operators

• / is excellent and stays
• /../ is cruft, rarely used, and can be replaced by rebase or something else

raise, don't silently accept invalid data

rationale: invalid data (or even returning "" and propagating it) simply hides a bug under the rug; the bug is still there, it's just hard to track down to its origin. Embrace exceptions at the point where invalid data is found.

• walkDir defaults to checkDir=true
• getEnv("nonexistant") raises (but getEnv("nonexistant", "def") does not)

orthogonal API's

• findExe, getAppFilename should not resolve symlinks; if user wants to resolve symlinks he can via `findExe.resolveSymlinks
  symlinks are usually there for good reasons, eg to mask a path that has spaces or shell unfriendly characters, eg:

ln -sf "Default (OSX).sublime-keymap" Default_OSX.sublime-keymap
ln -sf /Applications/Google\ Chrome.app/Contents/MacOS/Google\ Chrome ~/shortcuts/chrome

or semantic meaning eg in brew:

ls /usr/local/opt/go
lrwxr-xr-x 1 timothee 17 Mar 27 00:20 /usr/local/opt/go -> ../Cellar/go/1.14/

=> you want to keep it as /usr/local/opt/go, not resolve as /usr/local/Cellar/go/1.14 which looses its meaning as "whatever's the version chosen by brew"

extensions have leading dots

• rationale: I don't know any system that doesn't use leading dot; many things would break anyways; we shouldn't complicate things for this hypothetical OS

use the result: var Path pattern

• still need the ./ PR to make this easy to use (it's already implemented and works; just needs minor polish)

consistent naming (in docs and/or arg names)

• path: any path (dir/file/filename), eg: /usr, files.tar.gz
• file: any file (no dirs); note that for API's that don't touch filesystem, this is purely documentation
  eg: bar/files.tar.gz
• filename: any file name, eg: files.targ.gz but not bar/files.tar.gz
• cfile: a File instance (ie, c FILE*)
• add a rule to NEP1: it's subjectVerb (fileExists) not verbSubject (existsFile) so all we need is: proc fileExists(a: Path): bool and we drop existsFile

remove redundant API's

• expandSymlink

split module as appropriate (but with imports, not includes)

• includes are just can of worms that have very specific uses cases but aren't needed here
• temp file API (eg getTempDir) are in a separate module fusion/tempfiles.nim
  there's a lot of functionality in a temp file API; deserves dedicated module
• fusion/envs is its won module (anything related to environment variable handling)
• execShellCmd doesn't belong in os2
  => redundant with osproc (or fusion/osproc2, more on that later, to address independently settable stdin/stdout/stderr streams)

backends:

• VM partial support without --vmopsDanger, VM full support with --vmopsDanger, the VM name for foo is foo, not staticFoo (rationale: code that works at RT works at CT with no change whenever possible)
• js backend support with -d:nodejs via requires("fs")

symlink support on windows

exception handling

• internal logic results in Defect (eg assert or doAssert, but doAssert is preferred unless it'd result in measurable overhead; unlikely for most of os2)
• argument validation results in (subclasses of) CatchableError